API page: http://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/functio...
This page is still referring to using check_plain to protect against cross-site scripting attacks in IE6.
D8 does not support IE6 so this language should not be in there. I think that this issue: http://drupal.org/node/308865 removed IE6 specific stuff, though check_plain itself seems to still do a UTF-8 check. I don't know for sure which it is, but I do know the documentation should not say we are doing something for the sake of IE6. However, I'll understand if this needs to be filed against the code instead.
Comment | File | Size | Author |
---|---|---|---|
#5 | 1804054-remove-ie6-reference-from-check_plain-doc.png | 59.59 KB | tomgeekery |
#5 | remove-ie6-reference-from-check_plain-doc-1804054-5.patch | 537 bytes | tomgeekery |
Comments
Comment #1
jhodgdonThanks for reporting this!
It looks like we need to figure out why we are doing the UTF-8 check in check_plain(), since the documentation for previous versions says it is for IE6. I think we should put this in "base system" for the moment to discuss whether the UTF-8 check should be removed. If it should be removed, then remove it. If it shouldn't, maybe the base system maintainers can tell us why it needs to be there so we can update the documentation.
Comment #2
coderintherye CreditAttribution: coderintherye commentedWell, the issue it was trying to solve seems to be no longer an issue: http://stackoverflow.com/questions/5928476/is-this-horrifying-xss-vector...
We can see here what the params are for htmlspecialchars which check_plain is basically just a wrapper for http://us3.php.net/manual/en/function.htmlspecialchars.php
So, in a sense, it would seem fine to remove 'UTF-8'. However I have only one misgiving about creating a patch to do that. The PHP docs say that PHP < 5.4 that param will default to 'ISO-8859-1' and PHP >= 5.4 will default to 'UTF-8'.
Given that 8.x will run on PHP 5.3, I guess removing it now could produce inconsistent behavior between 5.3 and 5.4 installations, so not sure if that is a blocker.
Regardless this could be removed when Drupal adopts min. requirement of PHP >= 5.4. (9.x?)
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedWe can remove the reference to IE6, because there is more then IE6 in making sure that we output proper UTF-8.
Comment #4
jhodgdonThanks Damien!
OK, then this is probably a good Novice project -- we just want to remove (for D8 only) the part that says "to prevent ... on IE 6" from the documentation, and leave the code as it is.
Comment #5
tomgeekery CreditAttribution: tomgeekery commentedI am attempting my first patch here. Hopefully this is correct! I removed the "to prevent ... on IE6" text in the check_plain function in bootstrap.inc. Attaching a screenshot to show what I modified.
Comment #6
jhodgdonThat's perfect, thanks! (assuming the test bot turns green) I'll get it committed probably tomorrow [Note to self: this is D8 only!!].
One note: In the future, it's not necessary (or even a good idea) to attach a screen shot -- the patch file should stand on its own. Anyway, congratulations on your first patch! You probably already know about it, but there's a guide to contributing patches at:
http://drupal.org/novice
Comment #7
jhodgdonThanks again - committed to 8.x.