-4

I have this code to insert into a DB. And for some reason it is breaking on the 3rd parameter. If i take it out the other 2 work just fine.

Also i know it should be mysqli, im working with old code and just trying to get this part working. Will address updating later.

echo $_SESSION['test']['categoryname'];
$result = mysql_query('INSERT INTO '.$db_table_prefix.'saved_tests (user, category, categoryname) VALUES ('. $user["id"] .','. $_SESSION["test"]["category"] .','. $_SESSION["test"]["categoryname"] .')', $db_connection);

this outputs on the page:

Airway Respiration and Ventilation

failed to insert data1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Respiration and Ventilation)' at line 1

I cant seem to figure out what the issue is. you can see printing the variable displays the full string.

robby
  • 23
  • 6
  • 3
    Updating now - even only for this statement - and using a prepared statement will both solve your problem and the sql injection problem you have now. – jeroen Jun 25 '19 at 18:04
  • 3
    You're not quoting your values. It's not ending mid-string, it's just showing you where the issue is. Since mysql_* functions are deprecated and removed as of PHP 7.0.0, you need to switch your queries to use [PDO](https://secure.php.net/manual/en/pdo.prepared-statements.php) or [mysqli](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Use prepared statements and parameter binding, and you'll never have to worry about quoting issues again. – aynber Jun 25 '19 at 18:04
  • 1
    Possible duplicate of [When to use single quotes, double quotes, and backticks in MySQL](https://stackoverflow.com/questions/11321491/when-to-use-single-quotes-double-quotes-and-backticks-in-mysql) – aynber Jun 25 '19 at 18:04
  • "i know it should be mysqli" ... "To know the good is to do the good" -Socrates – landru27 Jun 25 '19 at 18:31
  • Possible duplicate of [Why shouldn't I use mysql\_\* functions in PHP?](https://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) – Dharman Jun 25 '19 at 22:58
  • Also duplicate of [How can I prevent SQL injection in PHP?](https://stackoverflow.com/q/60174/1839439). If you switch to prepared statements your problem will be gone. This extension was removed for a very good reason, because of problems like this one. – Dharman Jun 25 '19 at 23:07

3 Answers3

1

Add single quotes to your value for string representation.

  • Usually the date and string values should be enclosed in single quotes and
  • The numeric values do not need to be enclosed in quotes.

Also make your code more simpler like this way by using some intermediate variables to make the query string much simple, For an example-

$db_table_prefix = 'tst';// table prefix goes here
$category = $_SESSION["test"]["category"];  //category goes here;
$categoryname = $_SESSION["test"]["categoryname"]; //category name goes here
$user_id = $user['id']; //700
$query = "INSERT INTO $db_table_prefix.saved_tests (user, category, categoryname) VALUES ('$user_id','$category','$categoryname')"; 
$result = mysql_query($query,$db_connection);

N.B Also consider about how to prevent SQL Injection by using something like https://php.net/manual/en/function.mysql-real-escape-string.php.

A l w a y s S u n n y
  • 36,497
  • 8
  • 60
  • 103
  • +1 all the answers about quoting are correct, but I've upvoted this one for increasing the readability via intermediate variables; readability is a virtue in it's own right, and can be just the thing to make it clear when needed quotes are missing; all the comments about parameter binding are, of course, even better than quoting, but IMHO, this sort of readability improvement helps also when parameter binding is being used – landru27 Jun 25 '19 at 18:29
0

You need quotes around the values that contain strings.

$result = mysql_query('INSERT INTO '.$db_table_prefix.'saved_tests (user, category, categoryname) 
    VALUES ('. $user["id"] .','. $_SESSION["test"]["category"] .',
            "'. mysql_real_escape_string($_SESSION["test"]["categoryname"]) .'")', $db_connection);

If $_SESSION["test"]["category"] also contains a string you need quotes there as well.

You should also use mysql_real_escape_string() to prevent SQL injection.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Although your answer is the only one which seems to actually fight SQL injection, I can't upvote this, because it still uses extension which was removed from PHP. If we want to get rid of questions like this on SO, we need to close them as duplicates, and don't answer them. There were plenty questions like this before. The root cause is the same, not using prepared statements. – Dharman Jun 25 '19 at 23:05
  • @Dharman The question says that he can't upgrade. I can sympathize, my site has hundreds of scripts that were written using the mysql extension, and we have no resources to rewrite them. When we upgraded to PHP 7.x we installed a shim. – Barmar Jun 26 '19 at 15:06
-1

$_SESSION["test"]["category"] and $_SESSION["test"]["categoryname"] are string. You need to use quote. 

mysql_query('INSERT INTO '.$db_table_prefix.'saved_tests (user, category, categoryname) VALUES ('. $user["id"] .',"'. $_SESSION["test"]["category"] .'","'. $_SESSION["test"]["categoryname"] .'")', $db_connection);

You are not avoiding SQL Injection. Use parameterized queries, instead. Please consider to use PDO. PDO will work on 12 different database systems, whereas MySQLi will only work with MySQL databases.

Wolfetto
  • 1,030
  • 7
  • 16