MySQL does not update correctly

Question

I seem to have a little trouble with some basic mysql commands. This is the table called item that I created:

mysql> describe item;
+------------+------------------+------+-----+---------+----------------+
| Field      | Type             | Null | Key | Default | Extra          |
+------------+------------------+------+-----+---------+----------------+
| item_id    | int(10) unsigned | NO   | PRI | NULL    | auto_increment |
| item_name  | varchar(255)     | NO   |     | NULL    |                |
| item_price | int(10) unsigned | NO   |     | NULL    |                |
+------------+------------------+------+-----+---------+----------------+
3 rows in set (0.00 sec)

and this is the line of code i'm using to update this table:

$db = mysql_connect('127.0.0.1', 'root', 'mac');
mysql_select_db('spending') or die(mysql_error($db));

$query = 'UPDATE item SET item_name = "' . $_POST['item_name'] . '",
                item_price = ' . $_POST['item_price'] . '  
                WHERE item_id = ' . $_GET['id'] . '';

$result = mysql_query($query, $db) or die(mysql_error($db));

but every time I run it, it always complains like this:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE item_id = 1' at line 4

Can anybody tell me what is wrong with this thing? Thanks in advance!

replace mysql with mysqli – Vamshi .goli Jan 16 '15 at 06:10 — Vamshi .goli, Jan 16 '15 at 06:10

score 0 · Answer 1 · edited May 23 '17 at 12:05

The problem is that you have not parameterized your query to prevent MySQL injection. Therefore, the $_POST and $_GET variables can contain anything, including malicious code.

In this case, you're lucky. One of the variables is probably causing a syntax error. A hacker could easily manipulate any of your variables to DROP you entire table, or read sensitive data from other tables.

In addition, php's original MySQL extension is now deprecated. Make sure you use MySQLi or PDO_MySQL instead of php's MySQL extension.

Mysteryos · Answer 2 · 2015-01-16T05:36:04.770

Stop using mysql functions, they are deprecated

Now that we got that out of the way:

//Connect to db
$db = mysqli_connect('127.0.0.1', 'root', 'mac','spending');
if ($db->connect_errno) {
    echo "Failed to connect to MySQL: (" . $db->connect_errno . ") " . $db->connect_error;
}

//Prepare our query
$query = $db->prepare('UPDATE item SET item_name = ?,
                item_price = ?  
                WHERE item_id = ?');

//Bind the parameters
$item_name = (string)$_POST['item_name'];
$item_price = (int)$_POST['item_price'];
$item_id = (int)$_POST['id'];

$query->bind_param('sii',$item_name,$item_price,$item_id);

//Execute
$query->execute();

Read more about mysqli: http://codular.com/php-mysqli

yeah i tried it this is what it says: Warning: mysqli_stmt::bind_param(): Number of variables doesn't match number of parameters in prepared statement in /Library/WebServer/Documents/doc/dailyexpense/edit.php on line 37 — Drexel, Jan 16 '15 at 05:23

score -1 · Answer 3 · edited Jan 16 '15 at 05:13

-1

use this

 $query = 'UPDATE item SET `item_name` = "' . $_POST['item_name'] . '",
                    `item_price` = ' . $_POST['item_price'] . '  
                    WHERE `item_id` = '.$_GET['id'];

edited Jan 16 '15 at 05:13

Andy

49,085
60
166
233

answered Jan 16 '15 at 05:04

parveen

557
3
13

MySQL does not update correctly

3 Answers3