Proper variable checking with GET & POST variables

Question

When checking that variables passed via GET and POST are correct, I might have something like this:

<?php
//Controller
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
  if(!isset($_POST['new_email']))
    header('Location: somepage.php');
  else if(empty($_POST['new_email']))
    //Report error to user and prompt to try again
  else
    $newEmail = $_POST['new_email'];

  if(!isset($_POST['full_name']))
    header('Location: somepage.php');
  else if(empty($_POST['full_name']))
    //Report error to user and prompt to try again
  else
    $newName = $_POST['full_name'];

  if(!isset($_POST['new_password_a']))
    header('Location: somepage.php');
  else if(empty($_POST['new_password_a']))
    //Report error to user and prompt to try again
  else
    $newPasswordA = $_POST['new_password_a'];

  if(!isset($_POST['new_password_b']))
    header('Location: somepage.php');
  else if(empty($_POST['new_password_b']))
    //Report error to user and prompt to try again
  else
    $newPasswordB = $_POST['new_password_b'];

  //Do some things with the variables
}
else
{
  header('Location: somepage.php');
}

//View
//Display relevant view here
?>

How would you check GET and POST variables in your PHP script? I wonder if there is a better way?

Answer 1

Maybe creating a function to avoid the repeated code?

function check($varname,$destination,$message) {
    if (!isset($_POST[$varname])) {
        header("Location: $destination");
    } else if (empty($_POST[$varname])) {
        //Do something with $message
    } else {
        return $_POST[$varname];
    }
    return NULL;
}

And then,

<?php
//Controller
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
  $newEmail = check('new_email','somepage.php','Error message');
  $newName = check('new_name','somepage.php','Error message');
  $newPasswordA = check('new_password_a','somepage.php','Error message');
  $newPasswordB = check('new_password_b','somepage.php','Error message');

  //Do some things with the variables
  //Checking for NULL values (although if some var was null, 
  //it should have either redirected or reported an error)
}
else
{
  header('Location: somepage.php');
}

//View
//Display relevant view here
?>

What The Pixel Developer says is true though, you should sanitize the inputs at least against SQL injection (if you will use the data in a database) and CSRF attacks.

Answer 2

<?php
//Controller
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
  foreach ($_POST as $key => $value) {
    if (empty($value)) {
      echo 'whoops, remember to set ', $key;
    } else {
      switch($key) {
        case 'new_password_a':
          $newPasswordA = $value;
          break;
        //etc
      }
    }
  }
  if (isset($newPasswordA) && isset($newPasswordB)) { //check all vars have been set or whatever
    header('Location: somepage.php');
  } else {
    header('Location: somepage.php');
  }

Sorry I couldn't be more specific with the code, your sample code was kinda vague. I hope that helps.

Answer 3

Your code is a wild mess for a start. Please use brackets, better code comments and classes / functions.

You're not checking for anything correct other than if the key has a value. You might want to add a CSRF token to make sure the request has come from the form you are expecting.

Look at CSRF on Wikipedia.