Register to insert into MariaDB

[sigmahokies]

I don't understand why it doesn't work to insert in Phpmyadmin (MariaDB) with my code...It saying failed to add all times. I know about security issue, I will install security later. I just want to make sure insert into MariaDB is working, then I will set up security system. Here my code:

	/* Register and check username and email is exist or not */
	if (isset($_POST['submitted'])) {
	$username = $_POST['user'];
	$email = $_POST['email'];
	$first = $_POST['first'];
	$last = $_POST['last'];
	$password = $_POST['password'];
	$check_user = "SELECT * FROM username where Username = '".$username."'";
	$check_email = "SELECT * FROM username where Email = '".$email."'";
	$check_user2 = mysqli_query($GaryDB, $check_user);
	$check_email2 = mysqli_query($GaryDB, $check_email);
	if(mysqli_num_rows($check_user2) > 0) {
	$taken_user = "→ Sorry, Username is taken"; }
	elseif (mysqli_num_rows($check_email2) > 0) {
	$taken_email = "→ Sorry, E-mail is taken"; }
	else { $register = "INSERT INTO username (Username, Password, FirstName, LastName, Email) VALUES ('$username','$password','$first','$last','$email')"; $insert = mysqli_query($GaryDB, $register); }
	if ($insert) {
	$insert1 = "successfully added"; }
	else {
	$insert1 = "Failed to added"; }
	}
	

Can you find what I did do wrong?

Thank you in advance!

[Barand]

Check for mysql error message to see why it failed

[Phi11W]

As Brand says, try the assembled SQL manually and see what the database is objecting to. 

Some other thoughts: 

1. You're wide open to an SQL Injection Attack. 
Obligatory XKCD Reference - Little Bobby Tables. 

Less severely, you're effectively excluding anybody with an apostrophe in their name, e.g. "Peter O'Toole", from registering with your site!
Not that you'd be the only one, according to IrishCentral. 

Why is this?  Because in your PHP code you're building a String value that just happens to contain some text that your database should be able to make sense of.  By blindly bolting things together in this way - easy though it might be - you're not following the Rules that SQL expects.  Look into Prepared Statements as a way to correct this. 

2. Never use "Select *" in Production code. 

Both of your checks are pulling every field in the username table.  That might be fine now, when you only have a handful of small fields to worry about but sooner or later, someone [else] is going to think it's a "great idea" to add a BLOB field into this table that contains terabytes of video profile for each user.  Suddenly, your user check, which used to be really quick, is having to haul all of that data back across the network, even though its not interested in a single byte of it! 

Always specify the fields that you want to work with explicitly. 

All that said, I wouldn't perform the check this way at all. 
What you have here is a potential Race Condition.  Computers are fast.  Really fast.  It's possible that, after checking for duplicate username and email but before doing the actual insert, someone else could get in and insert the same values into the database.   Now you have duplicate user records or two people using the same account.  Nightmare. 

Instead, get the database to do the heavy lifting for you: 

Add a unique index on username.email.
Add a unique index on username.username. 
Remove both the check queries and just attempt the insert.  If the user tries to reuse an existing username or email address, you'll get an error that you can handle in your code. 

Finally, your PHP code, as given, is difficult to read.  You'll spend far more time reading code than writing it so start to think about readability now.  (I very nearly "went off" on one because you were building a SQL string instead of executing it.   I found the execution, eventually, way over to the right, off the edge of my screen!)  

Think about code readability, not least of which "one statement per line" and "indenting".  With that in mind, let's take another look at your code: 

/* Register and check username and email is exist or not */
if (isset($_POST['submitted'])) {
  $username = $_POST['user'];
  $email = $_POST['email'];
  $first = $_POST['first'];
  $last = $_POST['last'];
  $password = $_POST['password'];
  $check_user = "SELECT * FROM username where Username = '".$username."'";
  $check_email = "SELECT * FROM username where Email = '".$email."'";
  $check_user2 = mysqli_query($GaryDB, $check_user);
  $check_email2 = mysqli_query($GaryDB, $check_email);
  if(mysqli_num_rows($check_user2) > 0) {
    $taken_user = "→ Sorry, Username is taken"; 
  }
  elseif (mysqli_num_rows($check_email2) > 0) {
    $taken_email = "→ Sorry, E-mail is taken"; 
  }
  else { 
    $register = "INSERT INTO username (Username, Password, FirstName, LastName, Email) VALUES ('$username','$password','$first','$last','$email')"; 
    $insert = mysqli_query($GaryDB, $register); 
/* } --- Closing brace - wrong place! */ 
    if ($insert) {
      $insert1 = "successfully added"; 
    }
    else {
      $insert1 = "Failed to added"; 
    }
  }
/* --- Closing brace SHOULD be here */ 

Note that, because of the misplaced braces (which are far easier to see with the code nicely laid out) the code is always passing through the "if($insert)" test, regardless of what else it does.  It should only go through that bit if it's tried to do an insert, which it will with the closing brace moved as I've described. 

Regards, 
   Phill  W.

 

[sigmahokies]

Hi Phill W.,

I know about SQL injection, it's wide open. but it's practice, I said i will set up the SQL security after I make sure Insert and update is working. Relax! Also, I know how to close SQL in PHP, I want to make sure it is working, beside, it's in xampp, so it's localhost, not open to internet. I'm not that dumb. I need to practice to make it work, so I can move on with my demonstration to interview. SQL injection is my last to install in my code in PHP.

Thanks,

Gary