0

As you can see, I have a query I want to insert variables in. What's wrong with my syntax?

$query = "UPDATE house SET epname=".$newtitle" WHERE epid= ".$epid;
Improve this question
4
  • 2
    newbie common mistake, you need to quote the string like "UPDATE house SET epname='".$newtitle."' WHERE epid= ".(int)$epid;, and of course you need to escape it Commented Dec 26, 2010 at 22:52
  • @ajreal-Thanks! And again you come to the rescue:) Commented Dec 26, 2010 at 23:02
  • Did you understand at last that there was 2 syntax errors - PHP one and mysql one? Commented Dec 26, 2010 at 23:41
  • Not quite-I got the php one, what is the mysql one? Commented Dec 27, 2010 at 11:19

3 Answers 3

Reset to default
3

The basic syntax error is:

…e=".$newtitle" W…
             ^^

If you were going to go down the route of bashing strings together to make SQL statements, then you should make use of the fact that double quotes interpolate. This results in much more readable code.

$query = "UPDATE house SET epname=$newtitle WHERE epid=$epid";

But the approach of string bashing is flawed . Use prepared statements (preferably with PDO), they are harder to create SQL injection vulnerabilities with and (arguably) easier to read.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Hi David-thanks, will make sure to do that. Can you give a brief explanation of the difference between PDO and PEAR->DB?
2

Are your variables strings? You will want to enclose them in quotes for the purpose of the MySQL query.

Also, you're missing a concatenation operator (period) after $newtitle.

If you echo out the value of $query, you should see the error:

UPDATE house SET epname=[value of newtitle] WHERE epid= [value of $epid]

Assuming that epname is a char/varchar value, and epid is an integer of some sort, you probably want to do this:

$query = "UPDATE house SET epname = '" . mysql_real_escape_string($newtitle) . "' WHERE epid= " . $epid;

If you do not use the mysql_escape_string function around your strings, you are vulnerable to SQL injection attacks

Improve this answer

4 Comments

Thanks TehShrike! Works like a charm.
LOL! You are talking of injections and still leave $epid naked!
@Col. Shrapnel: Yes, I mentioned in there that I was making the assumption that $epid was an integer (in code). Personally, I would enclose it in an intval() call to be sure.
Don't use mysql_escape_string, it's deprecated. Use mysql_real_escape_string instead, or better yet, PDO.
0 
$query = "UPDATE house SET epname=".$newtitle" WHERE epid= ".$epid;

should be

$query = "UPDATE house SET epname=".$newtitle." WHERE epid= ".$epid;

or better

$query = "UPDATE house SET epname= $newtitle WHERE epid= $epid";
Improve this answer

26 Comments

Without a warning about injection or using escaping or prepared statements, this approach should not be an answer on Stackoverflow.
He asked what is wrong with his code, not how he can make it more secure. Maybe i should also talk about mysql injection or even xss for you to consider this correct, but the fact is that my answer is the exact reply for such a question.
@SpyrosP-thanks! @Konerak-I understand the importance of securing my code, but foe the sake of what I'm doing now-learning, and not developing, this is more than enough. But I will take into consideration to learn about security before getting into some serious dev.
No, he will experience another error with your code, mysql one. And, to let you know, escaping has nothing to do with injections. It's part of the syntax. Obligatory part.
@Col. Shrapnel-I understand that, but don't you agree that for the sake of learning one must be able to first write any code, no matter how bad, in order to write good code afterward? I'm merely a beginner here.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.