PHP include() alternative?

Question

I want to know if my code is safe and if there are other safer alternatives to include external files..

So this is my code example, is it safe? How can I make it safer? Thanks!

<?php switch($_GET['p']){
   case 'test1':
      include 'test1.php';
      break;
   case 'test2':
      include 'test2.php';
      break;
   case 'test':
                echo 'something';
      include 'pages/test.php';
                echo 'something';
      break;
   default: 
      include 'main.php';
      break; 
} ?>

Yacoby · Accepted Answer · 2010-04-11 15:49:55Z

You code is fine. There is no issue conditionally including files like you are doing as the file names are hardcoded. The issue occurs when a the file included is based on an un-sanitized value from the user. E.g

include $_GET['p'];

Which can include whatever the user wants (depending on PHP settings it may also include files on other domains)

The other options are variations on what you are doing

require and require_once will fail if the file doesn't exist. inlucde_once and require_once ensure that the file is only included once, so it that file has been inlucded elsewhere in the program it won't be included.

include_once 'myfile.php';
include_once 'myfile.php'; //does nothing as the file is already included

If you have use classes, there is also the option of the autoloader. From the looks of your application you would have to re-structure it to be able to use it though.

user50049 · Accepted Answer · 2010-04-11 15:48:15Z

You might consider examining the contents of $_GET['p'] prior to even entering the switch. If it contains special characters, rubbish or something else, your program may want to log the incident (and not waste time trying to render the page).

At the least, a nice and polite "Sorry, we could not process your request" page would be in order.

This still allows the switch to fall through to the main page, provided that p contained something worthy of the switch evaluating in the first place.

This is especially true if the main page does any amount of queries in order to render. Sooner or later, someone will notice your URI structure and decide that it might be fun to play with it, don't burn CPU cycles on idiots :)

Arda Xi · Accepted Answer · 2010-04-11 15:42:06Z

0

Seeing as you only include those you've hardcoded, I don't see why this wouldn't be safe. These aren't external files though, but I see what you mean. External would mean on a different server.

As for your question, the only alternative to include is require but that isn't necessarily safer, it just works differently.

answered Apr 11, 2010 at 15:42

Arda Xi

2,6763 gold badges19 silver badges24 bronze badges

Comments

Stomped · Accepted Answer · 2010-04-11 15:42:16Z

0

Yes, perfectly safe.

You're including files you know the contents of, and not doing it based on variables coming from outside sources. an include wont cause your script to fail if it can not load, if that is the result you'd want, choose require('filename');.

answered Apr 11, 2010 at 15:42

Stomped

2,1206 gold badges21 silver badges26 bronze badges

Comments

Hanseh · Accepted Answer · 2010-04-11 15:45:11Z

0

It is safe as it is and the switch statement made the logic clearer. Just to make it more safer maybe you can use $__POST just to hide the switch variable data source to make it little bit safer. :D

answered Apr 11, 2010 at 15:45

Hanseh

7174 silver badges17 bronze badges

Comments

karim79 · Accepted Answer · 2010-04-11 15:43:33Z

-1

You could make that more readable, as follows:

$safeIncludes = array('test1', 'test2', 'test3');
$p = $_GET['p'];
if(in_array($p, $safeIncludes)) {
    $scriptName = $p . '.php';
    include($scriptName);
}

Other than that it is safe as others have pointed out.

answered Apr 11, 2010 at 15:43

karim79

343k67 gold badges420 silver badges409 bronze badges

3 Comments

user50049 Over a year ago

But if its only a few pages, is it really worth allocating an array and searching it if a simple switch() could do? I don't see why its not readable as-is? (unless the cases grew significantly, ie 20+)

karim79 Over a year ago

@Tim Post Maybe so - I did think it worth suggesting, as any further modification need only happen on the array. Furthermore, if he decides to store the list of allowed scripts in the DB, he would need something like the above.

AndyS Over a year ago

This doesn't actually match the supplied code - there is no default and test isn't handled. It is shorter, but isn't (arguably) more readable to novice programmers.

Collectives™ on Stack Overflow

PHP include() alternative?

6 Answers 6

Comments

Comments

Comments

Comments

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

Comments

Comments

Comments

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related