$POST values not updating database? bad MySQL query?

Question

I am currently writing an application where i have a page called devices.php which queries all the records in the mysql database and displays them in an html table. I have a check box in each row of the table, which the user can check, and then click the "edit" or submit button to edit that record. When the submit button is clicked, it takes you to another page, edit_device.php, where you can then edit that device.. i $_GET the value of the checkbox clicked, which is the "primary key" of that record, which i then query the database to enter the values of that query as the default values for another html form.. the goal here is to pull that device into a form, and if they need to change all the column values of that record, they can, or if they need to just edit one column value, they can just erase the default value, and enter in the new one... i am trying to $_GET the values of that form, and update that record within the database.. right now, when i click edit on the devices.php, it takes me to edit_devices.php, and pulls that record form the database and enters them into the fields of the form... the problem i am having though, is when you change one of the values in the form text box/field, it isnt updated in the database. its still just the original values. heres my code:

<!doctype html>
 <html>

    <?php
      include 'dbcon.config';

      $id=$GET_['check_box'];
      $result=mysql_query("SELECT * FROM device_entries WHERE Id=$id");
      $record=mysql_fetch_row($result);

      $ident    = $record[0];
      $suspend  = $record[1];
      $device   = $record[2];
      $ipadd    = $record[3];
      $dep      = $record[4];
      $emailadd= $record[5];
   ?>

    <body>
      <form method="get">
            ID:         <input type="text" value="<?php echo $ident; ?>" name="id">
            Suspended:  <input type="text" value="<?php echo $suspend; ?>" name="sus">
            Device :    <input type="text value="<?php echo $device; ?>" name="device">
            IP :        <input type="text" value="<?php echo $ipadd; ?>" name="ip">
            Dependency: <input type="text" value="<?php echo $dep; ?>" name="depend">
            Email:      <input type="text" value="<?php echo $emailadd; ?>" name="email">

            <input type="submit" value="update" name="submit1">
      </form> 
   </body>
 </html>

<?php
   if(isset($_GET['submit1'])) {
      $id=$_GET['id'];
      $sus=$_GET['sus'];
      $dev=$_GET['device'];
      $ip=$_GET['ip'];
      $depend=$_GET['depend'];
      $email=$_GET['email'];

      $update=mysql_query("UPDATE device_entries SET Suspended=$sus, Device=$dev, IP=$ip, Depend=$depend, Email=$email WHERE Id=$id")
   }
   mysql_close($con);
?>

The top part seems to be doing what i want it to... it seems like it is something with the $update query.

