2

I am new to using MySQLi. I try to use MySQLi in order to insert data in my database. But does not work. Where may be the error?

echo 'connected';
$con = mysqli_connect("localhost",$username,$password,$database);

// Check connection
if (mysqli_connect_errno())
{
    echo "Failed to connect to MySQL: " . mysqli_connect_error();
}
// mysqli_select_db($con,"kraus");
$firstname = $_POST['uname'];
$lastname =  $_POST['address'];
$age = $_POST['pass'];
$sql = "INSERT INTO registration('uname', 'address', 'password') VALUES ('$firstname', '$lastname', '$age')";
mysqli_query($con,$sql);
echo "1 record added";


mysqli_close($con);
AbcAeffchen
  • 14,400
  • 15
  • 47
  • 66
user3685376
  • 63
  • 1
  • 1
  • 9
  • Think "quotes" (*do I need them, should I use them, and where*?). Then, remove them. – Funk Forty Niner Jun 05 '14 at 17:27
  • 1
    ...`('uname', 'address', 'password')` - yet, you'll be open to sql injection (in more ways than one). Do take heed. – Funk Forty Niner Jun 05 '14 at 17:27
  • remove single quotes from your column names – ɹɐqʞɐ zoɹǝɟ Jun 05 '14 at 17:32
  • You're also not checking for errors. Here, add error reporting to the top of your file(s) `error_reporting(E_ALL); ini_set('display_errors', 1); mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);` <= ***it's a "self-fix" tip.*** ;-) – Funk Forty Niner Jun 05 '14 at 17:32
  • 1
    Yup, I then read this - completely missed it so deleted answer...woops! – n34_panda Jun 05 '14 at 17:39
  • Your present code is open to [**SQL injection**](http://stackoverflow.com/q/60174/). Use [**`mysqli_*` with prepared statements**](http://www.php.net/manual/en/mysqli.quickstart.prepared-statements.php), or [**PDO**](http://php.net/pdo) with [**prepared statements**](http://php.net/pdo.prepared-statements). – Funk Forty Niner Jun 05 '14 at 17:43
  • You're storing passwords in plain text; **don't**. Use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner Jun 05 '14 at 17:44
  • Consider PDO as if you use mysqli then you limit the possibility of moving DBMS without significant code rewrite... not an answer to your question, thus i added it as a comment. – ladieu Jun 05 '14 at 17:57
  • mysqli does not having something we call "database abstraction" in otherwords it is bound to a specific database system, in this case "mysql" Not good design – ladieu Jun 05 '14 at 17:58
  • @ladieu, unless one is using mysql. – Bill Karwin Jun 05 '14 at 22:48
  • @BillKarwin this assumes your DBMS will always remain the same, a bad assumption. It makes your code less portable which is always a bad thing. Not having database abstraction is bad design. In fact if you really want to produce professional code I would recommend using an ORM.. however there is a benefit for using SQL when you are a beginner so I won't recommend that as a place to start to learn. – ladieu Jun 09 '14 at 17:12

2 Answers2

2

Why is line this commented out? You are selecting the database in mysqli_connect("localhost","root","root","kraus") but it makes no sense why that is there:

// mysqli_select_db($con,"kraus");

Should you not have that commented like this?

mysqli_select_db($con,"kraus");

Also there is no space here between registration and the fields in (…) as well as the quotes around your fields:

$sql = "INSERT INTO registration('uname', 'address', 'password') VALUES ('$firstname', '$lastname', '$age')";

That should be like the following with a space added between the table name & the fields. And since there should just be no quotes around your field names so the final query should be this: 

$sql = "INSERT INTO registration (uname, address, password) VALUES ('$firstname', '$lastname', '$age')";

Or perhaps have back ticks like this: 

$sql = "INSERT INTO registration (`uname`, `address`, `password`) VALUES ('$firstname', '$lastname', '$age')";

Also, you should really refactor & cleanup your whole codebase like this:

// Set the connection or die returning an error.
$con = mysqli_connect("localhost","root","root","kraus") or die(mysqli_connect_errno());

echo 'connected';

// Select the database.
// mysqli_select_db($con, "kraus");

$post_array = array('uname','address','pass');
foreach ($post_array as $post_key => $post_value) {
   $$post_key = isset($_POST[$post_value]) && !empty($_POST[$post_value]) ? $_POST[$post_value] : null;
}

// Set the query.
$sql = "INSERT INTO registration (uname, address, password) VALUES (?, ?, ?)";

// Bind the params.
mysqli_stmt_bind_param($sql, 'sss', $uname, $address, $pass);

// Run the query.
$result = mysqli_query($con, $sql) or die(mysqli_connect_errno());

// Free the result set.
mysqli_free_result($result);

// Close the connection.
mysqli_close($con);

echo "1 record added";

Note how I am using mysqli_stmt_bind_param and also setting an array of $_POST values & rolling throughout them. Doing those two basic things at least enforce some basic validation on your input data before it gets to the database.

Giacomo1968
  • 25,759
  • 11
  • 71
  • 103
0

You have quotes around the column names in your query. Maybe you meant to use backticks instead:

(`uname1`, `address`,...)

You are also vulnerable to sql injection. Look into mysqli prepared statements.

Nick Rolando
  • 25,879
  • 13
  • 79
  • 119