MySQL query is incorrect

Question

I have no clue why the following MySQL Query keeps failing. It tells me it has invalid syntax. It always says it at the em-mail area of the code, but there is no obvious errors. I am cleaning up the strings before passing them into the MySQL query

$query = "INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, ".$role.", ".$name.", ".$email.", 
".$dob.", ".$englishskills.", ".$previousmember.", ".$reasonYes.", 
".$whyjoin.", ".$whatcouldyoubring.", ".$roleplayexperience.", 
".$roleplayexperiencedetail.", ".$commit.", ".$minimumperiod.");";

$result = mysqli_query($con, $query) or die(mysqli_error($con));

What is the result of `mysqlI_error($con)`? Can you print `$query` and see what it gives? — Qirel, May 19 '18 at 18:47
How to **get a clue**. Add `ini_set('display_errors', 1); ini_set('log_errors',1); error_reporting(E_ALL); mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);` to the top of your script. This will force any `mysqli_` errors to generate an Exception that you can see on the browser and other errors will also be visible on your browser. — RiggsFolly, May 19 '18 at 18:48
Then try to understand how PHP treats $variable in a double quoted string. It will simplify the concatentation — RiggsFolly, May 19 '18 at 18:48
**Then realise** Your script is wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's — RiggsFolly, May 19 '18 at 18:49
**Also** All text variables need to be wrapped in quotes like `'$var_name'` — RiggsFolly, May 19 '18 at 18:58
@RiggsFolly `@test, 1992-11-27, 5, First Application, , Best communityBest communityBest ' at line 4' in /srv/disk2/2729388/www/thamesvalleyrpc.co.uk/apply/process.php:54 Stack trace: #0 /srv/disk2/2729388/www/thamesvalleyrpc.co.uk/apply/process.php(54): mysqli_query(Object(mysqli), 'INSERT INTO `To...') #1 {main} thrown in /srv/disk2/2729388/www/thamesvalleyrpc.co.uk/apply/process.php on line 54` It looks like it's to do with the 'Insert into' — David Wheatley, May 19 '18 at 18:58
Start with the security risk. Resolving that often solves the other problems — Strawberry, May 19 '18 at 19:10

jvk · Answer 1 · 2018-05-19T18:57:02.580

2

you are closing double quotes at the ending semicolon, remove that and try again

$query = "INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, '$role', '$name', '$email', 
'$dob','$englishskills', '$previousmember', '$reasonYes', 
'$whyjoin', '$whatcouldyoubring', '$roleplayexperience', 
'$roleplayexperiencedetail', '$commit', '$minimumperiod')";

edited May 19 '18 at 18:57

answered May 19 '18 at 18:51

jvk

2,133
3
19
28

That's not it, still gives me the same problem. – David Wheatley May 19 '18 at 18:53
Did you change anything else than the semicolon `;` at the end of the query? That along would change absolutely nothing. – Qirel May 19 '18 at 18:54

score 1 · Accepted Answer · answered May 19 '18 at 19:04

Your query doesn't put any single-quotes around each value in the VALUES clause. Unless they're all numeric values or NULL, this will result in invalid SQL.

You can see it if you echo $query before you use it:

INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, myrole, myname, my@email.com, 
2014-02-03, excellent, no, myreason, 
myreason, cookies, lots, 
lots and lots, yes, 1 month);

Each string or date value in your INSERT statement needs single-quotes.

You could fix this by writing the code like this:

$query = "INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, '".$role."', '".$name."', '".$email."', 
'".$dob.", '".$englishskills."', '".$previousmember."', '".$reasonYes."', 
'".$whyjoin."', '".$whatcouldyoubring."', '".$roleplayexperience."', 
'".$roleplayexperiencedetail."', '".$commit."', '".$minimumperiod."');";

Except I made a mistake and forgot one of the single-quotes. Can you spot it?

It would be much easier if you use prepared queries with parameters, and stop copying variables into SQL strings. It makes it way easier to write dynamic SQL like this, without tearing out your hair trying to find

$query = "INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";

It a prepared statement, the ? don't need any quotes. In fact you must not put quotes around each ? because that would make it a literal '?' instead of a placeholder.

Then you prepare the query, bind variables into it.

$stmt = mysqli_prepare($con, $query) or die(mysqli_error($con));

mysqli_stmt_bind_param($stmt, 'sssssssssssss', $role, $name, $email, 
  $dob, $englishskills, $previousmember, $reasonYes, 
  $whyjoin, $whatcouldyoubring, $roleplayexperience, 
  $roleplayexperiencedetail, $commit, $minimumperiod);

You need the same number of variables as placeholders, and you bind them in the same order they appear in your query. Mysqli also requires a string like 'sss...' where each letter in that string corresponds to one of your parameters, and 's' means the parameter is a string, 'i' means it's an integer, etc.

Once you have prepared and bound parameters, just execute the prepared statement and get the results:

mysqli_stmt_execute($stmt);

$result = mysqli_stmt_get_result($stmt);

This way you don't give yourself eyestrain trying to match all the different types of quotes.

Another tip: PDO makes this even easier! You can bind and execute in one step, just by passing an array to the execute function.

$stmt = $pdo->prepare($query);

$stmt->execute([$role, $name, $email, 
      $dob, $englishskills, $previousmember, $reasonYes, 
      $whyjoin, $whatcouldyoubring, $roleplayexperience, 
      $roleplayexperiencedetail, $commit, $minimumperiod]);

$results = $stmt->fetchAll();

Only issue is that it doesn't know what `mysqli_stmt_get_result(...)` is... — David Wheatley, May 19 '18 at 19:09
"Available only with mysqlnd." http://php.net/manual/en/mysqli-stmt.get-result.php — Bill Karwin, May 19 '18 at 19:10
mysqlnd is not guaranteed to be in your build of PHP, but it's default in all builds of PHP since version 5.4.0. — Bill Karwin, May 19 '18 at 19:11
If you don't have a build of PHP that supports mysqli_stmt_get_result(), I think you have to use the bind-result function. This is less convenient. Take a look: http://php.net/manual/en/mysqli-stmt.bind-result.php — Bill Karwin, May 19 '18 at 19:13

score 0 · Answer 3 · answered May 19 '18 at 19:00

0

Strings like $name need to be enclosed using '.

$query = "INSERT INTO `ToReview` (`number`, `role`, `name`, `email`, `dob`, 
`englishskills`, `previousmember`, `reasonYes`, `whyjoin`, 
`whatcouldyoubring`, `roleplayexperience`, `roleplayexperiencedetail`, 
`commit`, `minimumperiod`) VALUES (NULL, ".$role.", '".$name."', '".$email."', 
'".$dob."', ".$englishskills.", ".$previousmember.", '".$reasonYes."', 
'".$whyjoin."', '".$whatcouldyoubring."', '".$roleplayexperience."', 
'".$roleplayexperiencedetail."', '".$commit."', '".$minimumperiod."');";

Or you could use prepared statements

answered May 19 '18 at 19:00

Lukas T

116
2
8

Ouch, thats easy to debug. **Not** – RiggsFolly May 19 '18 at 19:00
@RiggsFolly I never knew about the variables in double quotes. Thanks for that! – David Wheatley May 19 '18 at 19:03
Why is it only some that need the single quotes? – David Wheatley May 19 '18 at 19:03
I can't really tell if attributes like `englishskills` are booleans, numbers or strings – Lukas T May 19 '18 at 19:08

MySQL query is incorrect

3 Answers3