check out this 1st script I ever made

cyberlew15

please tell me what u think of this script that I use for a page select system

<?php
//if there are no ?page variables run this code
if ($HTTP_GET_VARS['page']=="")
{
// put where you want the code for the 1st page
include ("./news/news.php");
}
// this redirects the user for ?page=games to ./games/games.php
// also it meaqns that there are ?page variables
else { include ("./" . $HTTP_GET_VARS['page'] . "/". $HTTP_GET_VARS['page'] .".php");
}
?>

just wanted to check if there is a cleaner way to do this 😕

digitalDaze

That seems to be a common use for PHP.
This is how I do it.
<?
if (!empty($GET['page']))
{
include $GET['page'] . 'php';
}
else
{
include 'defult.php';
}
?>
Its a little over simplified but you get the picture.

Weedpacket

Originally posted by digitalDaze
Its a little over simplified but you get the picture.

For example, there are no security considerations.

And please use [php] tags, you two. It makes code so much easier to read (ref: pt.5 of the guidlines 🙂).

digitalDaze

Sorry about no

 tags.  What kind of security issues should we be aware of when doing this?

planetsim

Your not checking to see if the file exists. An attacker could potentially include a file from their own server and get details about your server or install things.

digitalDaze

I never thought of that, thanks for the tip.

Merve

You might want to indent the code inside your control structures. This makes code much easier to read and understand. It's good to see that you've began commenting your code. This is important from day. It may be obvious to you what your code does, but to someone else, it might be a whole load of blah.

Are you using a version of PHP older than 4.1.0? If so, then your code is fine, but if you are using a newer version of PHP then please use $_GET instead of $HTTP_GET_VARS.