-3

Having some issues with this line of code:

$usersql= mysql_query("INSERT INTO tp_request (user_id, week_no, floor, code) VALUES ($insertuser, $week, $floor, $code)");

Variable values are as follows:

$insertuser = 1;
$week = 4;
$floor = "First";
$code = "ABC123";

Both $insertuser and $week go into the database without issue when used together without the other two variables. However, when $floor and $code are used, nothing gets entered into the database. They both echo correctly on their own as "First" and "ABC123" respectively.

The fields in the database have been tried as both Text and VarChar and it's not working with either of these.

Any ideas on how to get $floor and $code to work here?

Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
user910930
  • 73
  • 1
  • 8
  • 2
    stings need to be quoted –  Feb 13 '14 at 19:38
  • using something like mysqli and prepared statements would automatically handle this for you as well as sanitize your input. – Kai Qing Feb 13 '14 at 19:39
  • 1
    So it should be: `INSERT INTO tp_request (user_id, week_no, 'floor', 'code')` ? – user910930 Feb 13 '14 at 19:39
  • All your problems would be fixed if this code wasn't incredibly insecure and hackable. 1. Don't use `mysql_`. It's an old extension, and it's deprecated. Use `mysqli_` or `PDO` instead. 2. Use prepared statements and bind variables, or at *least* use `mysqli_real_escape_string` to avoid all these problems. – h2ooooooo Feb 13 '14 at 19:40
  • 4
    Stackoverflow needs to automatically reject any posts with `mysql_*()` with links to `PDO` or `mysqli` – Populus Feb 13 '14 at 19:41
  • @Populus You'd think from a site with thousands of programmers someone would've made a bot already! – h2ooooooo Feb 13 '14 at 19:41
  • Funny, I kind of had the same thought some time back; being that an automatic message gets posted, then going straight to [`this page`](http://stackoverflow.com/q/60174/) @Populus – Funk Forty Niner Feb 13 '14 at 19:42

4 Answers4

1

Try this, Added ' to wrap around the variables $floor and $code since it contains strings,

$usersql= mysql_query("INSERT INTO tp_request (user_id, week_no, floor, code) 
 VALUES ($insertuser, $week, '$floor', '$code')");

NOTE: use mysqli_* functions or PDO instead of using mysql_* functions(deprecated)

Krish R
  • 22,583
  • 7
  • 50
  • 59
  • Beat me to it. @user910930 You're breaking the SQL syntax by not wrapping strings in quotes. – Casey Dwayne Feb 13 '14 at 19:40
  • why are you quoting the integers? –  Feb 13 '14 at 19:41
  • 1
    @dagon why does it matter? Technically it isn't needed because they're integers, but it's just good practice in case they're not. – Casey Dwayne Feb 13 '14 at 19:43
  • its bad practice because they are not. any future programmer will seeing this code will amuse they are strings, and process them as such, when they are in fact integers. –  Feb 13 '14 at 19:44
  • *"A common mistake that people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools."*. Safe is better than sorry. If he ever replaces the `var` with a string instead, the SQL can remain. – Casey Dwayne Feb 13 '14 at 19:46
  • 1
    @kcdwayne Your database would fail if you input a string and it assumes an INT, so it's not any better. If anything, don't use quotes, and make sure to cast it to an int before you paste it into your query. – h2ooooooo Feb 13 '14 at 19:57
1

Rewrite like 

  $usersql= mysql_query("INSERT INTO `tp_request` (`user_id`, `week_no`, `floor`, `code`) VALUES ('$insertuser', '$week', '$floor', $code)");

Sidenote : You need to stop using mysql_* functions as they are deprecated. Switch to PreparedStatements.

Shankar Narayana Damodaran
  • 68,075
  • 43
  • 96
  • 126
  • why are you quoting the integers and why are you using `` smart quotes at all? –  Feb 13 '14 at 19:42
  • 2
    +1 for side note. PDO is more secure, unless you're into SQL injections. – Casey Dwayne Feb 13 '14 at 19:42
  • I really think "fixing" code like this is actually a bad thing, as when it's broken it's not a vector someone could crack into your site with. It's like "My gun is jammed, help me fix it so I can shoot myself in my foot!" – tadman Feb 13 '14 at 20:29
0
 $usersql= mysql_query("INSERT INTO tp_request (user_id, week_no, floor, code) VALUES ($insertuser, $week, '$floor', '$code')");
Chris
  • 3,405
  • 4
  • 29
  • 34
0

Add single quotes to you sql clause around string variables.

makallio85
  • 1,366
  • 2
  • 14
  • 29