Search code examples
phpmysqlmysql-real-escape-stringquotation-marks

Escaping MySQL Query issue


I'm terribly bad at keeping MySQL queries straight, but that aside I have one query working for some data input, but not all. My guess is quotation marks getting escaped where they should be.

I have the entire query string get escaped at the same time. Is this bad practice or does it really matter?

Here's the query:

"INSERT INTO bio_manager_pubs(userid,category,citation,date,link,requests) VALUES ( ".     
$userid.",'".
$_POST['category']."', '".
htmlentities($_POST['pub'])."',
FROM_UNIXTIME(".strtotime($_POST['date'])."),'".
$_POST['link']."',
0)"

In query:

  • Userid and requests are ints
  • Link and Category are Tiny Text (not sure if that's appropriate, but max is 255 char, so would VarChar be better?)
  • Date is a date (is it better to reformat with php or reformat with mysql?)
  • Citation is a text field

Any ideas?

Thanks

EDIT: The answer to this question was posted four times there abouts where the issue was me escaping the entire query.

What was left out, and cause some confusion was the code surrounding the query. It was like this

$db->query($query)

This where the function query was:

public function query($SQL)
{
    $this->SQL = $this->mysqli->real_escape_string($SQL);
    $this->result = $this->mysqli->query($SQL);

    if ($this->result == true)
    {
        return true;
    }
    else
    {
        printf("<b>Problem with SQL:</b> %s\n", $this->SQL);
        exit;
    }
}

I just found a class that made life a bit simpler on smaller projects and stuck with it. Now, the issue I'm running into is removing $this->mysqli->real_escape_string($SQL); and adding in escapes elsewhere in the code.


Solution

  • I really don't see any sanitizing of your $_POST data, and there is really no need to run htmlentities before you insert into the database, that should be done when you take that data and display it on the page. Make sure to sanitize your posts!! Using mysql_real_escape_string() or preferably PDO with prepared statements.

    If you are running mysql_real_escape_string() on this whole query, after you build it, than that is what is breaking it.

    Use it on the individual posts, and / or cast variables that should only ever be numbers to integers.

    Heres what I would change it to in your case:

    $posted = $_POST;
    
    foreach($posted as &$value)
        $value = mysql_real_escape_string($value);
    
    $date = strtotime($posted['date']);
    
    
    $q = "INSERT INTO bio_manager_pubs(userid,category,citation,date,link,requests) VALUES
    (
    '{$userid}',
    '{$posted['category']}',
    '{$posted['pub'])}', 
    FROM_UNIXTIME({$posted['date']}),
    '{$posted['link']}',
    '0'
    )";