Динамическое включение

Question

<?php
// Default page
if (!$_SERVER['QUERY_STRING']) $Page = "news";

// View
elseif (isset($_GET['newsID'])) $Page = "newsView";
elseif (isset($_GET['userID'])) $Page = "profile";
elseif (isset($_GET['messageID'])) $Page = "message";
elseif (isset($_GET['threadID'])) $Page = "thread";
elseif (isset($_GET['forumID'])) $Page = "forum";
elseif (isset($_GET['imgID'])) $Page = "imageView";

// Pages
elseif ($_GET['content'] == "search") $Page = "search";
elseif ($_GET['content'] == "gallery") $Page = "gallery";
elseif ($_GET['content'] == "forums") $Page = "forums";
elseif ($_GET['content'] == "messages") $Page = "messages";
many more...

// If page don't exist
else $Page = "error";

// Output page
include($config['PAGE_PATH'].$Page.'.php');
include($config['TEMPLATE_PATH'].$Page.'.html');
?>

Это код, который мой друг написал несколько лет назад ...
Мне интересно, насколько это безопасно и могу ли я сделать его чище?

Спасибо.

Answer 1

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

Но вы можете очистить его с помощью таких массивов, как:

$argToPage = array(
    'newsID'    => 'newsView',
    'userID'    => 'profile',
    'messageID' => 'message',
    'threadID'  => 'thread',
    'forumID'   => 'forum',
    'imgID'     => 'imageView'
);
$contents = array(
    'search',
    'gallery',
    'forums',
    'messages'
);

$Page = null;
if (trim($_SERVER['QUERY_STRING']) == '') {
    $Page = 'news';
} else {
    foreach ($_GET as $key => $val) {
        if (isset($argToPage[$key])) {
            $Page = $argToPage[$key];
            break;
        }
    }
}
if (is_null($Page) && isset($_GET['content']) && in_array($_GET['content'], $contents)) {
    $Page = $contents[$_GET['content']];
} else {
    $Page = 'error';
}

Но это не намного чище.

Answer 2

Ну, это безопасно в том смысле, что код дезинфицирует параметр. Люди часто делают это (к катастрофическим результатам обычно).

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

Как говорится, в вышеуказанном подходе нет ничего принципиально неправильного.

Просто убедитесь, что вы обрабатываете любые данные, поступающие от пользователя, с крайней паранойей.

Answer 3

Вы можете создать массив с опциями GET в качестве ключа и страниц в качестве значений ... затем использовать оператор switch () Это должно сделать код чище. Но, как сказал Клетус, это не лучший способ сделать контроллер.

Answer 4

Я бы очень осторожен с уборкой, как и раньше. Использование переменной, предоставленной пользовательским вводом в include, является серьезным недостатком безопасности. Я бы оставил это как есть.