-4

I know a lot of people have the same error occasionally however I have looked at all previous answers and my code and i have tried col with and without backticks Here is my current code I also have tried with $var as well as just $var but same

if(!empty($_POST['email'])){
 $date = date('dmY'); #Todays Date
 $ip = str_replace('.','',$_SERVER['REMOTE_ADDR']); #Visitor IP
 $verify = md5($date.$ip); #MD5 ENCRYPT THE 2 VALUES
 $fname = $_POST['fname'];
 $lname = $_POST['lname'];  
 $email = $_POST['email'];
 $password = md5($_POST['password']);
 $link = mysqli_connect($dbh,$dbu, $dbp, $dbn);

 $query = mysqli_query($link, "INSERT INTO `users` (`email`,`fname`,`lname`,`verify`,`password`,`joined`)
VALUES($email,$fname,$lname,$verify,$password,$date)");

 if($query){ 
  echo "inserted"; 
 }
 else { 
  echo mysqli_error($link);
 }

There are other columns in the table however its only the above columns I want to add data for the rest can use default values initially

I've been looking at this code for so long now I just cant spot my problem, I know its something silly

Chris Yates
  • 243
  • 3
  • 16
  • because uid is an auto_increment – Chris Yates Nov 21 '17 at 11:39
  • its NOT for a password I am creating a verification string.. ignore what the actual column names say.. Can we just focus on the actual question please – Chris Yates Nov 21 '17 at 11:41
  • Your query _would_ be missing quotes for the values (`..'$email','$fname'.."`). But you'd be prone to SQL Injection. Use prepared statements with parameterized queries – FirstOne Nov 21 '17 at 11:41
  • _its NOT for a password..._ Well.. your code says "it is" – B001ᛦ Nov 21 '17 at 11:42
  • NO my code says password and says its a column name called password.. Ive said twice its not regardless of what the col names say – Chris Yates Nov 21 '17 at 11:43
  • 1
    _a column name called password_ A column called "password" is usually for storing "password" – B001ᛦ Nov 21 '17 at 11:44
  • 2
    In as much as you want solution to your question, people will not enjoy helping you out without pointing you to best practices. – ultrasamad Nov 21 '17 at 11:44
  • So use prepared statements and if it is password hashing make sure to use `password_hash()` and your code will be neat and easy to debug. – ultrasamad Nov 21 '17 at 11:47

1 Answers1

1

The most mistake-proof way to add a variable into an SQL query is to add it through a prepared statement.

So, for every query you run, if at least one variable is going to be used, you have to substitute it with a placeholder, then prepare your query, and then execute it, passing variables separately.

First of all, you have to alter your query, adding placeholders in place of variables. Your query will become:

$sql = "INSERT INTO users (fname, lname) VALUES (?, ?)";

Then, you will have to prepare it, bind variables, and execute:

$stmt = mysqli_prepare($conn, $sql);
mysqli_stmt_bind_param($stmt, "ss", $fname, $lname);
mysqli_stmt_execute($stmt);

As you can see, it's just three simple commands:

  • prepare() where you send a query with placeholders
  • bind_param where you send a string with types ("s" is for string and you can use it for any type actually) and than actual variables.
  • and execute()

This way, you can always be sure that not a single SQL syntax error can be caused by the data you added to the query! As a bonus, this code is bullet-proof against SQL injection too!

It is very important to understand that simply adding quotes around a variable is not enough and will eventually lead to innumerable problems, from syntax errors to SQL injections. On the other hand, due to the very nature of prepared statements, it's a bullet-proof solution that makes it impossible to introduce any problem through a data variable.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Exactly what I was actually hoping someone would post I tried prepared statements but couldnt get it working hence I went back to a basic query ( had issues with the bindparam not working ) Will accept answer many thanks – Chris Yates Nov 21 '17 at 11:45
  • It now give this Warning: mysqli_stmt_bind_param() expects parameter 1 to be mysqli_stmt, boolean given – Chris Yates Nov 21 '17 at 11:51
  • So check to make sure the first parameter is actually your `$statement` variable thus if you declared it. – ultrasamad Nov 21 '17 at 11:53
  • it means you still have an error in your query. the number of fields should be equal to the number of placeholders and the syntax should be a correct one – Your Common Sense Nov 21 '17 at 11:56
  • $sql = "INSERT INTO `users` (email,fname,lname,verify,password,joined) VALUES(?,?,?,?,?,?)"; $stmt = mysqli_prepare($con, $sql); mysqli_stmt_bind_param($stmt, "ssssss", $email,$fname, $lname, $verify, $password, $date); Everything matches , I even removed the uid column so just have 6 cols having data added to and have 6 placeholders – Chris Yates Nov 21 '17 at 12:02
  • I have 9 columns altogether in the table will that be causing the error ? – Chris Yates Nov 21 '17 at 12:17
  • I have $con defined in an external file, I removed a load of code earlier but obviously missed the $link so its still there – Chris Yates Nov 21 '17 at 12:24
  • ahh the verify value is too long .. will increase the col value size in db and try again – Chris Yates Nov 21 '17 at 12:35
  • Yep works perfectly. I'll edit my originally pasted code to reflect the $link connection now – Chris Yates Nov 21 '17 at 12:38