Database bug for form_submit?

[eluser]Unknown[/eluser]
If you try to use the form_submit helper function and then insert the data into a database from the $_POST data, the Database class will try to insert data into a column with the same name as the submit input button, which most likely will not exist.

For example, suppose you want to add a blog comment to the blog_comments table.

Suppose that in your comment_add.php view your have:

echo form_submit('submit', 'Add Comment');

and in your comment_add() function in your blog controller, you have

$this->db->insert('blog_comments', $_POST)

and that you do not have a column named 'submit' in your blog_comments table of your database,

then you will get an insert error that says it cannot find the column named 'submit'

To fix this, we may want to have the insert function of the Database class check the type of the input, and if it is of type submit or reset, then it should not treat it as a column name.

This seems to be a bug for the Database class. Am I correct?

[eluser]Colin Williams[/eluser]
$this->db->insert(’blog_comments’, $_POST) is going to send every post value to your table. Since 'submit' is a posted value, the insert() method tries to send it. Construct the array to match columns in your table before sending it on. Also, you probably want to validate the values anyway. Even if you use the validation class, $_POST is still unlaundered.

[eluser]Michael Wales[/eluser]

$_POST is still unlaundered.

Not quite true - after validation, the Validation and Input classes, and $_POST will all return the exact same value.

But - you are absolutely correct, define what is being inserted explicitly.

Here's why:

Let's say you have the following database table:
- username varchar(20)
- password varchar(120)
- email varchar(120)
- salt varchar(9)
- admin enum('Y', 'N')

I am filling out your registration form (which is username, password, and email) and I decide to get a bit sneaky with it. I launch Firebug, add a hidden field named 'admin' with a value of 'Y' and submit it - just to see what happens.

Your code:

Code:

$this->db->insert('users', $_POST);

Just made me an administrator.

This code - will make sure the only thing headed into your database is what you expect:

Code:

// Of course, you would never insert a plain-text password, but for the sake of brevity...
$insert = array('username' => $this->input->post('username'),
'password' => $this->input->post('password'),
'email' => $this->input->post('email'));
$this->input->post('users', $insert);

[eluser]Colin Williams[/eluser]

Not quite true - after validation, the Validation and Input classes, and $_POST will all return the exact same value.

Doh! Good to know! That's where assuming gets me, I guess...

[eluser]Unknown[/eluser]
thank you for your answers. it looks like the answer is that no one should ever submit a $_POST directly (without validation). perhaps they should remake the video tutorial which suggests otherwise.

also, looking at the documentation (http://ellislab.com/codeigniter/user-gui...input.html), I do not see a way to do $this->input->post('users', $insert). According to the documentation, the second parameter indicates whether or not to clean XSS.

also, rebuilding the array and cleaning each element seems like a lot of redundant code. is there a single function call that can do all of this, perhaps supplying an explicit list of column names?

[eluser]Colin Williams[/eluser]

perhaps they should remake the video tutorial which suggests otherwise

I believe the video does explain they are doing it for brevity and it's not a good idea for true applications.

also, rebuilding the array and cleaning each element seems like a lot of redundant code

Well, if you haven't even done it once yet, how can you say it's redundant? You should only need to validate input once per form, say, once for handling a blog post, once for handling a comment post, etc. I don't see how this would become redundant.

Also, the $this->input->post() method retrieves posted data; it has nothing to do with your database. I think Michael just slightly goofed when posting that. Should be $this->db->insert('users', $insert).