0

I am having an issue with this segment of code in an application I'm writing:

<?php

session_start();

include('api/dbconnect.php');

$msg = "";

if(isset($_POST['register-submit'])) {
// Now we check if the data was submitted, isset() function will check if the data exists.
    if (!isset($_POST['first_name'], $_POST['last_name'], $_POST['email'], $_POST['username'], $_POST['password'])) {
        // Could not get the data that should have been sent.
        $msg = 'Please complete the registration form';
    }
// Make sure the submitted registration values are not empty.
    if (empty($_POST['first_name']) || 
        empty($_POST['last_name'])  || 
            /* empty($_POST['email']) || */ 
        empty($_POST['username']) || 
        empty($_POST['password'])) {
        
        // One or more values are empty.
        $msg = 'All fields are required for the form to be submitted';
    }

    /*if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        die ('The email address provided is invalid');
    }*/

    if (preg_match('/[A-Za-z0-9]+/', $_POST['username']) == 0) {
        $msg = 'The username provided is invalid';
    }

    if (strlen($_POST['password']) > 20 || strlen($_POST['password']) < 5) {
        $msg = 'Password must be between 5 and 20 characters';
    }

    if ($_POST['password'] != $_POST['confirm_pwd']) {
        $msg = 'The two password provided do not match.';
    }

    // We need to check if the account with that username exists.
    if ($stmt = $pdo->prepare('SELECT id, password FROM users WHERE username = ?')) {
        // Bind parameters (s = string, i = int, b = blob, etc), hash the password using the PHP password_hash function.
        $stmt->bindParam(1, $_POST['username']);
        $stmt->execute();
        // Store the result so we can check if the account exists in the database.
        if ($stmt->num_rows > 0) {
            // Username already exists
            $msg = 'Another account with this username already exists';
        } else {
            // Username doesn’t exist, insert new account
            if ($stmt = $pdo->prepare('INSERT INTO users (first_name, last_name, email, username, password) VALUES (?, ?, ?, ?, ?)')) {
                // We do not want to expose passwords in our database, so hash the password and use password_verify when a user logs in.
                $password = password_hash($_POST['password'], PASSWORD_DEFAULT);
                $stmt->bindParam(1, $_POST['first_name']);
                $stmt->bindParam(2, $_POST['last_name']);
                $stmt->bindParam(3, $_POST['email']);
                $stmt->bindParam(4, $_POST['username']);
                $stmt->bindParam(5, $password);
                $stmt->execute();
                header('Location: users.php');
            } else {
                // Something is wrong with the SQL statement. Check to make sure the users table exists with all three fields.
                $msg = 'There was a problem creating this account. Contact your Network Administrator.';
            }
        }
        $stmt->close();
    }
    else {
        // Something is wrong with the SQL statement. Check to make sure the users table exists with all 3 fields.
        $msg = 'There was a problem creating this account. Contact your network administrator.';
    }
    //$con->close();
}

?>

This code is supposed to create a new user and add them into a MySQL table. The code is working, but the code does not produce an error even if it's on purpose. For example, if I enter two different passwords, then the code thinks that there isn't a problem, so it completes the registration successfully. Everywhere there is a $msg variable, the code needs to stop and reload the page to show the error inside the HTML form so that the user can correct it, but it isn't working.

What am I not seeing in the code?

Here is the HTML form that the code belongs to:

<form class="user" method="post" action="user_new.php">
  <div class="custom-control small">
    <strong class="text-danger"><?php echo $msg; ?></strong>
  </div>
  <div class="form-group row">
    <div class="col-sm-6 mb-3 mb-sm-0">
      <input type="text" class="form-control form-control-user" id="first_name" name="first_name" placeholder="First Name" autocomplete="off" required>
    </div>
    <div class="col-sm-6">
      <input type="text" class="form-control form-control-user" id="last_name" name="last_name" placeholder="Last Name" autocomplete="off" required>
    </div>
  </div>
  <div class="form-group row">
    <div class="col-sm-6 mb-3 mb-sm-0">
      <input type="email" class="form-control form-control-user" id="email" name="email" placeholder="Email Address (optional)" autocomplete="off">
    </div>
    <div class="col-sm-6">
      <input type="text" class="form-control form-control-user" id="username" name="username" placeholder="Username" autocomplete="off" required>
    </div>
  </div>
  <div class="form-group row">
    <div class="col-sm-6 mb-3 mb-sm-0">
      <input type="password" class="form-control form-control-user" id="password" name="password" placeholder="Password" autocomplete="off" required>
    </div>
    <div class="col-sm-6">
      <input type="password" class="form-control form-control-user" id="confirm_pwd" name="confirm_pwd" placeholder="Confirm Password" autocomplete="off" required>
    </div>
  </div>
  <button type="submit" name="register-submit" class="btn btn-primary btn-user btn-block" style="background-color: #a40000; border-color: #a40000;">Create User Account</button>
  <hr>
  <a href="users.php" class="btn btn-warning btn-user btn-block" style="background-color: #2658a8; border-color: #2658a8;">Return to Users</a>
