rewrite MySql update query

[znaji]

Hi 

 

Would like to rewrite this patch of MySQL update query better can someone please help. 

$sqlStatment = "UPDATE products SET "
			. "catid=" . $_REQUEST['cat']
			. ", cattxt='" . $catText
			. "', ptitle='" . $_REQUEST['ptitle']
			. "', saleoffer='" . $_REQUEST['saleoffer']
			. "', url='" . $_REQUEST['url']
			. "', tag=" . $_REQUEST['tag']
			. "', oldprice=" . $_REQUEST['oprice']
			. ", nprice=" . $_REQUEST['nprice']
			. ", availability=" . $_REQUEST['availability']
			. ", shortdesc = :shortdescription"
			. ", pdescription = :description"
			. ", meta_title =" . $_REQUEST['meta_title']
			. ", meta_keyword =" . $_REQUEST['meta_keyword']
			. "', meta_description=" . $_REQUEST['meta_description']
			. ", last_modified = NOW()"
			. " WHERE id=" . $_REQUEST['pid'];

	try{
		$query = $db->prepare($sqlStatment);
		$query->execute(array(':shortdescription'=>$_REQUEST['shortdescription'], ':description'=>$_REQUEST['description']));
		}
		catch(PDOException $e) {
		    die($e->getMessage());
		}

Thank you for any help. 

[Barand]

Use placeholders for all the variables and not just two of them.

[maxxd]

Barand is right - use your prepared statement correctly. Right now you're wide open to SQL injection attacks and your single quotes are irregular and incorrect in several spots. A prepared statement will take care of those issues for you.

 

Also, I'd recommend against using $_REQUEST. You should know where your data is coming from, and look only there for it - $_GET, $_POST, or $_SESSION. $_REQUEST is a catchall superglobal and can open you up to more security issues.

Edited January 25, 2016 by maxxd

[ginerjm]

I agree strongly with Maxxd on the use of $_REQUEST. You KNOW whether it is a post or get so USE that array name.

 

I also agree with the comments on using a fully prepared query, ie, use parms for all of the query's values, not just for a couple of them. This would also help alleviate the injection possibility that you are creating by using un-sanitized input values.

 

Lastly you could read up on how to use single and double quotes and braces for your query string. Consider this:

sqlStatment = "UPDATE products SET catid={$_REQUEST['cat']},
cattxt='$catText', ptitle='{$_REQUEST['ptitle']}',
saleoffer='{$_REQUEST['saleoffer']}',
...
...
WHERE id='{$_REQUEST['pid']}'";

A much simpler statement to construct and to read. Of course when you SWITCH to a properly prepared statement it will be even easier:

sqlStatment = "UPDATE products SET catid=:cat,
cattxt=:cattext, ptitle=:ptitle,
saleoffer=:saleoffer:'
...
...
WHERE id=:pid";

Edited January 25, 2016 by ginerjm