Directly modifying superglobals

Question

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?

Answer 1

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."

Answer 2

I would generally suggest that you shouldn't modify the pre-defined super-globals so that it's clear what is sanitised data and what's raw/untrusted data.

Others might suggest that if you clean up the superglobals at the start of the request cycle then you don't need to worry about them elsewhere.

I'd always match them out when you need them with:

$id = (int)$_POST['id'];

or similar.

In terms of the other variables it's good practice to not write to any of $_GET, $_POST, $_REQUEST, $_SERVER or $_COOKIE. $_SESSION however is different because you often want to write data into the session that's then persisted across different requests in the session.

Answer 3

You should avoid it. Maybe some time you forgot to sanitize something, then you can retrieve dangerous data. If you copy the data into a new structure while sanitizing

• You only get, what you want/need and not what is in $_POST too
• You will probably get an error, if the newly created array is missing some keys or is missing at all

Additional other scripts may assume, that the array is untouched and may react curious.

Answer 4

I've never liked the idea of modifying superglobal because it is misleading. It is a quick hacky way to do something that there will almost likely be a better way to do.

If you change the value of $_POST, for example, then you're saying that software received data that it did not.

THE REAL PROBLEM

There is a real life situation where this becomes a big problem:

Imagine you're working in a team. In an ideal world, everyone uses the same syntax, but we don't live in an ideal world. One developer, John, likes to access posted data using $_POST. He changes something in the post vars:

$_POST['ranking'] = 2; // John has changed ranking from 1 to 2 for whatever reason

Then you have another developer, Chris, who prefers to use filter_input to access inputted (i.e. GET, POST, SERVER, COOKIE) data as it goes to protect the software when processing data that the user can tamper with. In his part of the software, he needs to get the post value of ranking. His part of the code is AFTER John's.

$ranking = filter_input(INPUT_POST, 'ranking', FILTER_SANITIZE_NUMBER_INT);
// $ranking = 1

From the example above, by changing a superglobal, you have broken PHP. John has set the value of $_POST['ranking'] to 2 for whatever reason, but now Chris has received a value of 1

When I've seen no other way to do it:

I worked on a project that used wordpress as its blog behind an AWS load-balancer. This changes the value of $_SERVER['remote_address']. In this instance, the other developer had no choice but to do the following:

if (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
    $parts = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
    $_SERVER['REMOTE_ADDR'] = $parts[0];
}

Conclusion

There's almost certainly a better way than to change superglobals

Answer 5

I think the real question here is “why should you modify theme?”. I don't see any valid reason to do so. If you need to sanitize an imput, you might want to use a local variable…

Unless you code is short enough (say, less than 50 lines long), modifying those super-global would only make your code harder to maintain and to undersand.

By the way you don't need to pass $_POST to the function, since it's a superglobal array that can ben accessed even within the local scope of a function.

Answer 6

After initially answering this question by saying there should be no reason to modify superglobals, I'm editing this answer with an example of a time when I've decided to.

I'm currently working on a URL rewrite database table whereby the request column directs the user to its corresponding target column.

For instance, a request might be blog/title-here and its target might be blog.php?id=1.

Since blog.php is expecting $_GET variables, and I don't want to change the header("Location:"), I'm left doing something like this:

$uri    = explode('?', $uri_request)[0];
$params = explode('?', $uri_request)[1];
parse_str($params, $_GET);

This creates a $_GET array containing the intended parameters passed by the target column.

Ultimately, I would strongly advise against modifying superglobals unless you absolutely have to.