Restrict field values to either 1 or 0 in PHP

[VanityCrush]

I am trying to restrict the value entered for the form field 'active' to either 0 or 1, 0 being inactive and 1 being active

The code that is inserting the user in the database:

if ($_POST['active'] != '1' && $_POST['active'] != '0') {
    $errors[] = 'Active field can only take values of either 1 or 0';
} else {
        if (empty($_POST) === false && empty($errors) === true) {
            $register_data = array(
                'username'      => $_POST['username'],
                'password'      => $_POST['password'],
                'first_name'    => $_POST['first_name'],
                'email'         => $_POST['email'],
                'active'        => $_POST['active'],
                );
            admin_add_user($register_data);
            header('Location: add_user.php?success');
            exit();
        } 
        else if (empty($errors) === false) {
            echo output_errors($errors);
        }
}

The function:

function admin_add_user($register_data) {
    include('core/db/db_connection.php');
    array_walk($register_data, 'array_sanitize'); 
    $register_data['password'] = md5($register_data['password']);
    $fields = '`' . implode('`, `', array_keys($register_data)) . '`'; 
    $data = '\'' . implode('\', \'', $register_data) . '\''; 
    $sql = "INSERT INTO `_users` ($fields) VALUES ($data)";
    $query = mysqli_query($dbCon, $sql);
}

How can I restrict the values entered for the form field named 'active' to 1 or 0? I have tried 1 and 0 without the apostrophes but I still get the error associated with this if statement. Whenever I use 1 as a field value it works, however when using 0 it does not register it and the form field acts like it is blank.

 

Any suggestions?

[CroNiX]

 

if ($_POST['active'] != '1' && $_POST['active'] != '0') {

 

Your condition here should be OR, not AND. AND means it has to be 1 AND 0 at the same time, which it obviously can't. It can be 1, OR can be 0.

 

Another thing, what type of form control is named 'active'? Is it a checkbox? If so, only checked values get submitted with the form, not unchecked values. This is an HTML standard having to do with "successful form controls".

 

For checkboxes, I usually do something like:

<input type="checkbox" name="active" value="1">

 

And then when checking:

$active = (isset($_POST['active'])) ? 1 : 0;

 

If active was checked it will be set, and since we know it's value can only be 1, we just set it to 1. You could just as well use $_POST['active'] there forcing it's value to an int or something.

If active wasn't checked, it won't be set so we set the value to 0.

[VanityCrush]

That's the way I tried it in the first place - || instead of && - but I was encouraged to use AND. Using OR it won't work at all.

 

The form field I have is not a checkbox, but a regular text field. I guess this might be the problem and a checkbox would be more appropriate in this case, as you suggested. I'll try it out and let you know. Thanks

[VanityCrush]

Using the checkbox approach solved my issue and I have modified the code like this:
 
Required fields:

if (empty($_POST) === false) {
	$required_fields = array('usernamne', 'password', 'password_again', 'first_name', 'active', 'email');
	foreach ($_POST as $key => $value) {
		if (empty($value) && in_array($key, $required_fields) === true) {	
			$errors[] = 'Fields marked with * are mandatory';
			break 1;
		} 
	}

The code that is adding the user to the database:

if (empty($_POST) === false && empty($errors) === true) {
			$register_data = array(
				'username' 		=> $_POST['username'],
				'password' 		=> $_POST['password'],
				'first_name' 	=> $_POST['first_name'],
				'last_name' 	=> $_POST['last_name'],
				'email' 		=> $_POST['email'],
				'active'		=> isset($_POST['active']) ? 1 : 0,
				);
			admin_add_user($register_data);
			header('Location: add_user.php?success');
			exit();
		} 
		else if (empty($errors) === false) {
			echo output_errors($errors);
		}

Didn't even cross my mind that I could just simply use a checkbox, I guess I was too focused on my approach, haha. Thank you

Edited August 11, 2015 by VanityCrush

[ginerjm]

Checking if $_POST is empty is a pretty meaningless check. What are you hoping it will tell you? Upon receiving posted input one usually checks for the button value that was checked and then proceeds to handle the input data that that button should be providing. You're checking if there is anything at all in $_POST and then immediately proceeding to use the input provided without checking if all (or any) of the inputs are valid or even present.

 

As for your first statement that Cronix disputed - it seems correct to me. You ask if 'active' is NOT 1 and if 'active' is NOT 0 and if true you prepare an error message. Seems valid to me!

[VanityCrush]

The reason why I'm checking if $_POST is empty is because I want to display an error to the users that are trying to submit the form without inserting any data. Does that make sense ?

[mac_gyver]

this bit of code has several problems -

if (empty($_POST) === false) {
    $required_fields = array('usernamne', 'password', 'password_again', 'first_name', 'active', 'email');
    foreach ($_POST as $key => $value) {
        if (empty($value) && in_array($key, $required_fields) === true) {    
            $errors[] = 'Fields marked with * are mandatory';
            break 1;
        }
    }

1) you should not blindly loop through the $_POST data. a hacker can submit 100's or 1000's of values. you should instead loop through the definition of your fields and test the corresponding $_POST data. also, unchecked checkboxes and unselected radio button groups are not set in the $_POST data, so your current logic won't detect if a required checkbox/radio button choice hasn't be selected.

 

2) a zero is considered to be empty(), which is probably why your zero case wasn't working.

 

3) by breaking out of the loop upon the first empty value, you are only reporting a general error. you should specifically produce an error for each required field that was empty.  this will help the visitor know what he left empty (yes, they need to be specifically told), and it will help with programming errors/typo's by reporting exactly which fields the code thinks was left empty.

 

