Is it bad practice to dynamically create a variable from array to read a $_GET method in php

250 views Asked by At

Going through a php class file at work I found an interesting snippet. The script is dynamically creating a variable, dynamically checking if there is an active $_GET[''] for the variable it's creating and if there is it's loading the $_GET data and if it's not it's writing N/A to the variable that it's dynamically creating. The script then continues on to a switch function with the same logic for it's case breaks.

1.) Is this safe?

2.) Can it be attacked?

3.) Is there an easier way to do this?

4.) Why would you do this?

$switch_types = array("id","type","page");

foreach ($switch_types as $key => $value) {
    $$value = $value;
    if(isset($_GET[$$value])){
        $$value = $_GET[$$value];
        }
    else{
        $$value = "N/A";
    }        
}
1

There are 1 answers

0
Barmar On BEST ANSWER

This is basically just a long-winded way to write:

$id = isset($_GET['id']) ? $_GET['id'] : 'N/A';
$type = isset($_GET['type']) ? $_GET['type'] : 'N/A';
$page = isset($_GET['page']) ? $_GET['page'] : 'N/A';

It's perfectly safe because the list of variables to assign is specified in the program, it doesn't come dynamically from the client.

There's some unnecessary code in his loop -- $$value = $value is not needed. It can be simplified to:

foreach ($switch_types as $value) {
    if(isset($_GET[$value])){
        $$value = $_GET[$value];
        }
    else{
        $$value = "N/A";
    }        
}

or:

foreach ($switch_types as $value) {
    $$value = isset($_GET[$value]) ? $_GET[$value] : 'N/A';
}