Reputation: 83
I'm trying to build a really simple php controller page for a small site. Here is what I have so far. It seems to work well. Are there any issues I might be missing with doing it this way?
$page = $_GET['p'];
switch ($page)
{
case "":
ob_start();
include "inc/home.php";
$content = ob_get_contents();
ob_end_clean();
break;
case $page:
$page = str_replace("/", "", $page);
if (file_exists("inc/".$page.".php"))
{
ob_start();
include "inc/".$page.".php";
$content = ob_get_contents();
ob_end_clean();
}
else
include "inc/404.php";
break;
}
include("inc/header.php");
echo $content;
include("inc/footer.php");
UPDATE: Here is the final code based on comments that works well.
<?php
$page = (isset( $_GET['p']) && !empty($_GET['p'])) ? $_GET['p'] : 'home';
if( preg_match( '/[^a-z]/i', $page))
{
$page = '404';
}
if( !file_exists( "inc/".$page.".php"))
{
$page = '404';
}
ob_start();
include("inc/header.php");
include("inc/".$page.".php");
include("inc/footer.php");
?>
Upvotes: 0
Views: 516
Reputation: 59699
Your entire script can be rewritten as follows:
$page = ( isset( $_GET['p']) && !empty( $_GET['p'])) ? $_GET['p'] : 'home';
// Only allow alphabetic characters in a user supplied page
if( preg_match( '/[^a-z]/i', $page))
{
$page = '404';
}
if( !file_exists( "inc/".$page.".php"))
{
$page = '404';
}
include("inc/header.php");
include("inc/".$page.".php");
include("inc/footer.php");
However, this is also no longer susceptible to Local File Inclusion, as $page
is restricted to only alphabetic characters, and the script will show the 404 page if anything else is submitted.
It's also more efficient as its not using output buffering.
Upvotes: 2