Sanity check - why does this PHP MySql insert not work?

Question

Can you scan my code below and work out why the form doesn't successfully insert a record in to my MySql table 'users'?

insert.php:

<?php // insert.php
require_once 'login.php';
$con = mysqli_connect($db_hostname, $db_username, $db_password, $db_database);
// Check connection
if (mysqli_connect_errno())
{
    echo "Failed to connect to MySql: " . mysqli_connect_error();
}

$sql = "INSERT INTO users (name, email)
VALUES
('$_POST[name]]','$_POST[email]')";

if (!mysqli_query($con,$sql))
{
    die('Error: ' . mysqli_error($con));
}
echo "1 record added";

mysqli_close($con;)
?>

index.html:

<html>
<body>
<h1>Hello!</h1>

<form action="insert.php" method="post">
Name: <input type="text" name="name">
Email <input type="text" name="email">
<input type="submit">
</form>

</body>
</html>

Login credentials stored in a separate login.php file. Thanks!

Using MySQLi, but still injecting $_POST values directly into the query string..... use prepared statements/bind variables — Mark Baker, Feb 16 '14 at 17:15
Let me tell you two things. First: When you have variables inside of "-quoted strings, you should wrap them inside of {}'s, for readability -- "{$_POST['foo']}". Second: You should protect against SQL injection attacks -- Google it. — starlocke, Feb 16 '14 at 17:19
Sidenote: This line `mysqli_close($con;)` should be `mysqli_close($con);` misplaced semi-colon. — Funk Forty Niner, Feb 16 '14 at 17:30

score 2 · Answer 1 · edited Feb 16 '14 at 17:34

2

You have an extra square bracket here

 ('$_POST[name]]',
               ^------- //Remove this from your SQL Query

You need to switch to PreparedStatements seriously as the above code of yours is directly prone to SQL Injection.

edited Feb 16 '14 at 17:34

Funk Forty Niner

74,450
15
68
141

answered Feb 16 '14 at 17:15

Shankar Narayana Damodaran

68,075
43
96
126

Yep, plus the fact that, the method OP is using, is [`Open to SQL injection`](http://stackoverflow.com/q/60174/) – Funk Forty Niner Feb 16 '14 at 17:19
Yeah @Fred-ii-, I mentioned it :) Thanks :) – Shankar Narayana Damodaran Feb 16 '14 at 17:20
While that is a typo it wouldn't cause the sql to fail, it would just add a `]` to the end of the name in the column. – Patrick Evans Feb 16 '14 at 17:25
Am wondering if OP's [`misplaced semi-colon`](http://stackoverflow.com/questions/21814586/sanity-check-why-does-this-php-mysql-insert-not-work#comment33013591_21814586) would have anything to do with this. @ShankarDamodaran – Funk Forty Niner Feb 16 '14 at 17:32

score 0 · Answer 2 · answered Feb 16 '14 at 17:41

0

Try to do: VALUES ('".$_POST[name]."','".$_POST[email]."')"; so... use mysqli_real_escape_string for sql injection as it: example: $name = mysqli_real_escape_string($_POST['name']);

and it is mysqli_close($con); not mysqli_close($con;)

answered Feb 16 '14 at 17:41

Carbos

117
8

using mysqli's prepared statements would be better then concatenating the values. – Patrick Evans Feb 16 '14 at 17:42
Yes he can, why wouldnt he be able to, the docs show how to use it, the tutorials on there use are searchable – Patrick Evans Feb 16 '14 at 20:21

score 0 · Answer 3 · edited May 23 '17 at 11:45

For one thing, this line mysqli_close($con;) has a misplaced semi-colon, which should read as mysqli_close($con);

Typo or not, it was posted that way, but I'm not betting my bottom dollar on it.

Plus, your present method is open to SQL injection.

There was a slight syntax error in $_POST[name]] which should have read as $_POST[name] however, as Patrick Evans stated in a comment:

"While that is a typo it wouldn't cause the sql to fail, it would just add a ] to the end of the name in the column."

Use this method instead: (PDO) - it's safer.

<?php
/* Your Database Credentials */
$dbname = 'your_database';
$username = 'username';
$password = 'password';

$pdo = new PDO("mysql:host=localhost;dbname=$dbname", $username, $password);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$sql = "INSERT INTO users (name,
        email

        ) VALUES (
        :name, 
        :email)";

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

$stmt->bindParam(':name', $_POST['name'], PDO::PARAM_STR);       
$stmt->bindParam(':email', $_POST['email'], PDO::PARAM_STR); 

$stmt->execute(array(':name' => $_POST['name'],':email' => $_POST['email']));
if($stmt != false) {
    echo "Success!";
} else {
    echo "An error occured saving your data!";
}

?>

And your existing, with a few modifications:

<?php // insert.php
require_once 'login.php';
$con = mysqli_connect($db_hostname, $db_username, $db_password, $db_database);
// Check connection
if (mysqli_connect_errno())
{
    echo "Failed to connect to MySql: " . mysqli_connect_error();
}

$name=mysqli_real_escape_string($con,$_POST['name']);
$email=mysqli_real_escape_string($con,$_POST['email']);

$sql = "INSERT INTO users (name, email)
VALUES
('$name','$email')";

if (!mysqli_query($con,$sql))
{
    die('Error: ' . mysqli_error($con));
}
echo "1 record added";

mysqli_close($con);
?>

Sanity check - why does this PHP MySql insert not work?

3 Answers3