Welcome Guest, Not a member yet? Register   Sign In
Sanitizing Form inputs form quotes (' and ")
#1

Hi,

I was facing an issue where, if I post characters like single quote ( ' ) or double quote( " ) as a form input field, and due to some validation failure.. if I want the form to retain those values, it was showing garbage values like ' for single quote and " for double quote.

I tried many things like accessing form fields from $this->input->post instead of $_POST, enabling XSS filtering in config, using $this->security->xss_clean(), etc, but nothing helped.

Finally, I went to "system\helpers\form_helper.php" and changed the function form_input at line number 171 to following -

PREVIOUS : $defaults = array('type' => 'text', 'name' => (( ! is_array($data)) ? $data : ''), 'value' => $value);

CHANGED: $defaults = array('type' => 'text', 'name' => (( ! is_array($data)) ? $data : ''), 'value' => html_entity_decode($value, ENT_QUOTES, 'UTF-8'));

And this seemed to be working fine so far.
I want to know if this is correct way to achieve what I want.

Thanks.
Reply
#2

Use set_value() in your view:
http://www.codeigniter.com/userguide3/he...#set_value
Reply
#3

(12-30-2014, 05:05 AM)sujit510 Wrote: Finally, I went to "system\helpers\form_helper.php" and changed the function form_input

I want to know if this is correct way to achieve what I want.

Hi Sujit, aside from MWhitney's good advice, there's a much better way of doing what you did.

If you change something in the system folder, the problem is this -- next time you upgrade, you will have to make those changes again. And what if you forget?

Instead of changing a class, function, or helper in the system folder, you can "extend" them by creating their replacements in your application folder. That way, upgrading CodeIgniter won't break your application. The Codeigniter Documentation shows how to do that. Click the link and scroll down to "Extending" Helpers. It's very easy to do. Good luck! Smile
Reply
#4

While RobertSF's advice is good if/when you're extending helpers, I want to make my point a little clearer: you should revert your form_helper back to the original version and set your values using set_value(). Even if you continue to use the form_input() helper, the $value parameter should be passed to set_value() first.

This will ensure the original value is retrieved from the form_validation library if a rule was used to validate that field. Otherwise, you're retrieving the data which has already been processed by the library (usually with the intent of storing it in the database), and you may be double-/triple-encoding your data at this point, and potentially creating errors in your output (or worse, opening yourself to the very attacks you were trying to prevent with the XSS filtering).
Reply
#5

Yeah, Sujit, what mwhitney said. I just didn't mention it, but yes, you should change things back the way they were, and use set_value.
Reply
#6

(This post was last modified: 12-30-2014, 10:44 PM by sujit510.)

Thanks both.

I knew this way of not changing framework files directly, and creating overriders instead, but I also wanted to know that if this is a common problem, why CI maintainers haven't addressed this issue yet?

Because it also doesn't make sense of using set_value() everywhere if I have 100 form input fields. Does it?
Reply
#7

If you use the form_validation library, any fields which are processed by the library are updated in the post array. This is why set_value() is needed for these particular instances. Since it is possible that the validation used for a particular view will change over time, set_value() works whether the library was used or not, and for both values which were processed and those which were not.

At some point in the library's development, it was determined that it was more convenient to require using set_value() in the view than to require the use of a different function/method for retrieving the processed data after validation (or maybe this was just necessary for backwards compatibility when someone recognized an issue). Either way, it is not particularly difficult to use set_value() all the time in your views, and it does have some other benefits.

You could also setup your own helper functions to use set_value() if you want to avoid calling it explicitly in your views.
Reply
#8

(12-30-2014, 10:41 PM)sujit510 Wrote: why CI maintainers haven't addressed this issue yet?
You mean the conversion of quotes to "? Well, I think that's really a PHP issue. It's PHP that is doing the conversion, and Codeigniter just isn't reversing the conversion. I believe later versions of PHP don't do that anymore. Also, somebody might have actually wanted that conversion to happen, and then they would have complained that Codeigniter reversed the desired conversion.

Quote:Because it also doesn't make sense of using set_value() everywhere if I have 100 form input fields.
If you're using form_input(), you have to tell it what to display anyway, so using set_value() isn't much different from using $some_value.
PHP Code:
// #1
form_input($field$value);

// #2
form_input($fieldset_value($field); 
Reply
#9

(01-03-2015, 05:28 PM)RobertSF Wrote:
(12-30-2014, 10:41 PM)sujit510 Wrote: why CI maintainers haven't addressed this issue yet?
You mean the conversion of quotes to "? Well, I think that's really a PHP issue. It's PHP that is doing the conversion, and Codeigniter just isn't reversing the conversion. I believe later versions of PHP don't do that anymore. Also, somebody might have actually wanted that conversion to happen, and then they would have complained that Codeigniter reversed the desired conversion.

It's an HTML issue. If you don't convert the quotes when outputting the value to a view, you run the risk of getting broken HTML in your output, e.g.:


Code:
<input type='text' name='something' value='This isn't what I intended at all' />

Which would, at best, give you a text box with the value This isn followed by a bunch of invalid attributes. One of the common outcomes would look like this:

Code:
<input type='text' name='something' value='This isn' t='t' what='what' I='I' intended='intended' at='at' all='all' />
Reply




Theme © iAndrew 2016 - Forum software by © MyBB