How to correctly handle $ _GET variables in PHP?

Question

I currently have the following code:

if(isset($_GET['mid']) && !empty($_GET['mid'])) { $mid = $_GET['mid']; if(is_numeric($mid) && $mid > 0) { if(isset($_GET['op']) && !empty($_GET['op'])) { $op = $_GET['op']; if($op == 'info') { } if($op == 'cast') { } } } } 

But I think it is too "complicated" with if statements inside if statements, etc.

Could you do otherwise? How would you make it easier?

[EDIT] Accepted answer:

Well, I learned a few small details and new PHP features that I did not know about. I do not think there is a right way to do what I asked. I have explicitly used some PHP functions incorrectly and I will fix this.

It seems to me that inputs like this should be checked / processed using PHP filter functions, so I mark Ark's answer as accepted.

However, for a specific university project (which the PHP code is completely irrelevant) I am going to use a combination of its answer with Tatu support function . But for another project, I will use a combination of his answer with the idea of ​​the Ignacio class, because it looks better and more organized.

Answer 1

I used filter_input filter FILTER_SANITIZE_NUMBER_FLOAT for the middle. Something like that:

 $mid = filter_input(INPUT_GET, 'mid', FILTER_SANITIZE_NUMBER_FLOAT); $op = filter_input(INPUT_GET, 'op'); if($mid > 0){ switch($op){ case 'info': // Some code break; case 'cast': // Some more code break; default: break; } } 

Answer 2

I would write a function that would take the name of the index and either return the value in $_GET or throw an exception.

This or encapsulate it in a class similar to the following:

 #$_GET = array('name' => 'Ignacio', 'foo' => '42'); class Get { public static function string($name) { if (isset($_GET[$name])) { return $_GET[$name]; } else { throw new Exception(sprintf('Unknown GET parameter "%s"', $name)); } } public static function int($name) { $val = Get::string($name); $ret = filter_var($val, FILTER_VALIDATE_INT); if ($ret != $val) { throw new Exception(sprintf('Invalid int GET parameter "%s"', $name)); } return $ret; } } echo Get::string('name') . "\n"; echo Get::int('foo') . "\n"; #echo Get::int('name') . "\n"; #echo Get::int('age') . "\n"; 

Answer 3

• empty already checks if the isset variable is there.
• is_numeric is a bit verbose, since you also check for 0 .
• The switch statement is best suited for checking variables against multiple string values.

I would do this:

 $mid = empty($_GET['mid']) ? 0 : (int)$_GET['mid']; // casting as an integer will // make undesirable strings = 0 if ($mid > 0 && !empty($_GET['op'])) { switch($_GET['op']) { case 'info': break; case 'cast': break; default: break; } } 

If you need to save $_GET['op'] in a variable later, you can do this before the switch block, although I would not do this if it were not necessary.

Answer 4

I like the idea of ​​creating an InputFilter class that implements ArrayAccess. This is more object oriented and customizable because you can add methods as you like for customization and work with the same main filtering object.

 $get = new InputFilter($_GET); echo $get->value_integer('variable_name'); 

What is also nice is that it is reused for $ _POST, etc. You just need to do something like $post = new InputFilter($_POST); . And you can also use it for other input sources.

Or, if you have a fairly new version of php, you can later implement filter_input (), as suggested by @Arkh. IMO, having its own class, feels much more reusable and durable.

 <?php // empty for now, fill in later if desired class InputFilterException extends Exception {} /* * Use the ArrayAccess interface as a template. * * Usage examples: * $controller->get = InputFilter($_GET); * echo $controller->get->value_string_html('variable'); * $controller->post = InputFilter($_POST); * echo $controller->get->value_integer('variable'); */ class InputFilter implements ArrayAccess { protected $data; function __construct( $data ) { if( !is_array($data) ) { throw new InputFilterException ("Only arrays are allowed here"); } $this->data = $data; } // do not actually use these function __get( $offset ) { throw new InputFilterException( "Don't use as an array, use functions ->string() ->int() etc: ['" . $offset . "']" ); } function __set( $offset, $value ) { throw new InputFilterException( "Don't modify directly: ['" . $offset . "'] = \"" . $value . "\"" ); } // implement ArrayAccess function offsetExists( $offset ) { return isset( $this->data[$offset]) ); } function offsetSet( $offset, $value ) { $this->data[$offset] = $value; } function offsetUnset( $offset ) { unset( $this->data[$offset] ); } function offsetGet( $offset ) { throw new InputFilterException ("Don't use this object as an array, but were an array : ". $offset); } protected function getValue( $offset ) { if( is_array($this->data[$offset]) ) { throw new InputFilterException ("must use the asArray() function"); } return $this->data[$offset]; } function data_count() { return count($this->data); } public function set_value( $offset, $data ) { $this->offsetSet( $offset, $data ); } // get an array *in* the data public function asArray($offset) { if( !is_array ($this->data[$offset]) ) { throw new InputFilterException("only use asArray() for arrays"); } return new Filter( $this->data[$offset] ); } // validators... function is_set( $offset ) { return $this->offsetExists($offset); } function is_empty( $offset ) { return $this->is_set($offset) && strlen($this->data[$offset]) == 0; } function is_numeric( $offset ) { return $this->is_set($offset) && is_numeric($this->data[$offset]); } function is_integer( $offset ) { if( !$this->is_set($offset) ) { return false; } elseif( is_numeric($this->data[$offset]) ) { $int_value = intval($this->data[$offset]); return $int_value == $this->data[$offset]; } elseif( strlen($this->data[$offset]) == 0 ) { return true; } return false; } function is_array( $offset ) { return $this->is_set($offset) && is_array($this->data[$offset]); } // return data formatted function value_string( $offset ) { return $this->getValue($offset); } function value_string_html( $offset ) { return htmlentities( $this->getValue($offset), null, 'UTF-8' ); } function value_integer( $offset ) { return intval( trim($this->getValue ($offset)) ); } function value_numeric( $offset ) { return doubleval($this->getValue ($offset)); } function value_alphanumeric( $offset ) { return preg_replace("*[^A-Za-z0-9]*", "", $this->getValue ($offset)); } function value_unfiltered( $offset ) { return $this->getValue( $offset ); } } ?> 

Answer 5

You can create a helper function:

 function getvar($var) { return isset($_GET[$var]) && !empty($_GET[$var]) ? $_GET[$var] : false; } $mid = getvar('mid'); $op = getvar('op'); if(is_numeric($mid) && $mid > 0) { if($op == 'info') { } if($op == 'cast') { } } 

This will make your code a little cleaner, but you yourself code everything in order.

Answer 6

What you can do is define a filter function (which already exists in PHP> = 5.2) that will filter the variable based on an argument about what type it is, whether it is a number, a string, or more depending on your requirements.

 function myfilter($variable, $type) { switch($type){ case 'numeric': //check for numbers return the number or false based on check break; case 'alphanumberic': //check for alphanumeric return the text or false based on check break; } } 

Then use this function to filter the values ​​you get with $ _GET

 $foo = myfilter($_GET['foo'], 'numeric'); $bar = myfilter($_GET['bar'], 'alphanumeric'); 

Answer 7

I would do something like below. More or less. But you can also use a filter for most of these things. And you can also turn off warnings and just use empty () and not worry about isset in the first snippet.

 function checkVar($var) { if(isset($var) && !empty($var)) { return true; } return false; } function checkID($id){ if(checkVar($id) && is_numeric($id) && $id > 0) { return $id; } return false; } if($mid = checkID($_GET['mid'])) { if(checkVar($_GET['op'])) { switch($_GET['op']) { case 'info': break; case 'cast': break; } } } 

Answer 8

It looks a little trickier. But it looks like you want to test a lot of terrible conditions. However, there are ways to unify the process. I use the shell class to do this:

 if ($_GET->int["mid"]) { if ($_GET->in_array("op", "info,cast")) { 

But you can define a custom method that combines all isset and is_numeric or any validation.

Answer 9

I like the following approach for integer variables:

 private function _extract_int_val($val) { if ( isset($val) ) { $number = esc_sql($val); if ( is_numeric($number) && $number >= 0) { return (int) $number; } } } 

Answer 10

There is nothing wrong. Nested if statements are absolutely in order. You can use filter though.