Prevent pre-defined form fields from being empty

[robkar32]

So. I am currently bug testing my login system, I am checking for empty fields in the registration form with a loop like this: 

function is_allowed_field($string) {
		$allowed_fields = array('email', 'username','password');
		if(in_array($string, $allowed_fields, true)) {
			return true;
		} else {
			return false;
		}
	}

	$uid;
	$emptyfields;
	$i = 0; // Counts empty fields

	foreach($_POST as $name => $value) {

		if($name != 'submit') {

			if(is_allowed_field($name)) {
				$uid[$name] = $value;
			}

			
			if(empty($uid[$name]) && is_allowed_field($name)) {
				$emptyfields[$i++] = $name;
			}
		}
	}

	
	if($i > 0) {
		$error = implode('+', $emptyfields);
		die(header('Location: ../register.php?reg=false&errtype=emptyfields&field=' . $error));
	}

My problem is. If i change this HTML:

			<input type='text' name='email'></br>
			<input type='text' name='username'></br>
			<input type='password' name='password'></br>
			<input type='submit' value='Register' name='submit'>

Into this:

<input type='text' name='blablabl'></br>
<input type='text' name='blablablabalbal'></br>
<input type='password' name='bla'></br>
<input type='submit' value='Register' name='submit'>

I'm able to bypass this section of my code completely and move on to the next one. That's a problem. I want header to return all fields in the $allowed_fields array that are empty, no matter if you fiddle with the HTML.

 

( I know i could code this differently without a loop if i wanted just a basic login system, but i'm making something different. ) .. 

[Sepodati]

if(empty($uid[$name]) && is_allowed_field($name)) {

 

This comes out to false when you're dealing with $_POST['blabla']. empty() will return true, but is_allowed_field() will return false. Just like it did in the previous if() condition.

 

You're better off using filter_input() (http://php.net/manual/en/function.filter-input.php) and checking each value independently. You lose the context of what you're checking when you attempt to automate the checks like this.

[robkar32]

if(empty($uid[$name]) && is_allowed_field($name)) {

 

This comes out to false when you're dealing with $_POST['blabla']. empty() will return true, but is_allowed_field() will return false. Just like it did in the previous if() condition.

 

You're better off using filter_input() (http://php.net/manual/en/function.filter-input.php) and checking each value independently. You lose the context of what you're checking when you attempt to automate the checks like this.

Yessir i'm aware i could check each value independently, but the goal here is to make this login system highly customizable, and easily to set up for different projects without adding much to the code, other than editing the $allowed_fields array in a config file. I thought a loop was a much better idea than a long row of if statements. It just looks horrible. 

[Psycho]

That logic is backwards. It is only validating the fields ONLY IF they exist in the POST data. You should loop over the required fields and check if they exist in the POST data. Like so

//Create array of required fields
$required_fields = array('email', 'username','password');
//Create array to hold the values
$values = array();
//Create array to hold list of empty fields
$empty_fields = array();
 
foreach($required_fields as $field)
{
    //Get value of POST data (if exists)
    $value = isset($_POST[$field]) ? trim($_POST[$field]) : '';
    //Check for empty value
    if($value=='')
    {
        $empty_fields[] = $field;
    }
}
 
//Check if any required fields were empty
if(count($empty_fields))
{
    header('Location: ../register.php?reg=false&errtype=emptyfields&field=' . implode('+', $empty_fields));
    exit();
}
 
//All required fields present continue script

Loops are a great thing, but when you are verifying data from a user, it is much better to explicitly validate the fields. In my opinion is that if a piece of data is important enough for you to ask for it and for the user to provide it, you should explicitly check for it.

[robkar32]

That logic is backwards. It is only validating the fields ONLY IF they exist in the POST data. You should loop over the required fields and check if they exist in the POST data. Like so

//Create array of required fields
$required_fields = array('email', 'username','password');
//Create array to hold the values
$values = array();
//Create array to hold list of empty fields
$empty_fields = array();
 
foreach($required_fields as $field)
{
    //Get value of POST data (if exists)
    $value = isset($_POST[$field]) ? trim($_POST[$field]) : '';
    //Check for empty value
    if($value=='')
    {
        $empty_fields[] = $field;
    }
}
 
//Check if any required fields were empty
if(count($empty_fields))
{
    header('Location: ../register.php?reg=false&errtype=emptyfields&field=' . implode('+', $empty_fields));
    exit();
}
 
//All required fields present continue script

Loops are a great thing, but when you are verifying data from a user, it is much better to explicitly validate the fields. In my opinion is that if a piece of data is important enough for you to ask for it and for the user to provide it, you should explicitly check for it.

 

Thank you   Exactly what i needed. Makes sense now, yeah, i could've probably figured this out myself now that i think about it. If only i didn't have such a headache from all this error handling today. 

 

By the way, is there a reason you are not using the "empty" function, but instead $value==''

[ginerjm]

If your goal is to write a generic login FOR YOURSELF AND YOUR APPS, why are you so worried about changing the names for what sounds like a customization?

 

Write a quality script using normal names and make it authenticate your users properly.  After that just keep using it as is, since your login credential system is likely not going to change, is it?

 

If my first a line is a bad assumption on my part, perhaps you may want to do this anyway until you get yourself a 'perfect' script and ONLY THEN worry about customizing it, although I don't  know why your html has to be altered for that purpose.

[Psycho]

By the way, is there a reason you are not using the "empty" function, but instead $value==''

 

Yes. Because when I set the value it will either be a non-empty string or an empty string (i.e. '').

 

empty() will check for 1) an empty string, 2) a variable that does not exist, or 3) a value that equals FALSE.

 

Regarding #2: The variable should exist. So, I want that line to produce an error if the variable does not exist (it means I have a bug somewhere) as opposed to assuming the value was empty.

 

Regarding #3: There are lots of values that are considered FALSE for this function. The string "0" is one of those values. I don't know if that is a legitimately value for any of your fields (especially since you say this really isn't for a login script). If a user entered a '0' into a field, using the empty() function would cause it to be considered not entered.

 

Than manual is a great place to learn these things

 

 

Returns FALSE if var exists and has a non-empty, non-zero value. Otherwise returns TRUE.

The following things are considered to be empty:

• "" (an empty string)
• 0 (0 as an integer)
• 0.0 (0 as a float)
• "0" (0 as a string)
• NULL
• FALSE
• array() (an empty array)
• $var; (a variable declared, but without a value)