</form>
5
  • 2
    After you set $msg to something return instead of letting the code continue. As written the values you have placed in $msg aren't being used for anything.
    – Dave
    Feb 20, 2020 at 20:48
  • Do I need return somewhere? Feb 20, 2020 at 20:56
  • In addition to Dave's comment: $msg is just a variable declaration. You need to echo it and then make it stop there to no longer continue. This can be achieved in many ways though. Feb 20, 2020 at 21:00
  • You're also mixing different mysql apis; you can't do that. num_rows is mysqli_, not PDO. Feb 20, 2020 at 21:00
  • 1
    When the username does not exist, you go on with the INSERT without checking anything.
    – Ebby
    Feb 20, 2020 at 21:11

2 Answers 2

2

There are a few things with your code that need attention to.

Side note about point #1, see "Additional notes" below.

  1. $msg < you didn't do anything with that variable. They are only variable declarations. You can echo that on the page you want to echo that variable and add exit; after it in order to make it stop executing.

  2. num_rows is a mysqli_ function. You can't mix that with PDO. You could use rowCount() or fetchColumn() with an if ($stmt->rowCount() > 0){...} .

    or with fetchColumn():

     $if_row = $stmt->fetchColumn();
    
     if ($if_row > 0) {
         $msg = "A record exists.";
     }
    
  3. Add an exit; after each header. Otherwise, your code might want to continue to go further.

    Note: Since you're using a header to redirect, make sure that you're not outputting before header, otherwise it will fail.

Additional notes:

Using if ($stmt = $pdo->prepare....){...} probably isn't a good idea since that could fail silently and prematurely. I would "not" make it a conditional statement and then check if a row exists and handle the errors from there.

Plus, your <?php echo $msg; ?> inside your form could throw an undefined variable message. You could use a ternary operator for it or a standard if/else.

E.g.:

<?php if(!empty($msg)) { echo $msg; } ?>
11
  • I have to point out that rowCount is rarely a good option. You usually don't need to know the number of returned records. In this case OP definitely doesn't need it.
    – Dharman
    Feb 20, 2020 at 21:08
  • @Dharman Ok thanks. I added another option to it. I agree there are more than one method to use to check if a record exists in PDO. Feb 20, 2020 at 21:09
  • I don't understand how it's not being called. The last two $msg statements appear when I create a new account in my code. $msg is being called in the HTML form upon reload with an echo statement. Feb 20, 2020 at 21:27
  • @JeremyMyrtle I made quite a few edits since my original post. Please reload it. Feb 20, 2020 at 21:28
  • There is only one header in the PHP code and I have added the exit() to that. I still don't understand about $msg. It starts in the PHP as a blank variable. Then, if an error happens, the $msg is rewritten to show the message. After that, the page needs to be refreshed and the error message shown. That is happening on the last two $msg in the PHP code. Feb 20, 2020 at 21:32
0

You are only setting a variable $msg when a validation error occurs. This is not stopping your code from attempting to execute the database actions. Perhaps if your containing if statement checked if $msg was empty before running the database actions.

if (empty($msg) && $stmt = $pdo->prepare('SELECT id, password FROM users WHERE username = ?')) {

9
  • 1
    $$msg is already empty before the if statement. That's what $msg = ""; is for. Feb 20, 2020 at 20:56
  • @JeremyMyrtle the if that contains the database actions, as shown above. Feb 20, 2020 at 20:57
  • I get this error returned: Notice: Undefined property: PDOStatement::$num_rows in C:\xampp\htdocs\cabgop\user_new.php on line 43 Feb 20, 2020 at 21:00
  • @JeremyMyrtle sounds like you did something different than what I posted above. Feb 20, 2020 at 21:01
  • 1
    @dqhendricks No, it's not different. Look at their code again and my comment about it. Feb 20, 2020 at 21:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.