Update Database Not Working

Hi all, I have a small script that should process the data from a previous page and enter it into the database and then redirect the user. It is redirecting the user but not updating the information in the database. Any thoughts or ideas would be appreciated as always! :slight_smile:

<?php 
//Controls Edit section submit button!
if(isset($_POST['subit'])){
	$blip = preg_replace('/[^0-9\\.]/', '', $product['price']); 
		$total = $blip*$product['quantity'];
			$updateData = mysql_query("UPDATE order_products SET product_name='$_POST[prodname]' WHERE id='$_GET[row]'");
				header('Location: ../orderhistory-test.php?section=view&id='.$_GET['id'].'&ref='.$_GET['ref'].'');
}	
?>

Try echoing the query to see how it actually looks like with the variable values.

_POST and _GET at the same time? that doesn’t sound right

Not only is your query prone to SQL injection, you’re not even quoting your associative keys, forcing PHP to assume that undefined constant is actually a string… (to translate, it’s not $_POST[prodname], it’s $_POST[‘prodname’]);
‘Location’ header expects FULL URL, not path, it’s terrible to redirect users like that, some browser might not understand it.

And what Guido said, echo the query with variables and run it within some MySQL GUI so you can see the result.

So it’s recognizing that the form has been submitted, as guido said, print/echo (never usually use echo i must say xD) the query, to see what is being sent.

Although I always encase my variables in double quotes so:

$updateData = mysql_query("UPDATE order_products SET product_name='$_POST[prodname]' WHERE id='$_GET[row]'"); 

To;

$updateData = mysql_query("UPDATE order_products SET product_name='" . $_POST[prodname] . "' WHERE id='" . $_GET[row] . "'"); 

You might also want to change:

$_POST[prodname]

to:

$_POST['prodname']

Note: Single quotes.

And do the same on the $_GET.

why would someone submitting a form (with _POST variables) put the id variable into the query string?

That’s what I thought, but can’t you use get to get any URL variable?

(Apologies for double post) - =[

Yes, $_GET contains the variables from the query string.

I don’t know. I was answering joseph’s question. I have no idea why or if the OP does so.

The OP gives the impression he’s passing one through the query string as well as using _POST from a submitted form.
Which is what confused me.
I may have been a bit too presumptuous :slight_smile:

Your first line:

if(isset($_POST['subit'])){

is ‘subit’ supposed to be ‘submit’?

Instead of guessing whether to use _POST or _GET - you can try _REQUEST which has them both merged. More on that here: http://php.net/manual/en/language.variables.superglobals.php

Hey guys/gals, apologies, I did not realize I had any replies to this post. I’m using $_GET as I dragging up data from the database with the option to update that data. It’s part of a Point of Sale Intranet I’m working on. With regards to reformatting the query string, I will get on that straight away, appreciate the help and advice on here, cheers all :smiley:

Incidentally, if I use double quotes on my query string such as mysql_query(“UPDATE users SET names='”.$_POST[‘name’].“'”); would I need not to use the mysql_real_escape_string?

No. $_REQUEST is evil, for the simple reason that you just don’t know where any of its values are coming from, which makes it very error-prone. It’s slightly less evil than register_globals but it comes pretty close.

User input must always be sanitized before using it in a query.

Hi Guido, I’m using the mysql_real_escape_string - this should suffice shouldn’t it?

In this query, since both user input values are between quotes, yes.

It’s not in the code you posted, so that means that the code you posted isn’t the production code. So it will be impossible to tell you where the bugs in your production code are.

Post the production code.

Hi, this was rectified some time ago. This thread has been resolved.

Not sure I fully agree that “$_REQUEST is evil”. I would classify it as convenient more than evil. As long as all external data is fully scrubbed, I don’t think it matters. I’d say register_globals is certainly evil as it actually creates variables in global scope. Obviously a personal preference issue.