Dynamic включают

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

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

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

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

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

Answer 2

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

Answer 3

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

Answer 4

Как вы определяете, какие страницы разрешено включать (белый список), я не вижу никакого способа отравить переменную $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'; 
} 

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