10

I'm having an error message when inserting content which contains quotes into my db. here's what I tried trying to escape the quotes but didn't work:

$con = mysql_connect("localhost","xxxx","xxxxx");
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

mysql_select_db("test", $con);

$nowdate = date('d-m-Y')

$title =  sprintf($_POST[title], mysql_real_escape_string($_POST[title]));

$body = sprintf($_POST[body], mysql_real_escape_string($_POST[body]));

$sql="INSERT INTO articles (title, body, date) VALUES ('$title','$body','$nowdate'),";

if (!mysql_query($sql,$con))
  {
  
die('Error: ' . mysql_error());
  
}

header('Location: index.php');
Improve this question
1
  • 2
    Please show the error message, you seem to be already escaping the data correctly. Commented Apr 7, 2010 at 12:07

4 Answers 4

Reset to default
14

Please start using prepared parameterized statements. They remove the need for any SQL escaping woes and close the SQL injection loophole that string-concatenated SQL statements leave open. Plus they are much more pleasing to work with and much faster when used in a loop.

$con  = new mysqli("localhost", "u", "p", "test");
if (mysqli_connect_errno()) die(mysqli_connect_error());

$sql  = "INSERT INTO articles (title, body, date) VALUES (?, ?, NOW())";
$stmt = $con->prepare($sql);
$ok   = $stmt->bind_param("ss", $_POST[title], $_POST[body]);

if ($ok && $stmt->execute())
  header('Location: index.php');
else
  die('Error: '.$con->error);
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

12

it should work without the sprintf stuff

$title = mysql_real_escape_string($_POST[title]);
$body = mysql_real_escape_string($_POST[body]);
Improve this answer

6 Comments

@Mauro: Still you should not use this, but parameterized statements instead.
@Tomalak not "should" but "recommended". Prepared statements are more fool-proof, yes, but still not a silver bullet.
@Col. Shrapnel: I thought "should" and "recommended" was the same thing. I would have used some form of "have to" or "must" otherwise. Sorry, I'm not into watering down language constructs purely for the sake of politeness. ;)
Whilst parameterised statements are definitely a better approach in general, unfortunately PHP's mysqli_bind_param implementation of them is a bit verbose, and has a disastrous interface trap for the unwary in that it binds by variable reference instead of value. This often makes it a more difficult sell than escaping. (PDO is a bit better on this front.)
@bobince: Binding by reference should not be a problem as long as binding and execution are done in close succession. Apart from that it is bad style to re-use dedicated variables for something else anyway. ;) For my part, verbosity+security beats brevity. Code is more often read than written, so being verbose is actually a good thing.
|
2

With any database query, especially inserts from a web based application, you should really be using parameters. See here for PHP help on how to use parameters in your queries: PHP parameters

This will help to prevent SQL injection attacks as well as prevent you from having to escape characters.

Improve this answer

1 Comment

Thanks I'll have a look at that! :)
2

Your code 

$sql="INSERT INTO articles (title, body, date) VALUES ('$title','$body','$nowdate'),";

should be as follows

$sql="INSERT INTO articles (title, body, date) VALUES ('$title','$body','$nowdate')";

comma should not be there at the end of query

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.