0

I need a bit of help, I have this UPDATE query that isn't updating the records in my database. Maybe it's a small error that I'm missing, something I might have left out tha ta fresh pairs of eyes might pick up easier.

<?php
 $message = '<h4 class="alert_success">A Successfully updated '.$_POST['sell_itemBrand'].' '.$_POST['txtModel'].' - '.$_POST['sell_itemType'].'</h4>';
$color = "";
($_POST['colorSel'] =='old' ? $color = $_POST['color'] : $color = $_POST['newColor']);
mysql_query("UPDATE `total_stock` SET  `ItemType` =  '".$_POST['sell_itemType']."', `ItemBrand` =  '".$_POST['sell_itemBrand']."', `ItemModel` =  '".$_POST['txtModel']."', `Cost_Price` =  '".$_POST['CostPrice']."', `Color` =  '".$color."' WHERE `IMEI` =  '".$_POST['current_IME']."'")  or die(mysql_error());
?>

what is supposed to happen here is a user can update the various fields in the database from a form. The new values entered are put in the various variables and used for the updating.

Thanks.

Kofi
  • 15
  • 1
  • 6
  • 1
    -0.49 for still using `mysql_query` in 2014. – cHao Jul 30 '14 at 19:07
  • What is the output of mysql_error() or is it quiet? You better move on to mysqli and PDO with parameterized prepared statements. That's much easier to read and less error prone. What about Kacy's as ItemBrand or some such? – VMai Jul 30 '14 at 19:11
  • You're not checking for errors. Add error reporting to the top of your file(s) right after your opening ` – Funk Forty Niner Jul 30 '14 at 19:14
  • Plus, your present code is open to [**SQL injection**](http://stackoverflow.com/q/60174/). Use [**`mysqli_()` with prepared statements**](http://www.php.net/manual/en/mysqli.quickstart.prepared-statements.php), or [**PDO**](http://php.net/pdo) with [**prepared statements**](http://php.net/pdo.prepared-statements). **NEVER** trust user input. – Funk Forty Niner Jul 30 '14 at 19:15
  • Thanks for all the tips and advice guys, I just started learning PHP and MySQL, I obviously have more to learn. – Kofi Jul 31 '14 at 17:36

2 Answers2

0
$_POST['current_IME']

Maybe, you mean this:

$_POST['current_IMEI']
0

you need change somethings in your code to your own safety.

Isn't a good pratice to use your $_POST informations on your querys becouse some user can do a SQL Injection on your code, to prevent this use this function:

mysql_real_escape_string();

Functions mysql_* are deprecated, isn't a good pratice to use this form to acess your database.

Use like this:

To connect:

 $dbh = new PDO('mysql:host=xxx;port=xxx;dbname=xxx', 'xxx', 'xxx', array( PDO::ATTR_PERSISTENT => false));

To execute querys:

$query = 'Any query';
$stmt = $dbh->prepare($query);
$stmt->execute();
while ($result = $stmt->fetch(PDO::FETCH_OBJ)) {
    var_dump($result);
}

So in you case you should do this:

$query = "UPDATE `total_stock` SET  `ItemType` = '?', `ItemBrand` =  '?', `ItemModel` =  '?', `Cost_Price` =  '?', `Color` =  '?' WHERE `IMEI` =  '?'")
$st = $databaseconnection->prepare($query);
$st->execute(array(mysql_real_escape_string($_POST['sell_itemType']),
             mysql_real_escape_string($_POST['sell_itemBrand']),
             mysql_real_escape_string($_POST['txtModel']),
             mysql_real_escape_string($_POST['CostPrice']),
             $color,
             mysql_real_escape_string($_POST['current_IME'])));

This is the correct form to use database with php.

Hope help.

Guerra
  • 2,792
  • 1
  • 22
  • 32