set_value in Form_validation doesn’t escape less than ( |
|||
|---|---|---|---|
| Date: | 10/17/2008 | Severity: | Critical |
| Status: | Resolved | Reporter: | beemr |
| Version: | 1.7.0 SVN | ||
| Keywords: | Libraries, Form_validation | ||
| Forum Thread: | http://codeigniter.com/forums/viewthread/93421/ | ||
Description
Less than (<) doesn’t get escaped. This breaks the loadXML routine if you are loading views as XML. I’ve bandaged it by double escaping < to < in set_value from Form_validation like so:
function set_value($field = ‘’, $default = ‘’)
{
if ( ! isset($this->_field_data[$field]))
{
return $default;
}
return preg_replace(”/</”, “<”, $this->_field_data[$field][‘postdata’]);
return $this->_field_data[$field][‘postdata’];
}
Code Sample
I send output to a custom XSLT function from my custom template model 'render.php'
Line 211: $xml->loadXML($this->output->get_output());
Because the < passes through to the XML parser, it invalidates the markup upon set_value when an error causes Form_validation to reload the form.
Expected Result
The password field may only contain alpha-numeric characters, underscores, and dashes.
Actual Result
A PHP Error was encountered
Severity: Warning
Message: DOMDocument::loadXML() [function.DOMDocument-loadXML]: Unescaped ‘<’ not allowed in attributes values in Entity, line: 11
Filename: models/render.php
Line Number: 211
Comment on Bug Report
| Posted by: beemr on 17 October 2008 5:19pm | |
|
Ugh! the escapes were unescaped. One more try:
return preg_replace(”/</”, ”&lt;”, $this->_field_data[$field][‘postdata’]);
Also, Form_validation isn’t in the Bug Tracker yet, so I classified it under form_helper. |
| Posted by: Rick Ellis on 18 October 2008 3:47pm | |
|
I don’t think this is a validation issue, because it isn’t the Form_validation’s responsibility to convert characters into entities automatically. It is intended to validate data, not change data. If you need certain characters to be converted you should create a callback or other prepping function to do that for you. |
| Posted by: Rick Ellis on 18 October 2008 3:49pm | |
|
|
By the way, the set_value() function does escape characters, as it is intended to make data form safe. You might use it. |
| Posted by: beemr on 18 October 2008 11:50pm | |
|
|
I am using set_value(), but when it returns the less-than after a validation failure, the less-than is not escaped in the value attribute. I changed the return on the set_value() in form_validation, because that is eventually being called by the set_value in form_helper. If the unescaped return is intentional, my bad for listing the bug. I’ll write the regex into my loadXML routine. Thanks for checking this out for me. |
| Posted by: Rick Ellis on 20 October 2008 1:58pm | |
|
|
OK, I just checked the direct output of set_value, and you’re right, it’s not escaping characters. Seems Safari does this automatically… who knew? Anyway, I’ve committed a change in the form helper so it should work as expected now. |
| Posted by: beemr on 20 October 2008 2:44pm | |
|
|
Great! Thanks mucho, Rick! |
| Posted by: beemr on 20 October 2008 3:45pm | |
|
|
FYI—It looks like Google Chrome doesn’t auto-escape. I know that Chrome’s Webkit is a few paces behind Safari’s, but it looks like auto-escaping is Safari’s “feature” rather than Webkit’s. |
| Posted by: beemr on 20 October 2008 5:17pm | |
|
|
Just tried it out, and output still has unescaped less-thans going into loadXML(). I think I see where you are going with the new validation, so now my fix is in form_helper(). Just before form_prep() returns $str, add the following regex:
$str = reg_replace("/</","&lt;",$str)
loadXML won’t choke on greater-than, so I think the fix can be relegated to just less-than. Double-escaping all entities also worked, but I’m just trying to keep my solution focused. |
| Posted by: beemr on 20 October 2008 5:19pm | |
|
|
Ugh, the search string should’ve been “/”+ampersand+“lt;/” |
| Posted by: Rick Ellis on 20 October 2008 5:20pm | |
|
|
But form_prep uses htmlspecialchars, which converts less-than signs. It’s working fine for me as is. |
| Posted by: Rick Ellis on 20 October 2008 5:22pm | |
|
|
Oh, wait. I see what you’re saying… converting the ampersand. Well, we really don’t want the validation class to do that automatically. That’s an XML requirement, so you might use xml_convert() from the xml helper. |
| Posted by: beemr on 20 October 2008 8:16pm | |
|
|
Problem with that is that I’m parsing the final output and all of the tags need to pass through as xml tags. xml_convert() is indiscriminate at that point. I’ve been changing my code at the same time as updating from SVN, so I know there was a turning point between 1.6.4 and 1.7svn. I just can’t say for sure on which side of the updates the change came, mine or CI’s. In the meantime, one last down-and-dirty fix:
$str = preg_replace_callback('/(value=)([\'"])+([^\\2])\\2/', create_function('$matches','return $matches[1].$matches[2].htmlspecialchars($matches[3]).$matches[2];'), $this->output->get_output()); |
| Posted by: beemr on 20 October 2008 8:55pm | |
|
|
Is it possible that during the creation of the master fields array an escaping routine was skipped in 1.7 that was present in 1.6.4? Maybe before lines 643 and 647? |
