URL for Non-Existent Users Shows Messages for Current Logged In User [#850552]

Hey Berdir and everyone,

If a user with "Read all private messages" permission navigates to the message page of a non-existing user (at /user/[made-up-uid]/messages), instead of getting a Page Not Found error, he sees a list of his own messages (equivalent to /messages).

This confused me at first because I inadvertently entered a typo in the URL when testing, and then was puzzled by the list of messages I saw.

Probably we should return a "Page Not Found" if the user (and hence his messages) doesn't exist.

--Ben

P.S. As a side note, it occurred to me that it might be helpful to have a page for administrators that lists all messages on the site. This would have been helpful when testing to keep track of everything. But I can put in a separate feature request for this.

Comment	File	Size	Author
#5	not_found3.patch	7.13 KB	Berdir
#5
#2	not_found2.patch	5.45 KB	Berdir
#2
#1	not_found.patch	3.59 KB	Berdir
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Berdir

German

Switzerland

CreditAttribution: Berdir commented 12 July 2010 at 08:53

Status:

Active

» Needs review

File	Size
not_found.patch	3.59 KB

Makes sense.

Patch moves the account checking into a separate function, which can return MENU_ACCESS_DENIED/MENU_NOT_FOUND.

The old code assumed that we access messages of other users through messages/uid, that's not correct anymore.

Remember to rebuild the menu after applying the patch.

Comment #2

Berdir

German

Switzerland

CreditAttribution: Berdir commented 12 July 2010 at 09:09

File	Size
not_found2.patch	5.45 KB

Also changed menu definitions in privatemsg_filter.module

Comment #3

12 July 2010 at 09:20

Status:

Needs review

» Needs work

The last submitted patch, not_found2.patch, failed testing.

Comment #4

BenK CreditAttribution: BenK commented 12 July 2010 at 16:30

Okay, will test this when the patch passes the tests...

Comment #5

Berdir

German

Switzerland

CreditAttribution: Berdir commented 12 July 2010 at 18:29

Status:

Needs work

» Needs review

File	Size
not_found3.patch	7.13 KB

Here is a new version.

Instead of renaming the form function, I left that and named the new page callback privatemsg_list_page(). It requires not so many changes to hook_form_alter() and similiar functions in that case.

This should hopefully pass now.

I've also found a bug in a test, which access messages/thread_id instead of messages/view/thread_id

PS: If you can, it would be great if you can join #drupal-games (which is more or less the inofficial privatemsg channel, hasn't been used much recently). Should be easier to coordinate testing and discussing. See http://drupal.org/irc

Comment #6

BenK CreditAttribution: BenK commented 14 July 2010 at 01:35

Status:

Needs review

» Needs work

Hey Berdir,

I tried out the patch in #5 and I suddenly got the following error message when visiting user/[uid]/messages for any user:

Notice: Trying to get property of non-object in privatemsg_sql_list() (line 891 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.module).
Notice: Trying to get property of non-object in privatemsg_sql_list() (line 927 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.module).

Also, when visiting /messages for the current user, I now get the following errors:

Warning: Missing argument 4 for privatemsg_list() in privatemsg_list() (line 142 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.pages.inc).
Notice: Undefined variable: account in privatemsg_list() (line 163 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.pages.inc).
Notice: Trying to get property of non-object in privatemsg_sql_list() (line 891 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.module).
Notice: Trying to get property of non-object in privatemsg_sql_list() (line 927 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.module).
Notice: Undefined variable: account in privatemsg_list() (line 174 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.pages.inc).
Notice: Undefined variable: account in privatemsg_list() (line 180 of /Users/benkaplan/git/drupal/sites/all/modules/contrib/privatemsg/privatemsg.pages.inc).

Note that these errors appear to be caused by the patch... if I remove the patch the above errors go away.

Also, if I visit user/[uid]/messages for a made-up user (a user who doesn't exist), I see the message list field headers with "No messages available" beneath them. Is this intended or were we going to return a Page Not Found error?

Thoughts?

--Ben