Welcome to Stack Overflow! [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://www.brightmeup.info/article.php?a_id=2). — NullPoiиteя, Apr 23 '13 at 03:23
ok boy see this http://bobby-tables.com/ ... your code is widely/completely open for sql injection — NullPoiиteя, Apr 23 '13 at 03:24
this script wont leave the building, just for tech support guys within the network... and im just trying to get the functionality down before i worry about anything else — nacy, Apr 23 '13 at 03:31
Even if it is internal only (which is no excuse for using deprecated and dangerous code) - you really should do some basic validation and verification of incoming data. If you're expecting an integer ID, for example, then at least verify it with [ctype_digit()](http://www.php.net/manual/en/function.ctype-digit.php), for example. — HorusKol, Apr 23 '13 at 03:38

score 2 · Answer 1 · edited Apr 23 '13 at 03:59

2

Try change query to this:

"UPDATE `device_entries` SET `Suspended`='$sus', `Device`='$dev', `IP`='$ip', `Depend`='$depend', `Email`='$email' WHERE Id=$id"

There was a comma , before WHERE;
You might be needed to escape column names with backqoutes / backticks;
Your string values should be in single quotes '.

UPD:

1 > I suggest you to check your variables with:

var_dump($id, $sus, $dev, $ip, $depend, $email);

Before query execution.

2 > Place:

if(mysql_errno())exit(mysql_error());

after query execution to determine problems by yourself. It's a debug measure which should help. You might remove them in production / deloyment version.

P.S.: php_mysql is deprecated. I suggest you to use php_pdo or php_mysqli instead. And yes, please note that NullPointer said. Your code is vulnurable to SQL Injection.

edited Apr 23 '13 at 03:59

NullPoiиteя

56,591
22
125
143

answered Apr 23 '13 at 03:12

BlitZ

12,038
3
49
68

i have tried... $update=mysql_query("UPDATE device_entries SET Suspended='".$sus."', Device='".$dev."', and so on... wouldnt that produce the same result? – nacy Apr 23 '13 at 03:18
**@nacy**, Would. There is something else. – BlitZ Apr 23 '13 at 03:21
i originally had all mysqli for the database things, but the VP of operations told me not to use it.. idk, something about the way they do things.. im just an intern, and a newbie, dont fully understand everything – nacy Apr 23 '13 at 03:27
Don't trust `VP` if he / she told you to use `mysql` instead of `mysqli`. He / She is deprecated))) – BlitZ Apr 23 '13 at 03:30
@nacy check this http://stackoverflow.com/a/14110189/1723893 i have described why not use mysql_* function ... however i think mysql_* are as safe as pdo or mysqli but the thing is that they are deprecated from php >=5.5 so now weather they are safe or not we can not use them in php future version .... so switching pdo would be better over mysqli – NullPoiиteя Apr 23 '13 at 03:42
Why you no sql injection warning sir ? – NullPoiиteя Apr 23 '13 at 03:44
**@NullPointer**, because I've marked your message as **useful**. – BlitZ Apr 23 '13 at 03:53
1

please do not bold test if you address other user .. since user wont get notify.. thankyou – NullPoiиteя Apr 23 '13 at 03:58
thank you, the ticks before the column names did it.. i really appreciate it... how do i mark this question as solved? – nacy Apr 23 '13 at 17:09
@nacy press `check` / `tick` near the answer, that you find correct, to accept it. – BlitZ Apr 24 '13 at 02:46

score 1 · Answer 2 · edited Apr 23 '13 at 03:21

1

Your query i wrong, you are placing comma before where clause that is invalid. Change it to this

"UPDATE device_entries SET Suspended=$sus, Device=$dev, IP=$ip, Depend=$depend, Email=$email  WHERE Id='".$id."'";

Secondly it is not executing because you are trying to assign a non html existing field called 'sus' to variable $suspend which result in failing of update query

edited Apr 23 '13 at 03:21

NullPoiиteя

56,591
22
125
143

answered Apr 23 '13 at 03:10

chandresh_cool

11,753
3
30
45

im sorry, that isnt there in the real code, i accidentally put that there while typing it here – nacy Apr 23 '13 at 03:13
then can u edit your question. It looked like an obvious syntax error – chandresh_cool Apr 23 '13 at 03:13
i have even tried concatenating those variables into the query, and it still didnt work – nacy Apr 23 '13 at 03:15
I see one more problem you are getting $suspend = $_GET['sus'] where is sus field in html? – chandresh_cool Apr 23 '13 at 03:17
2

@nacy why dont you check that other field are set too .... and also your code is vulnerable to sql injection you need to escape all request .. – NullPoiиteя Apr 23 '13 at 03:18

score 0 · Answer 3 · answered Apr 23 '13 at 03:14

0

Change as

$update=mysql_query("UPDATE device_entries SET Suspended='$sus', Device='$dev', IP='$ip', Depend='$depend', Email='$email' WHERE Id=$id");

answered Apr 23 '13 at 03:14

Tamil Selvan C

19,913
12
49
70

score 0 · Answer 4 · answered Apr 23 '13 at 17:12

$update=mysql_query("UPDATE device_entries SET Suspended='$sus', Device='$dev', IP='$ip', Depend='$depend', Email='$email' WHERE Id=$id");

--Just adding the ticks before the column names solved it.. and the single quotes, which i had tried by themselves, with no luck

$POST values not updating database? bad MySQL query?

4 Answers4