Is this class unsafe to use in my public PHP-scripts?

[Markus Igeland]

I have no idea if it's safe to use or not, so I have to ask you.

	class Requests {
		private $type;
		public function __construct($type='') {
			$this->type = strtolower($type);
			$this->createObject();
		}
		private function createObject() {
			switch($this->type) {
				case 'post':
					$arr = $_POST;
					break;
				case 'get':
					$arr = $_GET;
					break;
				default:
					$arr = array_merge($_POST, $_GET);
			}
			foreach ($arr as $key => $value) {
				$this->$key = $value;
			}
		}
	}

Haha, I'm not able to explain how this code works in plain english, but I can explain in the following example:

$post = new Request('post'); //makes me use all $_POST-variables as an attribute of $post

/*
Say the variables stored in $_POST are the following:
key1 => value1
key2 => value2
key3 => value4
key4 => value3
*/

echo $post->key1; // prints "value1"
echo $post->key2; // prints "value2"
echo $post->key3; // prints "value4"
echo $post->key4; // prints "value3"

I need to know if this code is (un)safe to use in my public PHP-scripts, as I think it's easier than always doing this:

$var1 = $_POST['key1']; //so that $var1 = value1

[code green]

Well it is safe because it will not work.
Have you tested this? What do you hope it does?

[Dormilich]

it’s not unsafe per se, it’s just overcomplicatin g things. it is indeed way more simple to just use the superglobals or a union of them ($input = $_GET + $_POST; // or $_POST + $_GET, depending on which should have precedence) (superglobals are always an array, which might be just empty)

[Markus Igeland]

@green: Yes, it works.

@Dormi: In what way is it overcomplicatin g things?

[Dormilich]

it is overcomplicatin g in that way, as you have to be aware of scope. while $_GET & $_POST are available everywhere in your script (they are superglobals, after all) any variable (be it a scalar or object) adheres to PHP’s scope rules (a variable is only visible inside the scope it was defined in). thus to use $post->key you first have to define $post, and that in every different scope (e.g. in every function you need it).

this becomes less of a problem, if the object does more than just fetching the data (e.g. validating the data).

[dgreenhouse]

I'd say it could be very unsafe if you don't sanitize the data before you use it.

See:

SQL injection attacks:

http://php.net/manual/en/security.database.sql-injection.php

PHP code injection attacks:

http://www.theserverpages.com/articles/webmasters/php/security/Code_Injection_Vulnerabilities_Explained.html

And yes...

It's a bit over complicated; unless you include data sanitizing and validation within the class...
There are also other ways of determining the type of request - POST or REQUEST.