strlng

[cluelessPHP]

I'm working on this currently

if (count($uname)) {

if (empty($uname) ) {
 $nameErr = "Userame is required";
   }
  else if (strlen((string)$uname[0]) < 3) {
      $nameErr = "Userame must be longer than three letters";
   }
   else if(strlen((string)$uname[0]) > 15) {
     $nameErr = "You need a username shorter than fifteen characters";
   }
}

However the error message is consistently showing, I'm not sure where I'm going wrong

 $nameErr = "Username must be longer than three letters";

[Bonesnap]

So it appears $uname is an array (any reason for that?).

This might offer some insight:

PHP Manual wrote:

Arrays are always converted to the string "Array";

Link

Off the top of my head I can't remember if you can implicitly add an array deference to the end of a string in PHP ($uname[0]). If you can, then you are essentially getting "A", which is of course less than 3 characters and thus explains the error you are getting.

[cluelessPHP]

I went for this solution in the end, need to read the manual more

if (empty($uname)) {
		$nameErr = "Userame is required";
	}
	else
	if (strlen($uname)>15) {
		$nameErr = "You need a username shorter than fifteen characters";
	}
	else
	if (strlen($uname) < 3)
	{
		$nameErr = "Userame must be longer than three letters";
	}

[Bonesnap]

Your message '"Userame must be longer than three letters"' is slightly incorrect as your condition is checking if the username is LESS THAN 3, not less than 4. You could change it to <= 3 or simply < 4 to get the effect you're looking for.

However, you could really just simplify the whole thing and do:

if(strlen($uname) <= 3 || strlen($uname) >= 15) {
    $nameErr = "Usernames must be between 4 and 14 characters";
}

If $uname is empty it will be less than 3. Would also be worth it to trim() $uname before the length comparisons in case user just hits the space bar (and I don't mean to get a drink 🙂).

[laserlight]

It would be better to define named constants and use them instead of magic numbers like 3, 4, 14 or 15, e.g.,

if (strlen($uname) < USERNAME_MIN_LEN || strlen($uname) > USERNAME_MAX_LEN) {
    $nameErr = sprintf("Usernames must be between %d and %d characters", USERNAME_MIN_LEN, USERNAME_MAX_LEN);
}

[cluelessPHP]

	
	$nameErr = $fnameErr = $snameErr = $emailErr = $passwordErr = $passwordConfirmErr = $phoneErr = "";
	$uname = $fname = $sname = $email = $gender = $password = $passwordConfirm = $phone = "";


if ($_SERVER["REQUEST_METHOD"] == "POST"){

$uname = $_POST['uname'];
//	$fname = $_POST['fname'];
//	$sname = $_POST['sname'];
//	$email = $_POST['email'];
//	$password = $_POST['password'];
//	$passwordConfirm = $_POST['passwordConfirm'];
//	$bio = $_POST['bio'];

//$phone = $_POST['phone'];
//$salt = Hash::salt(32);


function test_input($data) {
   $data = trim($data);
   $data = stripslashes($data);
   $data = htmlspecialchars($data);
   return $data;
}


if ($_SERVER["REQUEST_METHOD"] == "POST") {
   /*
   if (empty($_POST["uname"]) ) {
     $nameErr = "Userame is required";
   }

   if (count($uname)) {

if (empty($uname) ) {
 $nameErr = "Userame is required";
   }
  else if (strlen((string)$uname[0]) < 3) {
      $nameErr = "Userame must be longer than three letters";
   }
   else if(strlen((string)$uname[0]) > 15) {
     $nameErr = "You need a username shorter than fifteen characters";
   }
}
   */

   if (empty($uname)) {
		$nameErr = "Userame is required";
	}
	else
	if (strlen($uname)>15) {
		$nameErr = "You need a username shorter than fifteen characters";
	}
	else
	if (strlen($uname) < 3)
	{
		$nameErr = "Userame must be longer than three letters";
	}
     $uname = test_input($_POST["uname"]);

 if (!preg_match('/^[\p{L}\p{N} .-]+$/', $uname)) {
   $nameErr = "Only letters and white space allowed"; 
 }

if (empty($_POST["fname"])) {
 $fnameErr = "What is your name?";
   } else {
     $fname = test_input($_POST["fname"]);

 if (!preg_match("/^[a-zA-Z ]*$/",$fname)) {
   $nameErr = "Only letters and white space allowed"; 
 }
   }

if (empty($_POST["sname"])) {
 $snameErr = "What is your surname?";
   } else {
     $sname = test_input($_POST["sname"]);

 if (!preg_match("/^[a-zA-Z ]*$/",$fname)) {
   $nameErr = "Only letters and white space allowed"; 
 }
   }

   if (empty($_POST["phone"])) {
     $snameErr = "What is your number?";
   } else {
     $phone = test_input($_POST["phone"]);

 if (!preg_match("/^[1-9]\d{0,3}$/",$phone)) {
   $phoneErr = "Only numbers allowed"; 
 }
   }

  if (empty($_POST["password"])) {
     $passwordErr = "You must enter a password";
   } else {
     $password = test_input($_POST["password"]);

 if (!preg_match("/^[a-zA-Z ]*$/",$fname)) {
   $nameErr = "Only letters and white space allowed"; 
 }
   }

if (empty($_POST["email"])) {
 $emailErr = "Email is required";
   } else {
     $email = test_input($_POST["email"]);

 if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
   $emailErr = "Invalid email format"; 
 }
   }

if (empty($_POST["password"])) {
 $passwordConfirmErr = "You must enter a password";
   } else {
     $passwordConfirm = test_input($_POST["password"]);

 if (!preg_match("/^[a-zA-Z ]*$/",$fname)) {
   $nameErr = "Only letters and white space allowed"; 
 }
   }

else{
$sql = "INSERT INTO user (`userName`, `eMail`, `password`, `dateJoined`, `forename`, `surname`, `bio`,`geneder`, `telephone`) VALUES (:uname, :email, :password, NOW(), :fname, :sname, :bio, :sex, :phone) ; ";
$query = $db->prepare($sql);

	$query->execute(array(
		':uname'=> $uname,
		':email'=> $email,
		'password' => Hash::make(Input::get('password'), $salt),
		':fname'=> $fname,
		':sname'=> $sname,
		':bio'=> $bio,
		':sex' => $sex,
		':phone' => $phone
	));

}
}

}

