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

Question

Например, простая система типов 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
}

?>

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

РЕДАКТИРОВАТЬ По сути, причина, по которой я использую переменные переменные, заключается в том, чтобы упростить расширение базовой системы, т. Е. Добавить новый контроллер приятно и просто. Я определенно понимаю риски безопасности, связанные с созданием практически любой функции или класса без какого-либо фильтра.

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

Answer 1

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

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

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

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

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

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

РЕДАКТИРОВАТЬ: В качестве дополнительной проверки, вы можете рассмотреть возможность проверки method_exists на объекте перед вызовом метода.

Answer 2

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

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

Answer 3

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

Однако пара указателей: в вашем коде есть уязвимость, из-за которой злоумышленник может пройти через ваш каталог с помощью $_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
}