user_load() doesn't check for invalid arguments which leads to the following errors when accessing a URL like user/foo:

    * warning: Invalid argument supplied for foreach() in /home/amr/public_html/drupal/cvs/modules/user/user.module on line 89.
    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 query: SELECT * FROM users u WHERE in /home/amr/public_html/drupal/cvs/includes/database.mysql.inc on line 172.
CommentFileSizeAuthor
#4 user_url_check2.patch535 bytesAmrMostafa
#2 user_url_check.patch560 bytesAmrMostafa
user_load_check.patch605 bytesAmrMostafa
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

there are many many things we could check for like this, but we don't. these checks bloat the codebase IMO. my .02

AmrMostafa’s picture

AmrMostafa CreditAttribution: AmrMostafa commented
FileSize
user_url_check.patch560 bytes

There is a better place for this check ;-), in the menu system argument translation function user_current_load($arg), it makes more sense there as this is URL specific, stays away from user_load().

If I may say so, I completely agree about the code bloat, but to some extent. I believe that we shouldn't protect from programmers mistakes yes, but we should protect from user mistakes that lead to invalid SQLs hitting the database and filling the watchdog ;-)

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work

Ah. Use a ?: and I think we will be fine.

AmrMostafa’s picture

AmrMostafa CreditAttribution: AmrMostafa commented
Status: Needs work » Needs review
FileSize
user_url_check2.patch535 bytes

Rerolled with ?:)

chx’s picture

chx CreditAttribution: chx commented

Better, but still, visiting user/foo when foo is a string and is not defined -- do we want to make that not spourt out errors?

keith.smith’s picture

keith.smith CreditAttribution: keith.smith commented
Status: Needs review » Needs work

Patch no longer applies.

# patch -p0 < user_url_check2.patch
patching file modules/user/user.module
Hunk #1 FAILED at 966.
1 out of 1 hunk FAILED -- saving rejects to file modules/user/user.module.rej

dpearcefl’s picture

dpearcefl CreditAttribution: dpearcefl commented

Is there any interest in this issue any more?

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.