-2

I'm trying to figure out why my code isn't working. I'm getting the "Call to a member function execute() on boolean " which should indicate i'm making an SQL error mistake. Though i'm not sure what it is. I have been looking at other posts, but they haven't been quite useful for me. Sorry if it's a duplicate.

if(isset($_POST["signup"])) {
$username = filter_input(INPUT_POST, 'username', FILTER_SANITIZE_STRING)
      or die('Error: missing username');
$email = filter_input(INPUT_POST, 'email')
     or die('Error: missing email');
$password = filter_input(INPUT_POST, 'password')
     or die('Error: missing password');

$sql = $db->prepare('SELECT username, email FROM wyvern_user WHERE 
username=?, email=?');
$sql->execute();
$sql->bind_result($username, $email);
while($stmt->fetch()) {
if($username && $email > 1) {
echo "User Already in Exists<br/>";
} else {
$query = $db->prepare("INSERT INTO wyvern_user (username, email, password) 
VALUES(?, ?, ?)");
$query->bind_param('sss', $username, $email, $password);
$query->execute();
echo "Account created";}
}
}

Here is my form aswell, if it makes a difference.

    <form class="theform" action="signup.php" method="post">
    <p>Register as user:</p><br>
    Username<span>*</span><input type="text" name="username" value="" 
    placeholder="username"><br><br>
    Email<span>*</span> <input type="email" name="email" value="" 
    placeholder="email"><br><br>
    Password<span>*</span><input type="password" name="password" 
    value="" placeholder="password"><br><br>
    <input type="submit" name="signup" value="signup"><br><br>
    </form>
Shadow
  • 32,277
  • 10
  • 49
  • 61
  • 2
    `WHERE username=?, email=?` should be `WHERE username=? AND email=?` for one. And you don't bind those variables, just the result, you need `bind_param` in the select too. And `if($username && $email > 1)` doesn't do what you think it does. – Qirel Apr 10 '17 at 22:36
  • You should use unique constraint, not a query to prevent duplicate user names and emails from being entered. – Shadow Apr 10 '17 at 23:28
  • It's not the same problem. – Tobias Heide Apr 10 '17 at 23:36
  • @TobiasHeide yes, it is exactly the same problem. Just a completely different approach to the solution. Which happens to be insensitive to race conditions, unlike the approach followed by you. – Shadow Apr 10 '17 at 23:41
  • @Machavity et al, I think the purpose of a duplicate is not to enforce best practices. Obviously creating a unique index on the username/email columns would be the better approach but if this is marked a duplicate of anything it should be [this one](http://stackoverflow.com/q/27394710/1255289). – miken32 Apr 11 '17 at 16:55

1 Answers1

0

Your SQL syntax was incorrect. You need to bind parameters to your statement before you execute it, and you don't need to fetch the results of the query if you aren't using them. Maybe this will work:

<?php
if(isset($_POST["signup"])) {
    $username = filter_input(INPUT_POST, 'username', FILTER_SANITIZE_STRING)
        or die('Error: missing username');
    $email = filter_input(INPUT_POST, 'email')
        or die('Error: missing email');
    $password = filter_input(INPUT_POST, 'password')
        or die('Error: missing password');

    $sql = $db->prepare('SELECT username, email FROM wyvern_user WHERE username=? AND email=?');
    if (!$sql) {
        die("Database error: $db->error");
    }
    $sql->bind_param("ss", $username, $email);
    $sql->execute();
    if ($sql->num_rows) {
        echo "User already exists<br/>";
    } else {
        $query = $db->prepare("INSERT INTO wyvern_user (username, email, password) VALUES(?, ?, ?)");
        $query->bind_param('sss', $username, $email, $password);
        $query->execute();
        echo "Account created";
    }
}

Not sure what your filter_input() function is doing, but you don't need to do any sanitizing when using prepared statements. You could use filter_var() with FILTER_VALIDATE_EMAIL to check the email address though.

miken32
  • 39,644
  • 15
  • 91
  • 133
  • I've tried using the code, but it doesn't do the job either. if ($sql->num_rows >= 1) { echo "User already exists
    "; } instead of just num_rows
    – Tobias Heide Apr 10 '17 at 23:07
  • When using your code i get Call to member function bind_param failure again. – Tobias Heide Apr 10 '17 at 23:13
  • Just check the results of your functions. You should always do this, though i left it out of the example code. `prepare()`, `bind_param()`, `execute()` will all return false in case of error. If you were using a modern database API like PDO, you could do this much more easily with try/catch and exceptions. – miken32 Apr 10 '17 at 23:24
  • I'm fairly new to php, and i'm trying to improve, I think PDO right now is out of reach for me, since it feels like i still get basic problems. Though i have looked at it. – Tobias Heide Apr 10 '17 at 23:27
  • @TobiasHeide if this answer was helpful, you can still mark it accepted by clicking the green checkmark, even though it was closed as a duplicate. – miken32 Aug 18 '17 at 00:09