0

i am trying to update mysql table with php grabbing data from a form.

When i run the script with this

$query="UPDATE customers SET background='$_GET['background']',font='$_GET['font']',fontcolour='$_GET['fontcolour']',fontsize='$_GET['fontsize']',title='$_GET['title']' WHERE client='$_GET['client']'";

i get a black screen but if i do the below script

$background=$_GET['background'];
$client=$_GET['client'];
$query="UPDATE customers SET background='$background' WHERE client='$client'";

it goes through fine

although i can do it this way i am trying to understand why i can not just put the get right into the query rather than having more lines of code than necessary.

NOTE: all other code stripped can post if required.

Andrew Crawford
  • 33
  • 9
  • 2
    Single quote vs double quote explained here - http://stackoverflow.com/questions/3446216/what-is-the-difference-between-single-quoted-and-double-quoted-strings-in-php – rlatief Jul 28 '14 at 11:29
  • 1
    In the first query, where you are putting `$_GET[..]`, surround them with `{` and `}` like `...background='{$_GET['background']}...` – Jonathon Jul 28 '14 at 11:31
  • Thank you for that input had confused me why it was not working – Andrew Crawford Jul 28 '14 at 11:40
  • 1
    You need to make sure the parameters you want to use in your query does not contain anything harmful. If you put the get into the query directly you are prone to SQL Injection. For more info on SQL Injection have a look here http://www.unixwiz.net/techtips/sql-injection.html and for more info on preventing SQL Injection in PHP have a looking here http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Willie Nel Jul 28 '14 at 11:40

2 Answers2

1

Change your code to this

$query="UPDATE customers SET background='" . $_GET['background'] . "',font='" . $_GET['font'] . "',fontcolour='" . $_GET['fontcolour'] . "',fontsize='" . $_GET['fontsize'] . "',title='" . $_GET['title'] . "' WHERE client='" . $_GET['client']."'";

Also, your code is vulnerable to SQL injection

RajivRisi
  • 398
  • 1
  • 12
Lemuel Botha
  • 659
  • 8
  • 22
  • it would be if it was exposed to anyone but the page it is on only the admins have access to it, so it be pretty obvious who done it unless the site was hacked which if it was sql injection would be the least of our worries, and it will be on internal network closed to internet access, or i would have put in mysql real escape quotes code – Andrew Crawford Jul 28 '14 at 11:39
  • @AndrewCrawford "it would be if it was exposed to anyone" You can **never** take security for granted, and this is still poor security design. – Terry Jul 28 '14 at 11:43
  • @AndrewCrawford Famous last words! So often you see code copy-pasted from a secure environment to an insecure one without a thought! [I know because I've done it! :-( ] – Strawberry Jul 28 '14 at 11:45
  • i never downvoted the answer, fair point about the security never thought about it like that, i can't vote up or down the answer due to my reputation – Andrew Crawford Jul 28 '14 at 11:47
  • to prevent injection you should use mysql_real_escape_string() on every variables that you get from user. Eg: `background='" . mysql_real_escape_string( $_GET['background'] ) . "'` . But this is old, you should now use PDO instead: http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers -- and to your original question, your code doesn't work because you seem to don't understand the difference between single and double quotes (reference posted on my comment in your question). – rlatief Jul 28 '14 at 12:02
-1
$query="UPDATE customers SET background='$_GET[background]',font='$_GET[font]',fontcolour='$_GET[fontcolour]',fontsize='$_GET[fontsize]',title='$_GET[title]' WHERE client=$_GET[client]'";

When we use double quotes we haven't to write array index in single quotes

SQL Injection alert!

turson
  • 421
  • 2
  • 9