Jump to content

rewrite MySql update query


znaji

Recommended Posts

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. 

Link to comment
Share on other sites

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 by maxxd
  • Like 1
Link to comment
Share on other sites

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 by ginerjm
Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.