First attempt at prepared statements, is it correct? it seems to work!

[vexdarkness]

So this is my prepared statement to register a new user on my site. It works, but is it “secure”? Any comments would be appreciated!

  if (count($errors) == 0) {
  	$password = password_hash($password_1, PASSWORD_DEFAULT);//encrypt the password before saving in the database

      $stmt = $db->prepare("INSERT INTO members (member_name, member_password, member_email) VALUES (?, ?, ?)");
      $stmt->bind_param("sss", $member_name, $member_password, $member_email);

	$member_name = "$username";
	$member_password = "$password";
	$member_email = "$email";
	$stmt->execute();
}

[m_hutley]

$member_name = "$username";

This has no value. $username should already be a string, so just use it as the parameter. Same with $password and $email.

It’s a valid use of prepared statements, just longer than it needs to be

[vexdarkness]

So like this:

$stmt->bind_param("sss", $username, $password, $email);

[vexdarkness]

Worked great, thanks!

[benanamen]

@vexdarkness

if (count($errors) == 0)

This could simply be

if (!$errors))

Although, you should arrange your code so that you are doing truthy checks, as if is a truthy check by default.

if ($errors)){
//Handle Errors
}
else{
// Insert Data
}

[spaceshiptrooper]

You really don’t need all of those useless variables. You’re basically reassigning variables to new variables and only using those new variables. Why not cut to the chase and just use your desired variable names?

[vexdarkness]

@benanamen

Although, you should arrange your code so that you are doing truthy checks, as if is a truthy check by default.

Okay thanks!

@spaceshiptrooper

You really don’t need all of those useless variables. You’re basically reassigning variables to new variables and only using those new variables. Why not cut to the chase and just use your desired variable names?

This is my current code:

   $stmt = $db->prepare("INSERT INTO members (member_name, member_password, member_email) VALUES (?, ?, ?)");
      $stmt->bind_param("sss", $username, $password, $email);
	$stmt->execute();

Better?

[vincekaribusana]

I would also put some exceptions if any error occurs.

[benanamen]

You would think so, but no. Php is well suited to handle its errors on it’s own. Now, what you could do is use set_exemption_handler to customize how exceptions are handled. You now have one place to handle exceptions without littering your code base. There are only a couple instances where you would implement a try/catch and that is with the DB connection and DB duplicate errors. And you for sure don’t want to output internal system errors to the user. That information is only good to hackers and useless to the user.

[vincekaribusana]

Hi @benanamen please correct me if I’m wrong. My understanding is that we can use set_exempion_handler to handle all uncaught exceptions, but use try catch block if we want to create a custom exception message, is it correct?

[vexdarkness]

@vincekaribusana

I would also put some exceptions if any error occurs.

Already have. I only showed part of my code.

[spaceshiptrooper]

@vexdarkness

Better?

Much better

@vincekaribusana

is it correct?

I think @benanamen is alluding to the fact that you shouldn’t be littering your code with try/catch blocks. My understanding of why you shouldn’t try to “catch 'em all” is because you should already know what kind of errors your code produces. The only time when you should technically use try/catch blocks is on things that happen internally where you may not know the root cause of an issue. benanamen gave a great example of that. Usually when the database breaks for some reason, you don’t want to be carrying that broken connection over so you might want to catch those errors. Things like this do actually happen randomly. An example can be the database engine just shutting off randomly due to maybe a corrupted data or file. Another example is maybe the database engine breaking from an update. All of these things, you can’t really control. That’s when it would be plausible to use a try/catch block. If it’s just something logical, you should already be expecting some false positive or positive results. This is all logical.

[benanamen]

@vincekaribusana, yes, but see the excellent response from @spaceshiptrooper.

[vincekaribusana]

@benanamen you know everything I learn is thanks to this form but also searching online and ther eis where I found this article https://www.codewall.co.uk/php-mysqli-examples-the-ultimate-tutorial/

[benanamen]

There are many, many “tutorials” out there. I think it is safe to say that most of them get it “wrong”, or at least parts of it wrong including the one you just posted. Consider them guides until you know better.

[vexdarkness]

Anyway, thanks so much for everyone’s help, it is most appreciated!