Codeigniter: Array passes form validation for string element with the same name + fix

[Stanzi1791]

I've set up form validation for (amongst other fields) a text input box with the name "cms", this is in my application\config\form_validation.php file:

// ...

    array(
        'field' =>  'cms',
        'label' =>  'CMS',
        'rules' =>  'is_natural_no_zero' 
    ),

// ...

If I use a regular <input name="cms" value="1"> this works fine.

Yet, if I tweak the html for the form (trying to forge an invalid POST request), and send a "cms" array to the controller instead of a string value, like this

<input name="cms[]" value="some"><input name="cms[]" value="other">

this array bypasses the validation rule, because CodeIgniter treats 'cms' and 'cms[]' as different things in validation (as it should). However, since both the 'cms' and 'cms[]' values are retrieved from the POST array using $this->input->post('cms'); I have no way to tell which value (the string or the array) I am retrieving.

I believe this is a bug in CodeIgniter's form validation, because this is in the comments in the system\libraries\form_validation.php file (line 661):

// If we get an array field, but it's not expected - then it is most likely
// somebody messing with the form on the client side, so we'll just consider
// it an empty field

And the code beneath that actually tries to set the value to NULL, but then this NULL is then never re-assigned to the POST array.

I've fixed this by creating a file at application\libraries\MY_Form_validation.php, copying the _execute function from system\libraries\form_validation.php and replacing the following:

// If we get an array field, but it's not expected - then it is most likely
// somebody messing with the form on the client side, so we'll just consider
// it an empty field
$postdata = is_array($this->_field_data[$row['field']]['postdata'])
    ? NULL
    : $this->_field_data[$row['field']]['postdata'];

with

// If we get an array field, but it's not expected - then it is most likely
// somebody messing with the form on the client side, so we'll just consider
// it an empty field
$postdata = is_array($this->_field_data[$row['field']]['postdata'])
    ? ""
    : $this->_field_data[$row['field']]['postdata'];
$this->_field_data[$row['field']]['postdata'] = $postdata;

(Using NULL here won't work, because the _reset_post_array function won't re-assign NULL values back to the POST array.)

Did I fix this correctly?
I need to use this in production, so I just want to make sure I didn't break anything else. :)

CodeIgniter version 3.1.9

[narfbg]

If the problem is in _reset_post_array(), why not fix it there?

[Stanzi1791]

If the problem is in _reset_post_array(), why not fix it there?

If I understand correctly, the function I've modified (_execute()) validates the values in the POST array and stores the validated result in $this->_field_data.

All that _reset_post_array() does is reassign the values from $this->_field_data back to the POST array.

I modified this function and not _reset_post_array() because the ("" or NULL) value was never added to $this->_field_data to begin with, in the original CodeIgniter code.

[narfbg]

You initially said using NULL in that particular place won't work because _reset_post_array() won't re-assign NULLs. But you're making quite the hack (with possible side effects) to work around that.

Everything is assigned back to $this->_field_data with or without your change, NULLs just never make it back into $_POST. Hence my question; why not just make _reset_post_array() not ignore NULLs?

[Stanzi1791]

Everything is assigned back to $this->_field_data with or without your change

The NULL value (the result from the test that checks if we have an array where no array is expected) is (in the original CodeIgniter code) never assigned to $this->_field_data.

It is assigned to a $postdata variable, which does not get assigned to $this->_field_data, because of the continue statement half way through the loop in _execute(), line 707 in Form_validation.php

If you set up a form validation rule for a string value and submit an array with the same name, and then do a var_dump($this->_field_data) at the beginning of _reset_post_array(), you will see that this still contains the array and not the NULL value, therefore it isn't enough to modify _reset_post_array() alone.

[narfbg]

I see ... You're right, I missed that continue statement.

Assigning an empty string is a no-go though, and to be honest I'm still not happy with the hacky approach either. Will have to think of some other solution.

[tianhe1986]

How about just setting test result to FALSE and adding corresponding error message ?
Since it needs a string field according to the rule and finally gets a array field, it is unmatched.

[narfbg]

@tianhe1986 Sounds logically ok, but what would you propose code-wise?

[aanbar]

@Stanzi1791 I have submitted a pull request for this, Please have a look at let me know if it fixes your issue.

Since in your validation the field is not required, and you are not actually passing the field (you are passing an array instead) the validation should pass, the only issue was it's keeping the passed array field you are going to use in the post array, which I have modified in the reset_data method.

[Stanzi1791]

@Stanzi1791 I have submitted a pull request for this, Please have a look at let me know if it fixes your issue.

Since in your validation the field is not required, and you are not actually passing the field (you are passing an array instead) the validation should pass, the only issue was it's keeping the passed array field you are going to use in the post array, which I have modified in the reset_data method.

Yes, this fixed the issue, thanks!
Will close this thread. :)

[narfbg]

I've merged #5659, but please don't close issues until they are truly fixed in the source code. PRs can be rejected ...