1

I have a prepared php statement to insert a row into my database. I have (what i consider) error checking along the way. At the end there is a check for an error on $stmt->execute(). If it returns false I want it to cancel the INSERT operation and show my message. The value is false but the INSERT is still successful. I had assumed if $stmt->execute()===false then there would be no INSERT into my database.

I apologize if this is a duplicate, I was having trouble finding a previous question similar to mine.

Here is the section causing the issue:

    $sql = "INSERT INTO login (username, password, name, branch, officer, type, alerts) VALUES (?, ?, ?, ?, ?, ?, ?)";
    $stmt = $nbadmin->prepare($sql);
    if(false===($nbadmin->prepare($sql))){
        $nbadmin->close();
        echo '<script>alert("Something went wrong; try again.")</script>';
        error();
    }
    $stmt->bind_param('sssssss', $user, $pass, $name, $branch, $officer, $type, $alert);
    if(false===($stmt->bind_param('sssssss', $user, $pass, $name, $branch, $officer, $type, $alert))){
        $nbadmin->close();
        echo '<script>alert("Something went wrong; try again.")</script>';
        error();
    }
    $stmt->execute();
    if(false===($stmt->execute())){
        $nbadmin->close();
        echo '<script>alert("Something went wrong; try again.")</script>';
        error();
    }else{
        $nbadmin->close();
        finish();
    }

function error(){
    header("Refresh: 0; url=../edit-user.php#user-form");
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
FamousAv8er
  • 2,345
  • 2
  • 9
  • 27
  • 5
    You execute the statement twice, first with `$stmt->execute();` and immediately after that inside the `if(...)` – Matteo Tassinari Aug 23 '19 at 16:15
  • @MatteoTassinari were you referring to the `var_dump()`? I just removed. That was there just for testing purposes – FamousAv8er Aug 23 '19 at 16:17
  • 1
    I am not referring to `var_dump`. When doing `if(false===($stmt->execute())){`, that executes the statement again! And it fails probably because there is a unique constraint on one of the fields, I would guess `username`. – Matteo Tassinari Aug 23 '19 at 16:19
  • @MatteoTassinari I see now, thank you! – FamousAv8er Aug 23 '19 at 16:20

1 Answers1

3

You are executing your query twice.

In fact your code reads:

$stmt->execute();
if(false===($stmt->execute())){
    $nbadmin->close();
    echo '<script>alert("Something went wrong; try again.")</script>';
    error();
}else{
    $nbadmin->close();
    finish();
}

However, the first row of that fragmen executes the prepared statement, which is then executed again as part of the if(...) condition.

This so performs another insert, which fails, probably because there is some unique constraint on your database table, I would guess on the username field.

You have two possible solutions. First, you could save the result of the execute() into a variable, like this:

$result = $stmt->execute();
if(false === $result) { ... }

Or else you could call the method directly inside the if(...) statement, like this:

// $stmt->execute(); // remove this line
if(false === $stmt->execute()) { ... }

I myself would favor the first option though.

Finally, please note that you execute also $nbadmin->prepare($sql) and $stmt->bind_param('sssssss', $user, $pass, $name, $branch, $officer, $type, $alert) twice, for the same reason, but those do not seem to generate an error.

Matteo Tassinari
  • 18,121
  • 8
  • 60
  • 81
  • 2
    A third option would be to enable error reporting and stop checking the return value altogether. – Dharman Aug 23 '19 at 16:57
  • Well, I think the two things are complimentary – Matteo Tassinari Aug 23 '19 at 17:13
  • @Dharman That would be beneficial to me but end users probably wouldnt know what to do with the php/mysql error – FamousAv8er Aug 23 '19 at 17:54
  • Your not supposed to display errors to the end-users. Log the errors in a file on a server. PHP has an amazing error reporting capability. See [How to get the error message in MySQLi?](https://stackoverflow.com/a/22662582/1839439) – Dharman Aug 23 '19 at 17:55
  • That is true, but you also need to tell the user whether the operation was successful, and to do so you need to check the return value. – Matteo Tassinari Aug 23 '19 at 18:04