Page 1 of 1

mysql_real_escape_string

Posted: Fri Jan 25, 2008 5:49 am
by rsmarsha
I've been looking at mysql_real_escape_string in the php manual.

Until now i've been using it in the form:

Code: Select all

$query = "INSERT INTO table (name) VALUES ('".mysql_real_escape_string($name)."')";
or pre escaping before the variable is used in the query as below:

Code: Select all

$name = mysql_real_escape_string($name);
After reading the manual I can see they do it a different way, a query i wrote using that method is below:

Code: Select all

$cat_insert = sprintf("INSERT INTO icecat_categories (category_id,category_name,uncatid,searchable,thumbpic,score,lowpic,parent_category_id,parent_category_name) VALUES ('%d','%s','%d','%d','%s','%d','%s','%d','%s')",$category->tagAttrs['id'],mysql_real_escape_string($category_name),$category->tagAttrs['uncatid'],$category->tagAttrs['searchable'],mysql_real_escape_string($category->tagAttrs['thumbpic']),$category->tagAttrs['score'],mysql_real_escape_string($category->tagAttrs['lowpic']),$category->parentcategory[0]->tagAttrs['id'],mysql_real_escape_string($parent_name));
$catQ = mysql_query($cat_insert) or die ("Query Cat Insert: $cat_insert Failed".mysql_error());
Which way is the best, or does it not really matter?

Re: mysql_real_escape_string

Posted: Fri Jan 25, 2008 12:22 pm
by mwasif
All are equal but the 2nd method keeps your code clean specially your MySQL queries. But at the same time you need to take care when comparing these variables (after using mysql_real_escape_string()).

Re: mysql_real_escape_string

Posted: Fri Jan 25, 2008 5:15 pm
by Mordred
I would advise to use what you call "pre-escaping", you would notice that this is the best way to keep your code readable. printf() is an Abomination, I can't understand what makes mwasif think that the resulting code is clean.

Compare with:

Code: Select all

$name = mysql_real_escape_string($name);
$query = "INSERT INTO table (name) VALUES ('$name')";
or - and this is the best coding style in terms of readability (which also means security):

Code: Select all

$name = mysql_real_escape_string($name);
$query = "INSERT INTO `table` SET `name`='$name'";

Re: mysql_real_escape_string

Posted: Fri Jan 25, 2008 7:30 pm
by VladSun
sprintf() would correct the data types. I.e.:

Code: Select all

 
sprintf("%d", '1234')  --> 1234
sprintf("%d", '1234abcd')  --> 1234
sprintf("%d", 'abcd1234')  --> 0
 

Re: mysql_real_escape_string

Posted: Sat Jan 26, 2008 1:30 am
by Mordred
Data validation belongs to a subsystem totally different than the database layer.

One should do something like this (preferably hidden behind utility functions or request handler of course):

Code: Select all

 
//at the beginning: type check
$age = intval( (isset($_GET['age'])) ? $_GET['age'] : 0 );
//additional (busyness logic) constraints on the data:
$age = max(0, min($age, 100)); //put between 0 and 100
 
// ... much much later, when we decide we're putting that in the database:
$age = mysql_real_escape_string($age);
$query = "INSERT INTO `table` SET `age`='$age'";

Re: mysql_real_escape_string

Posted: Sat Jan 26, 2008 1:52 am
by Chris Corbyn
I'm easy either way. One thing I always trip over on with sprintf() is that when you modify the query you have to count your way through the arguments for printf() and figure out where to insert any new data you add.

I think named parameter binding such as that used in PDO works best. You need an API which allows it though (I think MySQLi does).

In pseudo:

Code: Select all

$sql = 'SELECT userid FROM users WHERE username = :username AND password = :password';
$stmt = $db->prepare($sql);
$stmt->bindValue('username', $_POST['username']);
$stmt->bindValue('password', md5($_POST['password']));
 
$result = $stmt->execute();
This method also checks data types and moreso, it delegates to the DBMS itself to decide how the parameters get used (notice there's no quotes around the strings in my query).

Re: mysql_real_escape_string

Posted: Sat Jan 26, 2008 10:46 am
by VladSun
Mordred wrote:Data validation belongs to a subsystem totally different than the database layer.
Just trying to figure out why MySQL dev team recommends this. I've never used sprintf() to do query formatting. :)