Destro77
Destro77

Reputation: 125

PHP form not inserting values into DB

I'm having an issue with a form I have created (Test purposes only, I am aware it's vulnerable to SQL Injection)

Basically, the form does not insert into the DB, yet it seems to be returning true on the script.

The code is as follows:

form.php

   <form action="create.php" method="post">
        <p>Username: <input type="text" name="username" />
        </p>
        <p>Password: <input type="password" name="password" />
        </p>
        <p><input type="submit" value="Create" name= "cre" />
        </p>
    </form>

create.php

<?php
session_start();
$dbname = "obsidian";

if(isset($_POST['cre'])){


    $username = $_POST['username'];
    $password = $_POST['password'];

    $mysqli = new mysqli('localhost','admin1', 'password1','obsidian' ) or die('Failed to connect to DB' . $mysqli->error );

    $hashed_password = password_hash($password,PASSWORD_DEFAULT);

         $registerquery = "INSERT INTO users (username, hash) VALUES('$username', '$hashed_password')";
        if($registerquery = true)
        {
            echo "<h1>Success</h1>";
            echo "<p>Your account was successfully created. Please <a href=\"index.php\">click here to login</a>.</p>";
        }
        else
        {
            echo "<h1>Error</h1>";
            echo "<p>Sorry, your registration failed. Please go back and try again.</p>";    
        }       
     }   


  ?>

I get the success message, but as I stated, the values do not get inserted into the DB.

Any help would be good.

Upvotes: 1

Views: 1227

Answers (5)

Anibal Itriago
Anibal Itriago

Reputation: 1051

Formally, You should do something like this:

if(isset($_POST['cre'])){
  $username = $_POST['username'];
  $password = $_POST['password'];
  $mysqli = new mysqli('localhost','admin1', 'password1','obsidian' ) or die('Failed to connect to DB' . $mysqli->error );

  $hashed_password = password_hash($password,PASSWORD_DEFAULT);

  $registerquery = "INSERT INTO users (username, hash) VALUES('$username', '$hashed_password')";
  $stmt=$mysqli->prepare($registerquery);
  if($stmt->execute())
    {
        echo "<h1>Success</h1>";
        echo "<p>Your account was successfully created. Please <a href=\"index.php\">click here to login</a>.</p>";
    }
    else
    {
        echo "<h1>Error</h1>";
        echo "<p>Sorry, your registration failed. Please go back and try again.</p>";    
    }       
    $stmt->close();
 }   

Also, you could call only mysqli_query

if($mysqli->query($registerquery)){
....
}

It would be enough. The first call is better if you need to bind parameters and make several calls to the same query with different values.

Regards.-

Upvotes: 1

pradeepcep
pradeepcep

Reputation: 932

You have to query the database. Try this:

$registerquery = "INSERT INTO users (username, hash) VALUES('$username', '$hashed_password')";

if ($mysqli->query($registerquery))
{
    // success.
}
else
{
    // failed.
}

Here is the documentation: http://php.net/manual/en/mysqli.query.php

Upvotes: 3

Marc B
Marc B

Reputation: 360872

This defines a query, but does NOT run it:

 $registerquery = "INSERT INTO users (username, hash) VALUES('$username', '$hashed_password')";

The this is NOT "testing" for success. It's simply seeing the variable to true:

    if($registerquery = true)

= is assignment, == is for equality testing.

Upvotes: 6

Garcia Hurtado
Garcia Hurtado

Reputation: 971

All you are doing here:

$registerquery = "INSERT INTO users (username, hash) VALUES('$username', '$hashed_password')";
if($registerquery = true)

is setting a string and then later setting the string to true. This is always going to return true. There are two problems with this:

  • You need to execute the SQL statement that you have stored in the string for anything to happen in the database.
  • You are not really checking the return value ("=="), but rather using "=", which simply sets the variable. A very common mistake.

Additionally, you should probably no longer use the mysqli built in functions, since they will soon be deprecated. I would recommend switching to PDO before moving any further.

Upvotes: 1

castis
castis

Reputation: 8223

You missed the step where you actually hand the SQL query to the database.

$mysqli->query($registerquery);

has to be run before it will be inserted.

You can also change your if statement to the following

if ($mysqli->query($registerquery))

Also, you're currently using a single =, which is setting $registerquery instead of checking its value.

Upvotes: 1

Related Questions