ansaurus

Question

Answer 1

A:

!is_null($_POST['personal_info_first_name']) && !isset($_POST['personal_info_first_name'])

Luis Junior 2010-07-19 22:35:53

How can something be **not null** and **not set** at the same time? Also, you want to test for `isset` before anything else.

deceze 2010-07-20 01:26:48

@deceze: `isset()` is a language construct, and won't auto-create a variable if you pass in one that doesn't exist. `is_null()` is a full-blown function, and WILL auto-create a variable (and set it to null) if it doesn't exist already. As such, `is_null()` can't be used to test for the non-existence of something, as it'll be created automatically just by testing for it.

Marc B 2010-07-21 09:04:01

@Marc The difference is that `isset` does not create the variable, but it will also not throw a warning when trying to access one that isn't defined. Therefore testing for `is_null` *first* is pointless, since it will throw a warning and make the subsequent `isset` call pointless. Also, if the variable does not exist or is `null`, the whole expression will fail at `!is_null`. If the variable exists and is not `null`, it will fail at `!isset`.

deceze 2010-07-21 10:49:40

Answer 2

+6 A:

It looks like you're trying to make sure all post variables are submitted. Would you like help with that?

Using !empty() may not be the answer to your specific question, but it would definitely help with what it looks like you're trying to do.

empty() returns TRUE if the $_POST key isn't set, if its an empty array, or even if its an empty string, so using !empty() is a good way to make sure that the user has filled in the information.

Navarr 2010-07-19 22:40:23

There is no `is_empty()`, do you mean `empty()`?

igorw 2010-07-19 22:43:58

Yeah, I already fixed that.

Navarr 2010-07-19 23:07:33

I just lol'd at the Clippy reference.

BoltClock 2010-07-21 03:56:38

Answer 3

A:

use array_key_exists('card_number', $_POST) && !empty($_POST['card_number'])

Casey 2010-07-19 22:41:32

That's pretty redundant, you can skip the `array_key_exists` call there.

deceze 2010-07-19 23:29:35

Answer 4

+1 A:

Try writing your own is_valid function and use that rather than is_null.

For example (and this is by no means comprehensive):

function is_valid(&$array, $key, $required=false) {
    if(!array_key_exists($array))
        return false;
    $value = trim($array[$key]);
    if(empty($value) && $required)
        return false;
    return true;
}

Use like so:

if(is_valid($_POST, 'personal_info_first_name', true) && ...)

thetaiko 2010-07-19 22:46:36

I'd vote this up if you fixed the error in the `array_key_exists` call, and possibly streamline the `return false`/`true` statements. :)

deceze 2010-07-20 01:24:17

Answer 5

+7 A:

"dispatch_address_postcode isn't mandatory and it will still run even if it's blank…"

Just look at that sentence again. If the field is not mandatory, it is perfectly okay if the code runs if the field is blank. If a field isn't mandatory, don't test it as mandatory.

The real problem is though, is_null only tests if the variable is null. POSTed values will never be null, if they're empty they will be '' (an empty string). All your !is_null tests will always be true, and you will get a warning if the variable isn't set (something you don't want to happen). The more appropriate test would be !empty.

Even more appropriate tests would include a test if the value appears to be valid (does email look like an email address, does telephone have at least x digits in it?). You should also loop through the fields to make your code more readable, endless nested and chained if conditions are no joy to look at.

$mandatoryFields = array('foo' => 'email', 'bar' => 'telephone');

foreach ($mandatoryFields as $field => $rule) {
    if (empty($_POST[$field]) || !validateByRule($_POST[$field], $rule)) {
        raiseHell();
    }
}

deceze 2010-07-19 23:27:53

Though, undefined variables _will_ pass as null too.

zneak 2010-07-19 23:30:29

@zneak Good catch, revised that statement.

deceze 2010-07-19 23:32:08

Answer 6

A:

Edit: Please consider this before a downvote. I'm leaving this here to serve as a "what not to do". I would delete it because it's bad, but then nobody would learn from my mistakes.

DO NOT DO THIS - read the comments for great info on why this is bad

My answer is going to be wildly different, but I am a wildly different guy...

I JUST found that this will work. Instead of all that isset and things, just assign the variables programmatically! I think I have some refactoring to do... y'know on all my code...

if (!is_array($_POST)){exit "$_POST isn't an array";}
foreach ($_POST as $param => $value){
    ${$param} = secure($value);
}

//now you have a set of variables that are named exactly as the posted param
//for example, $_POST['personal_info_first_name'] == $personal_info_first_name

if ($payment_method == "Pay by credit card."){
    //do stuff that you were gonna do anyways
} else if ($payment_method == "Pay by new credit card.") {
    if ($card_number && $expiration_date && $security_code){
        //do stuff that you were gonna do anyways
    } else {
        exit("info missing for credit card transaction");
    }
} else {
    exit("unknown payment method")
}

function secure($input){
    //sanitize user input
}

If you use this code, then it doesn't matter what is null and what isn't within the foreach because anything that's null just won't be made. Then you can use nicer looking code (and probably faster code) to check for anything that is required.

Tim 2010-07-19 23:35:17

The first line won't do much... $_POST is always an array, even in the PHP CLI, unless you've done something crazy to it earlier in the program. But how is this an improvement, anyway? Instead of checking for an array key that is not set, you'll end up checking variables that are not set?

Alex JL 2010-07-20 00:08:51

You'll get a lot of warnings about non-existent variables with code like this, you'll still need to use `isset`. You're also littering your namespace with tons of variables and will have to take more care with initializing them, you're essentially re-creating the old `register_globals` problem. No improvement at all.

deceze 2010-07-20 01:10:32

oh well... I guess my exuberance was a bit premature. Thanks for pointing out the flaws though. I didn't realize that this was a problem, it just seems so easy to use for this sort of thing. I guess this is why your reps are in the thousands and mine is in the hundreds :opTo be clear, though. It looks like the issue with non-existent variables is after the foreach loop? Would creating variables like I did with the foreach loop be ok if I didn't make some assumptions in the code afterward?

Tim 2010-07-20 02:17:07

Indeed, the problem is in the `if ($payment_method ...)` parts. If `payment_method` was not part of the `$_POST` array, the variable won't exist. It's the same problem whether you create variables using your `foreach` or just leave them in the `$_POST` array. Leaving variables in the `$_POST` array is much cleaner, since they only take up a single variable. If you start using a variable `$foo` for internal purposes, but your `$_POST` array also contains a key `'foo'`, you get into a bit of a naming conflict, possibly with security implications. That's why `register_globals` was deprecated.

deceze 2010-07-20 03:55:47

Right... someone could actually clobber your own variables by sending unexpected fields in the POST input, which would be a threat to security and sanity, exactly like `register_globals`.

Alex JL 2010-07-20 04:11:00

Thanks for all the input guys, I really appreciate it. This kind of info while learning is invaluable.

Tim 2010-07-20 21:29:22

ansaurus

tags:

views:

answers:

!is_null() not working as expected

related questions