Files: 
CommentFileSizeAuthor
#2 1533448.patch2.28 KBlotyrin
PASSED: [[SimpleTest]]: [MySQL] 35,068 pass(es).
[ View ]

Comments

Assigned:Unassigned» lotyrin

Assigned:lotyrin» Unassigned
StatusFileSize
new2.28 KB
PASSED: [[SimpleTest]]: [MySQL] 35,068 pass(es).
[ View ]

Initial start.

Remaining output from coder:

core/modules/user/user.module:
+993: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
+1031: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
core/modules/user/user.test:
+2076: [normal] global variables should start with a single underscore followed by the module and another underscore
core/modules/user/user.admin.inc:
+77: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
core/modules/user/user.pages.inc:
+75: [normal] global variables should start with a single underscore followed by the module and another underscore
+100: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.
+106: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.
+323: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.

Not sure what to do about the complaint about global names. I'm guessing that doesn't apply to core global space?

All of the string filtering still needs checked out. I imagine most if not all of them are false positives, but this issue is a good time to quadruple check.

Status:Active» Needs review

Status:Needs review» Needs work

They are all fine. The only ones I would find debatable are:

+100: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.
+106: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.

Both of those cases use a !url placeholder for inserting a URL into t(). I think we usually use @url for that, so that !foo means we specifically want to allow HTML/unfiltered text, but that is only an asthetical question, as the URLs that are generated contain no unsafe output anyway. Don't know if we want to change that.

As a note #1326666: Clean up API docs for user module is still open and this should not contain any docblock changes until that is fixed.

Assigned:Unassigned» lotyrin

Assigned:lotyrin» Unassigned

Unassigning because I'm not sure I'll have time for these issues and I want to make sure people know they can jump in without stepping on my toes.

As part of #1800046: [META] Add missing type hinting to core docblocks, I created a sub-issue #1800174: Add missing type hinting to User module docblocks to address the addition of missing type hinting to the User module. A first patch is posted there addressing about a dozen function docblocks. Reviews are welcome.

Status:Needs work» Postponed

Postponing till feature freeze. If you want to help in the meantime, please work on the blockers on the meta issue. Thanks!

Issue summary:View changes
Status:Postponed» Active