php evaluation sequence of if-statements

Question

I'm having trouble with sanitizing user input, where I don't know, if POST-Data has been sent or not. When I just do this to check:

if ($_POST["somedata"] == 2)
  DoSomething();

I will get an error notice with error_reporint = E_ALL, that $_POST["somedata"] may not be defined (e.g. when the page is loaded without the form).

So I do this check now:

if (isset($_POST["somedata"]) && $_POST["somedata"] == 2)
  DoSomething();

This doesn't output an error but it looks very unstable to me. I'm not sure if I just have luck with my PHP-Version or the simplicity of the statement. Is it also safe to use, when the if-statement is more complex, as long as the order of these two items are the same? Is it safe to use with all PHP-Versions?

Answer 1

Using isset in combination with a lazy-and (&&) is relatively correct (it prevents strictness warnings). A more advanced way would be to have automatic input checking against a schema or model.

You could have:

$schema = array(
    "somedata" => "number"
);

Using the schema approach requires a little bit of architecture but it removes the instability that you might be worried about.

---

One thing worth mentioning is that there is a difference between validating input on a syntax level (did I get all the required inputs) and input validation on a semantic level. Let's say you have a service called GetItem and you pass id = 3 the syntax checker will check for the presence of the id property and that it is a number. Then you need to check whether 3 actually exists.

So rather than returning invalid request (bad input) you might want to return no such item.

Answer 2

Your approach is functionally correct, as the && is short-circuited if the first term evaluates to false. Regarding stability, as you suspected, there is a small concern, related especially to code duplication, mainly you need to always pay care that both keys match the same string (it is appropriate to say this is a matter of code duplication).

One approach to this would be to define a function that does the check and returns the contents at the desired key if exist, like for example:

function arr(array $array, $key, $defaultValue=false) {
    return isset($arrray[$key]) ? $array[$key] : $defaultValue            
}

You pay a small performance penalty for calling a function (you can reduce it by removing the type hinting for the $array parameter), however you have a more stable code.

Usage example in your case:

if(arr($_POST, 'somedata') === 2)) {
    // do your stuff
}

Answer 3

We've been using this at work for some time now, going back years in code. It's fine to use since it first checks it the variable is actually there, and only then checks if it has a value.

Answer 4

Your code snippet is correct.

    if (isset($_POST["somedata"]) && $_POST["somedata"] == 2)
            DoSomething();

I would highly encourage you to use the triple equals for your quality check. Check out How do the PHP equality (== double equals) and identity (=== triple equals) comparison operators differ? for more info on equality checks.

The triple equal means it needs to be an integer and not just a string, good practice to get into doing this.