PHP Superglobals – Directly Modifying Superglobals

PHP

I've seen people (who generally write good code) directly alter the $_POST array with code like this:

// Add some value that wasn't actually posted
$_POST['last_activity'] = time();

// Alter an existing post value
$_POST['name'] = trim($_POST['name']);

// Our pretend function
// Pass the entire $_POST array as data to work with in the function
// The function update_record() will read only the values we actually need
update_record($_POST);

// ...That sure was easier than creating a new array 
//  with only the $_POST values we actually need.

It makes sense that update_record() should not access $_POST directly, so we can pass other arrays of data to it for instance, but surely this is lazy, bad design, or possibly just wrong? However, we are still passing a valid array to update_record(), so why create a new one?

This is not the point of the question, just an example of usage. However, I have heard plenty of people say that this should not be done with $_REQUEST data, and it's bad practice. But why? Looks harmless enough.

Examples:

  • Setting a default $_GET (or post) value that doesn't really exist

  • Adding $_POST values that weren't actually posted after a form submission

  • Directly sanitizing or filtering the $_GET array values or keys very early in the script (fallback sanitation… why not?)

  • Setting a $_POST value manually before form submission to populate an input with a default value (when the input reads $_POST for it's default value; I have done this)

  • Making up your own $_SERVER values? Sure, hey why not?

  • How about the others, like $_COOKIE and $_SESSION? Of course we have to modify those directly right? Then why not the others?

Should direct modification of superglobals never be done, or is it OK to do in some instances?

Best Answer

Given that PHP is already setting those superglobals, I don't think it's evil to modify them. In some cases, it may be the best way to solve problems... particularly when dealing with third party code that you cannot easily modify. (They might use $_GET directly or assume some key exists in $_SERVER, etc.)

However, generally speaking, I think it's a bad practice when you are writing your own code. Modifying the $_REQUEST data with some behind the scenes filter that runs on every page automatically is likely to introduce side effects. (See all the problems that "magic quotes" caused for proof.)

So if you aren't going to do that (automatically filter the superglobals), then the following doesn't give you any benefits: 

$_POST['foo'] = filter($_POST['foo']);

when you can easily just do:

$foo = filter($_POST['foo']);

I think it's much more clear to make the site-wide distinction that $_POST and $_GET are always unfiltered, untrusted data, and they should never be used as-is.

By copying the filtered value to another variable, you are making the claim that, "I understand what I'm doing... I've filtered this input, and it's safe to use."

Related Topic