Table not updating unless form is resent

PHP
1

I have a form which is used to update a shopping cart for a site I’m building. I have a quick cart at the top of the page which gets the number of items and the price of the item and times the two together to get a total. Simple enough. In my main cart I have the option to adjust the quantity of the product in the cart, which on submit will refresh the page and update the table and in doing so, re-calculate the cart and quick cart amounts. This works but it’s not updating the quick cart unless I resubmit the form. In other words, I have to enter the amount and click submit and click the submit button again for the quick cart to update.

I’ve looked at the query and the quantity is updated straight away. The price field however is not updating until it’s submitted again. Here’s some code to explain further;

Here’e the query code


<?php
if (@$_POST['go']=='2') {

	$itemPrice = $_POST['cost'];
	$itemID = (int)($_POST['orderLine']);
	$itemQty = (int)($_POST['prodquantity']);

	if(isset($itemID)) {
			$sql = "UPDATE order_items SET quantity = '$itemQty', price = '$itemPrice' WHERE itemID = '$itemID'";
			$perform_insert = mysql_query($sql) or die(mysql_error());	
			unset ($itemID);
			unset ($itemQty);
			unset ($itemPrice);
			header("Location: cart.html");
	}
}
?>

Here is the form. The item price is pulled form another table and submitted as a hidden value.


<?php

$hp = $price;

echo '<form action="" method="post">';
echo '<input type="text" size="1" name="prodquantity" value = "' . $row['quantity'] . '" />';
echo '<input type="image" src="images/refresh.png" border="0" alt="update" height="20px" height="20px" />';
echo '<input type="hidden" name="go" value="2">';
echo '<input type="hidden" name="cost" value="' . $hp . '">';
echo '<input type="hidden" name="orderLine" value="' . $row['itemID'] . '">';
echo '</form>';

?>

What I don’t understand is why it’s not updating the price but will update the quantity on the form submit. The price is entered into a table as a decimal (5,2). Can anyone help?

2

I can tell you that your page is wide open to sql injection attacks.

Also, why are you echoing out html from php? It’s messy - you should break out and write pure html instead of echoing html out of php like that.

Another “also” - why are you suppressing error messages? You shouldn’t suppress errors - they’re there for a reason… So much wrong…

In terms of the price itself - is the price actually coming through into the $itemPrice variable?

It could be because you’re wrapping the variable in ‘’ marks in your sql query - pretty sure you shouldn’t be doing that with a decimal value (been a while since I’ve written pure SQL as I tend to use ORMs these days, but try playing with that).

3

OK, firstly thanks for the reply. Site is in development at the moment and I have security to look at before going live, which is some months away yet. I take it that you’re referring to the lack of mysql_real_escape functions for the post variables. If I’ve missed something regarding security could you advise? I will take a look at your suggestion. Again, thanks for the reply. Greatly appreciated.

4

Here’s revised code. Still needed the form to be resubmitted to update the table.


<?php
if (@$_POST['go']=='2') {

	$itemPrice = mysql_real_escape_string($_POST['cost']);
	$itemID = mysql_real_escape_string((int)($_POST['orderLine']));
	$itemQty = mysql_real_escape_string((int)($_POST['prodquantity']));

	if(isset($itemID)) {
			$sql = mysql_query("UPDATE order_items SET quantity = '$itemQty', price = '$itemPrice' WHERE itemID = '$itemID'") or die (mysql_error());
			unset ($itemID);
			unset ($itemQty);
			unset ($itemPrice);
			header("Location: cart.html");
	}
}
?>



<form action="" method="post">
<input type="text" size="1" name="prodquantity" value = "<php echo $row['quantity'];?>" />
<input type="image" src="images/refresh.png" border="0" alt="update" height="20px" height="20px" />
<input type="hidden" name="go" value="2">
<input type="hidden" name="cost" value="<?php echo $hp; ?>">
<input type="hidden" name="orderLine" value="<?php echo $row['itemID']; ?>">
</form>
5

Hi, no probs :slight_smile:

Ok, specifically, let’s look at this:


	$itemPrice = $_POST['cost'];
	$itemID = (int)($_POST['orderLine']);
	$itemQty = (int)($_POST['prodquantity']);

	if(isset($itemID)) {
			$sql = "UPDATE order_items SET quantity = '$itemQty', price = '$itemPrice' WHERE itemID = '$itemID'";
//code continues...

The problem here is that a malicious user could break this query with potentially nasty consequences for you. $_POST data is just as unsafe as $_GET data, and can be manipulated easily.

If you look at your variables here:


	$itemPrice = $_POST['cost'];
	$itemID = (int)($_POST['orderLine']);
	$itemQty = (int)($_POST['prodquantity']);

We can see for the $itemID and $itemQty variables, you are type casting to integer values [(var)($_POST[‘orderLine’])] - now this is a good thing, because it means that no matter what data comes into the $_POST[‘orderLine’] and $_POST[‘prodquantity’] variables, that data will be converted to an integer. Malicious data won’t get through.

However, you’re not doing this for the $itemPrice variable.

So consider this - imagine I’m a malicious hacker and for the ‘cost’ field on the form, instead of writing a number, I do enter this: “1’; DELETE TABLE USERS;”.

Your $sql query will now hold:

“UPDATE order_items SET quantity = ‘1’; DELETE TABLE USERS; , price = ‘$itemPrice’ WHERE itemID = ‘$itemID’”;

See how this is a problem for you? You’re executing the queries: UPDATE order_items SET quantity = ‘1’; AND DELETE TABLE USERS;

There are a number of ways to get around this, but actually, you should not be using the mysql_ functions at all. Please look into learning something called PDO - it will involve learning some basics about objects, but it will be worth it because so long as you use prepared statements in PDO, sql injection is something that will no longer bother you :slight_smile:

I’d recommend taking some time out from your project and go read and practise through this instead: http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/

With regards to the other stuff I said: please don’t supress errors. Instead, you should actually set your error reporting to the highest level possible while developing. When you deploy your code to your server, you should then configure your error reporting to log to a log file on the server instead of outputting directly on the screen. Pay attention to every error you get, and make sure you have none in your application when you deploy. Your code will be better for it.

6

The html is much better… That’s a start!

In terms of the code still needing to be submitted twice: It looks like it’s the placement of the header() call.

Can you try putting in a die(); statement just before the header() call and then checking your database to see if it’s updated?

Could you also print the $sql statement on the first + second page impression, and see what comes through each time (and whether it’s different at all?)

7

aaarrrggh, your name certainly fits my thoughts on security in web design. Thanks for the suggestion regarding security and the link, I will certainly look into it. Im just stumped as to why the value of ‘cost’ isn’t updating.

8

Try outputting the value of $sql each time - see if it’s different on the first impression to the second?

9

OK, will try that. Furthermore I have followed the guide on setting error reporting and my local server(WAMP) is set to show eveything. Regarding PDO, can you use it with standard proceedure PHP coding or is it intended for OOP, which I am going to eventually migrate to? I really appreciate the help you’ve given me. The beers’ on me!!

10

It’s OOP only. The syntax is all explained in that tutorial though. I’d suggest going through that and writing some basic code using PDO to get your head around it, and then start trying to use it in your own projects :slight_smile:

11

Cheers