Register won't Insert

General PHP Help
#1

I am not sure what it is but my code will not insert into my database, I’ve tried changing the code a 100 ways but I am still learning this new stuff. Probably a simple fix but if you know your help would be appreciated.

<?php
$Username = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Username'])));
$Password = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Password'])));
$ConfirmPassword = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_ConfirmPassword'])));
$Email = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Email'])));
$Submit = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Submit'])));
function is_alphanumeric($Username) {
        return (bool)preg_match("/^([a-zA-Z0-9])+$/i", $Username);					 
}
					
if ($Submit) {
    $Username = filter($Username);
    if (!$Username||!$Password||!$ConfirmPassword) {
        echo "<b>Please fill in all required fields.</b>";
    } else {
	    $userExist = mysqli_query("SELECT * FROM Users WHERE Username='$Username'");
	    $userExist = mysqli_num_rows($userExist);
	    $userExist1 = mysqli_query("SELECT * FROM Users WHERE OriginalName='$Username'");
	    $userExist1 = mysqli_num_rows($userExist1);
	    if ($userExist > 0) {
	        echo "<div id='Error'>That username already exists.</div>";
	    } elseif ($userExist1 > 0) {
	        echo "<div id='Error'>That username already exists.</div>";
	    } else {
	        if ($ConfirmPassword != $Password) {
	            echo "<div id='Error'>Your password and confirm password does not match.</div>";
	        } else {
	            if (strlen($Username) >= 15) {
	                echo "<div id='Error'>Your username is above fifteen (15) characters!</div>";
	            } elseif (strlen($Username) < 3) {
	                echo "<div id='Error'>Your username is under three (3) characters!</div>"; 
	            } elseif (!is_alphanumeric($Username)) {
	                echo "<div id='Error'>Only A-Z and 1-9 is allowed, or there is profanity in your username.</div>";
	            } else {
	                $_ENCRYPT = hash('sha512',$Password);
	                $IP = $_SERVER['REMOTE_ADDR'];
	                mysqli_query("INSERT INTO Users (Username, Password, Email, IP) VALUES ('$Username', '$_ENCRYPT', '$Email', '$IP')");
	                $_SESSION['Username']=$Username;
	                $_SESSION['Password']=$_ENCRYPT;
	                header("Location: index.php");
	            }
	        }
        }
    }
}
?>
#2

Marvin, for some reason the new version of this site does not do well with posting code. The owner was looking into this but has not got a solution for it so far.
I took your code and put it into one of my editors and fixed the layout so I could read it better. Although you use a complicated nested if-clause set up, most of it looks correct in general, but, several incorrect lines are in the code.

First, you there is no reason to import the submit button, just test for it something like this:
if ( isset( $_POST[’_Submit’] ) )
This will save you loading the $Submit variable.

Next, right after you check for the submit button, you call a function to load the $Username variable a second time. You call a function named filter(), but there is no such function in your code. That line should be removed since you already load the username earlier in the code.

Now that is also an error. You should only load the username, password, confirmed-password and email AFTER you check for the submit button. If the submit button is NOT pressed, then this entire code is not needed as the form was not submitted. Here is a better version of this section:

//  Since this function is only one line, I suggest removing it and just use the compare directly where needed!
function is_alphanumeric($Username) {
    return (bool)preg_match("/^([a-zA-Z0-9])+$/i", $Username);
}
//  Check if user posted the form, if so handle it
if (isset($_POST["_Submit"])) {
    $Username = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Username'])));
    $Password = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Password'])));
    $ConfirmPassword = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_ConfirmPassword'])));
    $Email = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Email'])));
    //  Rest of code...

Looking at the next problem, you run two queries in a row which makes no since at all. You load data if the username is in the table, then do a second check if it is in the original name field. This makes no sense. Just check once if the username is in either field. Something like this:

        $userExist = mysqli_query("SELECT * FROM Users WHERE Username='$Username' OR OriginalName='$Username'");
        $userExist = mysqli_num_rows($userExist);
        if ($userExist > 0) {

Less lines makes for simpler code. Simpler code is easier to debug. Now, my guess is that your problem was the invalid function you used in the code named “filter()” which does not exist.
Rewrite your code with some of these ideas and show us the new version. Oh, also, you need to enter code like this. Enter one blank line then post your code. Select your code and press the quote button or the preformatted-code button. (This depends on the content, ones work better than others.) This way your code will be listed apart from your notes. Like mine above.

Oh lastly, you should read up on the newer filter_input() functions for PHP. It is much easier to use than the escape-string-strip-tags-slashes code you use. Here is an example of how to do it: Filter-Input

#3

I tried that code and it does look a lot cleaner and I appreciate it but it errors out to “Please fill out all required fields” and I am not sure why.

if (!$Username||!$Password||!$ConfirmPassword||!$Email) { echo "<b>Please fill in all required fields.</b>"; } else {

#4

Are we looking at the same code? There is ALOT wrong with the OP’s code including the logic.

#5

Kev, you along with Astonecipher are probably the best programmers on this site. Sometimes in the beginners lists, you need to bring them along slowly and give them just enough to get over things step by step… I try to give small pushes instead of moving them into O.O.P. and PDO as you like to. Nothing wrong with that, but, most beginners are using text books and teachers that still give them MySQL code. Hard to believe in this day and age! Oh, and I never just give them a solution. What would that do for them? But, jump in and do your thing.

Marvin, you last asked about using !$Username. This means NOT $Username. I find that is confusing. If the field contains nothing, you might use $Username!="" which is not the same thing as !$Username. Some programmers like to check posted fields this way: if (empty($_POST[$field])) Others use something like this: if (trim($_POST[$field]=="")) These are two ways to check if the input fields are empty or equal to nothing. “NOT $field” is confusing because this means “NOT CONDITION” and you did not put in a condition. Therefore, ’ NOT $field==“something” ’ would mean NOT a condition of a field equaling something. Using your example, you could rewrite it as if(!$Username=="") and it would check if the username was NOT empty. But, that is opposite from what you want as it checks for NOT empty and you want to check for empty fields. So, I suggest if($trim($Username=="") which will check for that field being empty or just containing spaces. You should try to rewrite this section and post you try back if it does not work. Hope that explains it.

1 Like
#6

I apologize for the late response, but I figured it out before I had a chance to read your message. Just took a little bit of digging into google to find the right code. I am definitely getting better but I am at the beginning of the road for programming. Sorry for not posting this reply sooner.

#7

Well, step by step, Marvin! When you get stuck on the next step, please open another post and we are here to help you.

Perhaps I should add that @ benanamen is an advanced programmer above me in many areas. He promotes the use of secured code and PDO. PDO is the newer object-orientated language that is better than MySQL coding. At some point, once you get your programs working you might want to read up on it.

Glad you solved it… See you in your next post.

#8

Cleaned up the formatting of the original post.

#9

Now that you have your code “working”, post the latest version and we will help get you to the next level.

#10

Ok so here is my new code and I cleaned it up to make it easier to read. The only problems I am getting is that the Username length checker doesn’t work right and the Alphanumeric doesn’t work either, also when I remove these two codes to test if my information enters into the database it does not go into the table it just goes to index.php

if (isset($_POST["_Submit"])) {
    	$Username = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Username'])));
 		$Password = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Password'])));
   		$ConfirmPassword = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_ConfirmPassword'])));
  		$Email = mysqli_real_escape_string(strip_tags(stripslashes($_POST['_Email'])));
function is_alphanumeric($Username)
					{
						return (bool)preg_match("/^([a-zA-Z0-9])+$/i", $Username);
						 
					}
  		
	if (!$_POST['_Username']||!$_POST['_Password']||!$_POST['_ConfirmPassword']||!$_POST['_Email']) {
		echo "<b>Please fill in all required fields.</b>";
	} else {
	$userExist = mysqli_query($conn, "SELECT * FROM Users WHERE Username='$Username' OR OriginalName='$Username'");
    $userExist = mysqli_num_rows($userExist);
        if ($userExist > 0) {
            echo "<div id='Error'>That username already exists.</div>";
        } else if ($ConfirmPassword != $Password) {
            echo "<div id='Error'>Your password and confirm password does not match.</div>";
        } else if (strlen($Username)>15||strlen($Username)<3) {
            echo "<div id='Error'>Your username needs to be 3 to 15 characters!</div>";
        } else if (!is_alphanumeric($Username)) {
            echo "<div id='Error'>Only A-Z and 1-9 is allowed, or there is profanity in your username.</div>";
        } else {

    $_ENCRYPT = $Password;
    $IP = $_SERVER['REMOTE_ADDR'];
    mysqli_query($conn, "INSERT INTO Users (Username, Password, Email, IP) VALUES ('$Username', '$_ENCRYPT', '$Email', '$IP')");
    $_SESSION['Username']=$Username;
    $_SESSION['Password']=$_ENCRYPT;
    header("Location: index.php");
}
}
}
}

Addon - I also believe the code is not pulling the information from the form.

#11

Ok, on to the “real” problems…

  1. Hoping the name of a button to be submitted in order for your script to work will completely fail in certain cases. You need to check the REQUEST METHOD.

  2. You need to use Prepared Statements. That will also eliminate all that escaping code.

  3. Do not ever slap functions in the middle of a code block.

  4. You need to trim the POST array and then check for empty on required fields.

  5. Do not check if a username is already taken. Set a unique constrain on the column, then attempt the insert and capture the duplicate error if any. This is one of the few times you should use Try/Catch.

  6. Get rid of the variable for nothing $_ENCRYPT and use password_hash and password_verify.

  7. Never ever put variables in your queries. Use Prepared Statements

  8. Kill the script after a header redirect with die or exit. In this case nothing follows so it doesn’t really matter.

  9. Use a proper IDE with syntax highlighting and you wont miss simple errors like the extra curly brace in your code.

  10. I would suggest you use a more easily readable code format style. I use Whitesmiths myself.

#12

Thank you benanamen for the help, I will post a reply as soon as I get a chance to do these few things and hopefully get my code working and in good condition.

#13

You need to set php’s error_reporting to E_ALL and display_errors to ON, preferably in the php.ini on your system, so that php will help you by reporting and displaying all the errors it detects. You will save a TON of time while learning, developing, and debugging code/queries.

The reason it doesn’t seem that you are getting any form data is because the …_escape_string() statements you have in your code are incorrectly coded and you would be getting php errors to alert you to the problem.

However, when you switch to use prepared queries (and the php PDO extension), the …_escape_string() statements are no longer need and can be removed.

Here’s a list of issues with this code (repeats some of the items already mentioned in the thread) -

  1. Detect that a post method form was submitted before referencing any of the form data.
  2. If they are not already, your form and form processing code should be on the same page. This will let you display the validation errors when you re-display the form and repopulate the form field values with the submitted data so that the user doesn’t need to keep reentering the same data.
  3. Use an array to hold validation errors. The array is also an error flag. If the array is empty, there are no errors. If the array is not empty, there are errors.
  4. Forget about using striplsashes (not needed after php 5.5, released in 2012) and strip_tags (< and > should be allowed in usernames, passwords, and are actually valid in the name part of email addresses.)
  5. Forget about using any …_escape_string() function. You are also using them incorrectly. This is just a lot of code that can still allow sql injection if you haven’t set the character set that php is using to match the character set your database tables are using. You should be using prepared queries, which will result in the simplest code and the simplest sql query syntax. (If you did have a case where you were using …_escape_string() on data, you would do it right before using the data in a query. By doing it before validating the data, you are altering the values and changing what is getting validated.)
  6. You should be using the much simpler and more consistent php PDO extension.
  7. Trim all the input data at once so that you can detect if all white-space characters were entered. You can do this will one line of code (and a simple call-back function.)
  8. Validate all the input data at once (your nested conditional logic is only validating one input at a time, requiring multiples form submissions to validate all the inputs.) For a registration script, in addition to first validating that ‘required’ fields are not empty, you would validate the content/format/length… of the data as appropriate for the type of data.
  9. If here are no validation errors, use the submitted data. Any comparison (the password and confirm password) or other use of the input data must come after it has been validated.
  10. Rather than try to run initial SELECT query(ies) to find if data already exists, which has a race condition with concurrent users (or the same user submitting the form multiple times) that will allow same values to be used, just insert the data and detect any duplicate key error from the query (requires that columns holding unique values be set up as unique indexes - you should be doing this anyway so that the database enforces uniqueness.) If you detect a duplicate key error, if you have multiple columns that are unique, you can run a SELECT query to find which column values where duplicates. This will also reduce the amount of queries, since you only run a/one SELECT query if a duplicate key was detected.
  11. Php’s password_hash() and password_verify() should be used for hashing and testing the hashed password.
  12. The only value your login code should store in a session variable is the user id (auto-increment column in database table.) If you are doing this for real, your registration code would not automatically login the user. You would have an email verification system to activate the user’s account as the next step in the process and then require the user to enter the username and password to login.
  13. Any header() redirect needs an exit; statement after it to prevent the remainder of the code on the page from running.
  14. You need error handling for all your database statements (connection, query, prepare, execute.) The easiest way of adding error handling, without needing to add program logic, is to enabled exceptions for errors for the database extension you are using and let php catch the exceptions where it will use its error_reporting, display_errors and log_errors settings to control what happens with the actual error information. The exception to this is item #10 in this list, where your code needs to catch the exception and handle any duplicate key error.

Doing these things will greatly simplify the code/queries and improve the User eXperiance (UX.) They will also result in code that either works or it will tell you why it isn’t working.

#14

Do a tutorial on prepared statements in mysqli and pdo.

After you’re sure you know how to do them properly, just stop using manually written sql strings for queries 97% of the time by using an ORM or Query Builder.

I found these to be very noob friendly for small projects: http://j4mie.github.io/idiormandparis/

The less actual sql syntax you have to write, the happier you’ll be.

#15

I’ll agree and disagree with this. You should not use an exception for something that you expect to happen, that isn’t an exception, it is guiding logic by exception. You should check if the username exists AND have the constraint on the column for data integrity.

Why is that? I enjoy SQL and live in databases. ORM’s while useful, are also bloated and do their own thing. You can’t really tune an ORM query.

#16

That’s why i said 97% instead of 100%.

I’ve seen countless posts over the years where people have a very large form and they’re manually writing the SQL (whether it be bound parameters or string concatenation) and they’re running into bugs caused by a tiny mistake in the SQL syntax or missing one of the many fields from their form due to a typo. In those situations it’s so much easier to pass the entire post array to a class and hit ->save()

#17

Sorry boss, but that is incorrect. You do not check if the username exists. By doing so you create a race condition whereby simultaneous requests will all get the OK to insert but only one request will actually be entered into the DB, all the others will fail even though the username check passed. The correct and error proof method is to set a unique constraint on the column, attempt the insert and catch the duplicate error if there is one.

#18

In your opinion it is not correct. It is incorrect to use exceptions, that aren’t actually exceptions, to drive application logic.

#19

A duplicate constraint error is not an exception?

Please tell me then how you will avoid creating a race condition with your method.

It is not an opinion. Code execution proves it out.

#20

http://wiki.c2.com/?DontUseExceptionsForFlowControl

I didn’t say don’t use the constraint, I said don’t use it as the flow control method. What race condition are we talking about, the likelihood that two [or more] people are going to use the same value with in milliseconds?

Sponsor our Newsletter | Privacy Policy | Terms of Service