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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Remove reference to IE6 xss protection in check_plain documentation » Remove reference to IE6 xss protection in check_plain documentation, or in function?
Component: documentation » base system

Thanks 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.

coderintherye’s picture

Well, 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?)

Damien Tournoud’s picture

We can remove the reference to IE6, because there is more then IE6 in making sure that we output proper UTF-8.

jhodgdon’s picture

Title: Remove reference to IE6 xss protection in check_plain documentation, or in function? » Remove reference to IE6 xss protection in check_plain documentation
Component: base system » documentation
Issue tags: +Novice

Thanks 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.

tomgeekery’s picture

I 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.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

That'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

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks again - committed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.