That is currently the full code, woring on it when I get free time, need to delete bits and clean it up

[laserlight]

Your test_input function is poorly named and does too much:

• You probably should rename it to clean_input, or something along those lines. The word "test" does not convey what you apparently intend for it.

• stripslashes is a bad idea: you should only use it if magic_quotes_gpc is on, otherwise you would potentially be corrupting the input.

• htmlspecialchars is a good idea, but by using it on input prior to storage to a database, it fixes your data in HTML format. Rather, unless you are absolutely certain that the data will always be in HTML format, you should store the data as-is, then use htmlspecialchars when you want to output it in HTML.

Furthermore, you should call the function as early as feasible, after checking with empty, e.g.,

if (empty($_POST["uname"])) {
    $nameErr = "Username is required";
} else {
    $uname = clean_input($_POST["uname"]);

if (strlen($uname) >= USERNAME_MAX_LEN) {
    $nameErr = sprintf("You need a username shorter than %d characters", USERNAME_MAX_LEN);
} elseif (strlen($uname) <= USERNAME_MIN_LEN) {
    $nameErr = sprintf("Userame must be longer than %d letters", USERNAME_MIN_LEN);
} elseif (preg_match('/^[\p{L}\p{N} .-]+$/', $uname) == 0) {
    $nameErr = "Only letters and white space allowed"; 
}
}

Notice that my code is properly indented so as to show its logical structure. I have also used the named constants USERNAME_MAX_LEN and USERNAME_MIN_LEN that you should define earlier on. I am also careful with edge cases, e.g., the use of >= and <= instead of > and < because your error messages speak of "need (...) shorter than" and "must be longer than".

[Bonesnap]

Some things I noticed from reading your full script:

• You're testing the password and restricting what characters are accepted. This is a very bad practice, especially since you are restricting the password to letters and spaces only. I mean... they can't even use a number? You're basically forcing your passwords to be very weak. Testing to make sure they entered a password is normal and expected, but to restrict them is not a good idea.

• You really shouldn't sanitize or change password input at all (basically the password should not be subjected to your test_input() function). The value from the password field should be fed directly into a hashing algorithm of some kind (AKA, password_hash with the BCRYPT flag) and therefore will not cause any problems for SQL injection when you eventually add it into the database.

• You are actually checking the same $POST variable when you are checking for the password confirmation. You are checking $POST['password'] twice. I assume that's just a typo.

• +1 for using filter_var() instead of some convoluted (and often incorrect) regular expression to check emails.

[cluelessPHP]

Currently this should be viewed as early Alpha stages, only had an half an hour to work on it a day, hopefully I can improve all of it later today based on the suggestions made by both of you 🙂

[sneakyimp]

OK so there is a lot of good advice here, but I think the fundamental problem is that you are confused about whether $uname is a string or any array. Looking at this code:

if (count($uname)) {

if (empty($uname) ) {
 $nameErr = "Userame is required";
   }
  else if (strlen((string)$uname[0]) < 3) {
      $nameErr = "Userame must be longer than three letters";
   }
   else if(strlen((string)$uname[0]) > 15) {
     $nameErr = "You need a username shorter than fifteen characters";
   }
} 

I'm not sure but I suspect that $uname should just be a string -- you could possibly have defined your HTML to submit uname as an array but I tend to doubt that you have. You should not ever be checking [man]count[/man] of any variable that is not an array. It doesn't really make sense u

The secondary problem is that PHP lets you handle strings as though they were arrays:

$uname="foo"; // define it as a string
echo count($uname) . "\n"; // perhaps confusingly, this outputs 1 for some reason...PHP is a weakly typed language
echo (string)$uname[0] . "\n"; // this outputs f because PHP lets you treat strings as arrays. 

So basically your strlen checks were on a single character, f, which has a length of 1.

[Bonesnap]

Looks like he's changed it to a string now.