SQL statement inside a PHP IF statement is executed even if condition is false

Question

There are a lot of very similar, near identical in fact, questions to this one and I have read through them all before posting this to see if the solutions therein could apply to my case, but alas they have not so far.

I'm using PHP 5.6 and MariaDB as the SQL DB.

As part of an email verification process the user clicks a link that contains their email address and a verification code that if matched in the DB sets the 'approved' field to 'Yes'. Very straightforward. The URL is formatted https://www.myserver.com/verify.php?user=email@address.com&auth=ri5934ij4jjo49

The PHP $_GET's these fields, queries the DB for a match and if found sets the 'approved' field to 'Yes', displays a message, then redirects and if not it displays a message then redirects. Also there's a redirect if no data is passed in so someone doesn't accidentally get stuck on this page.

The problem is, SQL UPDATE is executed even when the condition is false and the else gets triggered. As you'll see in the code I've left in the error checking and flow checking print_r and echo statements and exit()'s before the redirect so I can read the output and it appears to be flowing correctly. However the darn UPDATE gets run even when it shouldn't.

<?php

ini_set('display_errors', '1');
error_reporting(E_ALL);

if(isset($_GET['auth']) && isset($_GET['user'])){
    $auth=$_GET['auth'];
    $user=$_GET['user'];
    echo($auth.'<br />'.$user.'<br />');
    $connect = mysqli_connect("server", "username", "password", "db") or die(mysqli_error($connect));
    $verify = mysqli_query($connect, "SELECT * FROM users WHERE email='$user' AND verification_code='$auth'") or die(mysqli_error($connect));
    $verify2 = mysqli_fetch_array($verify);
    print_r($verify2);echo('<br />');
    if(mysqli_num_rows($verify)==1){
        mysqli_query($connect, "UPDATE users SET approved='Yes', verification_code='' WHERE email='$user' AND verification_code='$auth'") or die(mysqli_error($connect));
        echo('Approved');exit();
        echo('<script type="text/javascript">alert("Your registration has been approved, please log-in");window.location.href = "log-in.php";</script>');
    } else {
        echo('Not approved');exit();
        echo('<script type="text/javascript">alert("There was a problem verifying your email, please try again, re-register or contact us for help");window.location.href = "log-in.php";</script>');
    }
} else {
    header('Location: log-in.php');
}

?>

What you're saying doesn't make sense - PHP can't execute both the `if` *and* the `else` branch of the same condition. Are you certain you're testing on a value where approved is set to `No` in the database? — El_Vanja, Apr 12 '21 at 09:14
@Grumpy you're right, I've only left those two exit()'s in to halt the flow so I can read the output whilst trying to debug this issue. — John Taylor, Apr 12 '21 at 09:18
What does this print ourt print_r($verify2);echo('
');? Can you run the query straight on the table and see the result? — Grumpy, Apr 12 '21 at 09:20
@El_Vanja - exactly! That's why I'm so stumped here. If I deliberately alter the verification_code in the URL then the else gets triggered and 'Not approved' is echo'd but the DB still then shows approved=Yes — John Taylor, Apr 12 '21 at 09:21
@Grumpy - it outputs the row I expect it to based on the SELECT query successfully matching the $auth and $email. If I alter either in the URL then it's blank so it's finding the correct entry. — John Taylor, Apr 12 '21 at 09:22
`Not approved' is echo'd but the DB still then shows approved=Yes`...then either it already had that value before the code was run (perhaps left over from a previous test??) Or something else is modifying it. Code **can't** run both and if and else block at once. You've missed something. Double check all your assumptions. Reset all your data. Strip back the code to the bare minimum and build it back up again until you start to get problems. — ADyson, Apr 12 '21 at 09:25
P.s. this is horrifically vulnerable to SQL injection attacks. Please learn about parametrised queries. See https://bobby-tables.com for an explanation and also some examples of writing your queries safely using PHP — ADyson, Apr 12 '21 at 09:26
You might want to add `AND approved='No'` to your `$verify` query — brombeer, Apr 12 '21 at 09:30
Why use two separate queries to begin with? Make _one_ UPDATE query with a appropriate WHERE clause, and then draw the proper conclusion, from whether that UPDATE affected zero or more records. — CBroe, Apr 12 '21 at 09:35
Re your (rolled-back) edit - don't update the **question** with the **answer**. This site follows a Q&A format. The question area should contain the Question (unsurprisingly!). There's a separate space for Answers - see the box below to write one. See also https://stackoverflow.com/tour (so you properly understand how the site works) and https://stackoverflow.com/help/self-answer (for info on self-answering) — ADyson, Apr 12 '21 at 09:48
P.S. In that rolled-back description you also said `One or the other or both together have solved the problem.` ...don't rely on guesswork like this. Test each change independently to see if it solves the problem on its own, or in combination, or has no effect. I can tell you now though that copying the mysqli_connect code as you stated will not help - all it does is create a pointless extra connection to your database. So it just creates inefficiency without any benefit. If you had a connection problem you'd have had a different error entirely. — ADyson, Apr 12 '21 at 09:52
And PHP 5.6 is well out of date. You need to upgrade urgently. See the list of [PHP Supported Versions](https://www.php.net/supported-versions.php). — ADyson, Apr 12 '21 at 09:53
And that would be because? If you disagree with anything specific I'm happy to discuss it. All my comments were intended to be constructive and help you get to the answer, and get the best out of this site. Please don't take anything personally. If you felt something was unhelpful please do let me know. — ADyson, Apr 12 '21 at 10:21
It is a very bad idea to use `die(mysqli_error($conn));` in your code, because it could potentially leak sensitive information. See this post for more explanation: [mysqli or die, does it have to die?](https://stackoverflow.com/a/15320411/1839439) — Dharman, Apr 12 '21 at 11:10
Dharman, thank you. The code I pasted has as much debug stuff on as I could think to have on whilst I got it working. — John Taylor, Apr 13 '21 at 16:13

score 1 · Answer 1 · answered Apr 12 '21 at 10:41

1

Modifying the SQL queries to include AND approved='No' was the solution. Thank-you to brombeer for this suggestion.

answered Apr 12 '21 at 10:41

John Taylor

9
4

SQL statement inside a PHP IF statement is executed even if condition is false

1 Answers1