jonnykates
jonnykates

Reputation: 26

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

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!

Upvotes: 0

Views: 229

Answers (3)

Funk Forty Niner
Funk Forty Niner

Reputation: 74217

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);
?>

Upvotes: 0

Carbos
Carbos

Reputation: 117

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;)

Upvotes: 0

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.

Upvotes: 2

Related Questions