4) this programming style is a killer - empty($_POST) === false. the empty() function is designed to return a boolean value that you directly use in your program logic. by adding another comparison operator to it, you are building up a forest of unnecessary code.

[VanityCrush]

Hello mac_gyver, thank you for your input. I am in fact displaying an error for each required field that was empty. The above was just a snippet of my code. 

include('core/init.php'); 
not_logged_in_redirect();
admin_protect();

if (empty($_POST) === false) {
	$required_fields = array('usernamne', 'password', 'password_again', 'first_name', 'active', 'email');
	foreach ($_POST as $key => $value) {
		if (empty($value) && in_array($key, $required_fields) === true) {	
			$errors[] = 'Fields marked with an asterisk are required';
			break 1; // breaks to foreach (if 1 error is found, we can't do anything else so there's no point for checking further)
		} 
	} // iterating through our post data - doing a check on each one

	if (empty($errors) === true) {
		if (user_exists($_POST['username']) === true) {
		$errors[] = 'Sorry, the username \'' . $_POST['username'] . '\' is already taken. Try ' . $_POST['username'] . rand(7, 77) . ' instead.';
		}
		if (preg_match("/\\s/", $_POST['username']) == true) { // regexp checking for any amount of spaces within the username 
			$errors[] = 'No spaces allowed.';
		}
		if (!ctype_alnum($_POST['username'])) {
			$errors[] = 'No special characters allowed';
		}
		if (strlen($_POST['password']) < 7) {
			$errors[] = 'Your password must be at least 7 characters long.';
		}
		if ($_POST['password'] !== $_POST['password_again']) {
			$errors[] = 'Your passwords do not match.';
		}
		if (filter_var($_POST['email'], FILTER_VALIDATE_EMAIL) === false) {
			$errors[] = 'The email address entered is not valid.';
		}
		if (email_exists($_POST['email']) === true) {
			$errors[] = 'Sorry, the email \'' . $_POST['email'] . '\' is already in use.';
		}
		if (empty($_POST['captcha_results']) === true) {
			$errors[] = 'Please enter captcha.';
		} else if ($_POST['captcha_results'] != $_POST['num1'] + $_POST['num2']) {
			$errors[] = "Incorrect captcha.";
		}
	}
}
// print_r($errors);

if(isset($_GET['success']) && empty($_GET['success'])) {
		echo '<body>
				<div id="body_wrap">
					<nav id="nav">
						<ul>
							<li><a href="recom.php">recommendations</a></li>
							<li><a href="blog.php">blog</a></li>
							<li><a href="includes/logout.php">log out</a></li>
							<li><a href="admin.php">admin page</a></li>
							<li><a href="../ro/login.php">ro</a></li>
						</ul>
						<div id="logo"><a href="index.php"><img src="img/provisory-logo.gif"></a></div>
					</nav>';
		echo '<h3 class="reg_success">User added successfully!</h3>';
	} else {
		if (empty($_POST) === false && empty($errors) === true) {
			$register_data = array(
				'username' 		=> $_POST['username'],
				'password' 		=> $_POST['password'],
				'first_name' 	=> $_POST['first_name'],
				'email' 		=> $_POST['email'],
				'active'		=> isset($_POST['active']) ? 1 : 0,
				);
			admin_add_user($register_data);
			header('Location: add_user.php?success');
			exit();
		} 
		else if (empty($errors) === false) {
			echo output_errors($errors);
		}
}

Is this any better?

[mac_gyver]

The reason why I'm checking if $_POST is empty is because I want to display an error to the users that are trying to submit the form without inserting any data. Does that make sense ?

 

that's not what testing if $_POST is empty or not does. testing if $_POST is empty or not detects if the form was submitted. it's testing if any of the named form fields are present, even if they are empty, in the $_POST array.

 

$_POST will be empty if no form was submitted or a form was submitted and the total size of the form data exceed the max_post_size setting.

[mac_gyver]

Is this any better?

 

 

no. that looks the same for the empty test.

 

your current logic only reports that at least one of the required fields was left empty. not which one(s).

 

your code also prevents the rest of the validation tests from running if there are any empty fields. there could be problems with the non-empty fields, but the user won't see those errors until they fill in the empty fields. this will result in an unhappy user, because he may have to submit the form extra times, when he could have corrected all the errors at once.

[VanityCrush]

I see where you're coming from. thanks for the suggestions, really appreciate the feedback.