This is not form validation — and how to fix it

I was working on a site and noticed something that concerned me.  This is fodder for TheDailyWTF and makes me worry for the fate of any person who receives the results of forms with [non-]”validation” like this.

The form element:

  1. <select name="favorite-color">
  2.  <option>Favorite Color – Choose One</option>
  3.  <option value="blue">Blue</option>
  4.  <option value="red">Red</option>
  5.  <option value="orange">Orange</option>
  6.  <option value="green">Green</option>
  7.  <option value="black">Black</option>
  8. </select>

Here is what was in the processing script:

  1. switch ($_POST["favorite-color"])
  2. {
  3.  case "blue":
  4.  case "red":
  5.  case "orange":
  6.  case "green":
  7.  case "black":
  8.   $data["favorite-color"] = $_POST["favorite-color"];
  9.   break;
  10. }
  11. //data is appended to a string – matt's comment not a "real" comment in the script
  12. $string .= $data['favorite-color'];

So… What is wrong with this? OMG what isn’t wrong with it?

  1. The select element has an “option” with no value
  2. The $data array is never initialized (I didn’t show this, but are you suprised?)
  3. The no value option is not accounted for in the processing: i.e.
    If no value is passed, then $data[‘favorite-color’] is never set
  4. The variable that would be set in processing, is then used in a string. If there is no value, PHP throws an error for accessing an array index that does not exist
  5. It is assumed that ONLY the values in the select will ever be passed to the processing and completely ignores the null value.
  6. Why in the hell would you want to type every single possible option that could be passed in the processing script? Especially when you are just going to append it to a string!!
  7. The string concatenation assumes that the variable being concatenated is already set.

So, how do we fix this?
Actually, it is not all that hard.

First, you have a choice. You can choose to add a value to the first option element, or you can handle the fact that it has no value in the processing, or you can do both (I suggest this one).

  1. <!– You can output processing errors here –>
  2. <select name="favorite-color">
  3.  <option value="0">Favorite Color – Choose One</option>
  4.  <option value="blue">Blue</option>
  5.  <option value="red">Red</option>
  6.  <option value="orange">Orange</option>
  7.  <option value="green">Green</option>
  8.  <option value="black">Black</option>
  9. </select>

Now handle the processing:

  1. //initialize the $data array
  2. $data = array():
  3. //initialize the $string variable
  4. $string = '';
  5. /**
  6.   * This code will handle the processing of the field
  7.   * It looks to see if the value is set, and not empty. It will use the non-empty value or it
  8.   * will set an empty string
  9.   */
  10. if (isset($_POST['favorite-color']) && !empty($_POST['favorite-color'])) {
  11.  $data['favorite-color'] = $_POST['favorite-color'];
  12.  /**
  13.   * if you were going to do a mysql db insert instead of string concatenation you
  14.   * would do something like:
  15.   * $data['favorite-color'] = mysql_real_escape_string($_POST['favorite-color']);
  16.   */
  17. } else {
  18.  //This is what was forgotten before
  19.  $data['favorite-color'] = ''; // or $data['favorite-color'] = null;
  20.  /**
  21.    * you could also set an error variable, which could be passed back to the form output
  22.    * to show the user what they missed or did wrong.
  23.    */
  24. }

Now that you set up your “validation” – which is really just a way to initialize the variable you will be writing to the string into the $data array.

So, all that is left is to concatenate the value to the string. You can do one of two things. The first is to just concatenate the string, since you know it has already been initialized properly:

  1. // you could add checks right here to see if there are errors and redirect back to the form before creating the string
  2. //concatenate the value to the string
  3. $string .= $data['favorite-color'];

The second way is to use a ternary operator (READ: inline if statement) to append the string:

  1. // you could add checks right here to see if there are errors and redirect back to the form before creating the string
  2. //append the string if it has a length greater than zero, otherwise append empty string
  3. $string .= (strlen($data['favorite-color']) > 0 ? $data['favorite-color'] : '');

This method of handling form elements takes a little more time, requires a little more effort up front, but it will save you WAY more trouble later on. You will not see the PHP errors that you saw before and if you are inserting the data into a DB you will prevent your users from being able to use SQL injection attacks on you.




Please consider donating Dogecoin if you like my content.

D5igT1DmhYGVMLcXhzkNdf64uEuWWhHTb3



Related Posts Plugin for WordPress, Blogger...

This entry was posted in Coding, Seriously? and tagged , , , , , , , , , , , , , , , , , , , , , , , , , , . Bookmark the permalink.