0

At the moment, I am submitting two 'WS_ID' numbers in a text box with a space in between them. The SQL command that I have is able to update two results in the sql database but when there is only one 'WS_ID' submitted in the text box, nothing is updated. any help would be briliant. Many thanks.

         if(isset($_GET["Update"]))
        {   $DEP_TIME = $_GET["DEP_TIME"];
            $ARR_TIME = $_GET["ARR_TIME"];
            $TRAVEL_TIME = $_GET["TRAVEL_TIME"];
            $HRS_WORKED = $_GET["HRS_WORKED"];
            $INC_DEP_TIME = $_GET["INC_DEP_TIME"];
            $INC_LAB_DAY = $_GET["INC_LAB_DAY"];
            $LAB_DATE = $_GET["LAB_DATE"];
            $WS_ID = $_GET["WS_ID"];
            $WS_ID2 = explode(" ", $WS_ID);
            $SQL = "UPDATE `elecsys`.`worksheet_labour` SET DEP_TIME = $DEP_TIME, ARR_TIME = $ARR_TIME, TRAVEL_TIME = $TRAVEL_TIME, HRS_WORKED = $HRS_WORKED WHERE WS_ID = $WS_ID2[0] OR WS_ID = $WS_ID2[1]"; 
            $resultset2 = mysql_query($SQL);                
        }
user1060187
  • 957
  • 5
  • 12
  • 28
  • 5
    Repeat after me: [SQL injection](http://en.wikipedia.org/wiki/SQL_injection) is a bad thing – PeeHaa Aug 02 '12 at 13:17
  • This is an internal thing so SQL injection isn't a priority – user1060187 Aug 02 '12 at 13:20
  • even so, php isn't updating `mysql_query` anymore, need to think about long term – lusketeer Aug 02 '12 at 13:21
  • @user1060187 that really isn't an excuse. Besides if this is new code I would suggest to stop using `mysql_*` functions. They are no longer maintained and the community has begun the [deprecation process](http://goo.gl/KJveJ). See the [**red box**](http://goo.gl/GPmFd)? Instead you should learn about [prepared statements](http://goo.gl/vn8zQ) and use either [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli). If you can't decide, [this article](http://goo.gl/3gqF9) will help to choose. If you care to learn, [here is a good PDO tutorial](http://goo.gl/vFWnC). – PeeHaa Aug 02 '12 at 13:25
  • http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php – PeeHaa Aug 02 '12 at 13:25

1 Answers1

2

In your sql you probably need quotes around the value as it currently is. However, at this current time you are subject to SQL injection. You should use PDO or MYSQLI to avoid this.

When $WS_ID2[1] is blank you get WHERE WS_ID=value OR WS_ID = ; I don't believe that MYSQL knows that is suppose to be blank without quotes.

Code using MYSQLI, to prevent SQL injection and your code failing after the deprecated code as been removed from PHP:

$con = new mysqli($databasehost,$dbuser,$dbpassword);
if ($con->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " .  $mysqli->connect_error;
}
//Set all your variables here, omitted from code...
$stmt = $con->prepare("UPDATE `elecsys`.`worksheet_labour` SET DEP_TIME = ?, ARR_TIME = ?, TRAVEL_TIME = ?, HOURS_WORKED = ?, WHERE WS_ID = ? OR WS_ID = ?");
//bind using data according to http://www.php.net/manual/en/mysqli-stmt.bind-param.php
$stmt->bind_param('ssddii', $DEP_TIME, $ARR_TIME, $TRAVEL_TIME, $HRS_WORKED, $WS_ID2[0], $WS_ID2[1]);
$stmt->execute();
$stmt->close();
$con->close();

Also, my other thoughts is that if you only have one value for $WS_ID2, does PHP not give an out of bounds error for $WS_ID[1]?

Community
  • 1
  • 1
Travis Pessetto
  • 3,260
  • 4
  • 27
  • 55
  • No, the quotes aren't required here. I am able to update two WS_ID's but not 1 so that can't be the issue – user1060187 Aug 02 '12 at 13:19
  • When you have two there is no one with a blank one. When you have one It looks like you are trying to assigning it to the other thing assigned. So you need quotes to show that that value is indeed blank. – Travis Pessetto Aug 02 '12 at 13:21
  • Where should the quotes be around? – user1060187 Aug 02 '12 at 13:23
  • around ``` $WS_ID2[1]``` but just a second for me to research mysqli and I will give you a better solution resistant to sql injection. – Travis Pessetto Aug 02 '12 at 13:25
  • I put the quotes around the $WS_ID[1] but still not change – user1060187 Aug 02 '12 at 13:27
  • @user1060187 you're welcome. Also, if you want sql injection safe code. See my updated answer. Though I did make some assumtions as to the data types so you may need to change 'ssddii' depending on if the data types are strings, decimals or integers. – Travis Pessetto Aug 02 '12 at 13:46