Is it safe to unset PHP super-globals if this behavior is documented?

Question

I'm building a PHP framework, and in it I have a request object that parses the url as well as the $_GET, $_POST and $_FILE superglobals.

I want to encourage safe web habits, so I'm protecting the data against SQL injection, etc.

In order to ensure users of this framework are accessing the safe, clean data through the request object, I plan to use unset($_GET, $_POST, $_REQUEST); after parsing those variables.

I will document this in the method comments, and explain in the framework documentation that this is happening.

My question is: Would this be desirable behavior? What are the potential pitfalls that I have not foreseen?

Answer 1

I know this was answered already, but here's my $0.02.

I would not unset or clear the input arrays. However, what I have done is to replace them with an object. So instead of having the raw array, I replace it with an object that implements ArrayAccess and Iterator. That way the vast majority of code which uses the native arrays will still work quite well with the object.

The rationale is that at least you can verify that the code paths are operating correctly via tests. You can replace those objects with a mock object to throw an exception during testing so that you can detect improper access to those arrays (if you do determine it to be "bad practice"). So it lets you run during production without putting un-necessary restrictions, but also lets you turn it on to verify best-practices during testing.

And while I do agree with @JW about escaping, you should be filtering input. Filter-in, Escape-out. Any time data comes into your program (either via user input or from a DB), filter it to expected values. Any time data goes out (either to the DB or to the user), you need to properly escape it for that medium. So using a request object that enables easy filtering of the submitted data can be very valuable.

An example using a fluent interface (which you may or may not want):

$id = $request->get('some_id')->filter('int', array('min' => 1));

And that doesn't include the benefits of compensating for differing platforms and configurations (for example, if magic_quotes_gcp is enabled or not, etc)...

Anyway, that's just my opinion...

Answer 2

I'm not sure what the point would be of preventing access to the $_GET or $_POST arrays. There's nothing harmful in them. If you're creating a framework for preventing SQL injection or cross-site-scripting, you should escape the data when creating an SQL query or HTML document.

Escaping GET/POST data at the beginning is too early; you don't know how the data will be used, so you can't escape or encode it properly.

Having said that, you still may have some valid reasons to want people to access GET/POST data through your code. In that case, I still wouldn't unset them. You may end up incorporating third-party code which relies on them. Instead, just encourage your users to avoid them (like they should avoid global variables in general).

Answer 3

I'd maybe expose a method (maybe hidden or super counter-intuitive ;)) to get the raw data, in the off chance that your sanitization routines corrupt data in some unforeseen manner. To protect the user is one thing, but to completely lock them from their ability to retrieve data in the most raw manner may lead to frustration and, as a result, those people not using your framework :)

Answer 4

Keep in mind this increases your maintenance costs...if anything is ever added, removed or changed with the super globals in PHP, you will need to update your framework.

Answer 5

Sounds like magic_quotes style thinking. Except, at least magic_quotes was 99% reversible at runtime. Your "cleaned" data might be lossy, which really sucks.