Это плохая практика использовать переменные переменные в php следующим образом? - PullRequest
Новая
       34

Это плохая практика использовать переменные переменные в php следующим образом?

4 голосов
/ 03 января 2012

Например, простая система типов mvc:

/ api / class / method переписывают в переменные php, используя .htaccess / nginx.conf

, затем делают что-то вроде: 

<?php

// Set up class + method variables
$className = some_class_filter($_GET['class']);
$method = some_method_filter($_GET['method']);

// Check if class exists and execute
if(file_exists(BASE . "/controllers/" . $className . ".class.php")) {
    require BASE . "/controllers/" . $className . ".class.php";
    $$className = new $className();

    // Execute the method
    $$className->$method();
} else {
    // Spit out some error based on the problem
}

?>

Это ужасно плохая практика?Если это плохая практика, может кто-нибудь объяснить, почему именно?И если так, есть ли лучший способ сделать то, что я делаю?

EDIT По сути, причина, по которой я использую переменные переменные, состоит в том, чтобы упростить расширение основной системы -т.е. добавление в новый контроллер приятно и просто.Я определенно понимаю риски безопасности, связанные с созданием практически любой функции или класса без какого-либо фильтра.

'some_filter_here' может быть списком разрешенных контроллеров - белого списка, как некоторые из них упоминали здесь.

Ответы [ 3 ]

6 голосов
/ 03 января 2012

Да, это довольно плохая практика.Вам нужна переменная переменная для этого экземпляра?Другими словами, нужно ли создавать более одного класса и метода в конкретном запросе?Ваша структура URI предполагает, что нет.Если нет, вы можете просто использовать: 

$object = new $className();
$object->$method();

В противном случае вы можете сделать следующее: 

$objects = array();
$objects[$className] = new $className();
$objects[$className]->$method();

Это позволит избежать загрязнения области переменными, которые сложнее отслеживать.

Поскольку проверка существования вашего класса в заданном каталоге проверяется, это должен быть достаточный белый список (предполагается, что злоумышленник не может записать в этот каталог).

EDIT :В качестве дальнейшей проверки вы можете рассмотреть возможность проверки method_exists на объекте перед вызовом метода.

1 голос
/ 03 января 2012

Они не желательны, но их можно использовать как есть.

Пара указателей: ваш код имеет уязвимость, из-за которой злоумышленник может пройти по вашему каталогу с параметрами $_GETкак ?class=../base.Если этот файл существует, ваш file_exists() вызов вернет true, и ваше приложение попытается включить его и создать экземпляр класса.

Безопасный сценарий - поместить в белый список эти параметры, которые будут буквами, числамии только подчеркивание (если вы разделяете слова подчеркиванием, например .php).

Кроме того, я предпочитаю синтаксис использования call_user_func и call_user_func_array.Использование этих функций в вашем коде будет выглядеть следующим образом: 

<?php
$class_name = $_GET['class'];
$method_name = $_GET['method'];

$parameters = $_GET;
unset($parameters['class'], $parameters['method']); // grabs any other $_GET parameters

if (file_exists(BASE.'/controllers/'.$class_name.'.class.php')) {
    require BASE.'/controllers/'.$class_name.'.class.php';
    $controller = new $class_name();
    $response = call_user_func_array(array($controller, $action_name), $parameters);
}
else {
    header('HTTP/1.1 404 Not Found');
    // ...and display an error message
}
1 голос
/ 03 января 2012

Поскольку вы пишете код "some_class_filter" и "some_method_filter", я бы сказал, что все в порядке. У меня также есть обработчик ошибок или обработчик по умолчанию, поэтому в конце я бы сказал, что все в порядке.

Я считаю, что многие MVC-фреймворки в любом случае работают аналогичным образом.

...