Page 1 of 1

using a registry to include files

Posted: Sun Aug 10, 2008 5:34 pm
by SidewinderX
Below is a very simple example of what I am doing. Essentially I have a "front controller" which validates an incoming http get request against a white list. The http request is a file that should be included. If it is valid, the request is stored in a registry. Once it is stored in the registry, an instance of another class is called - that class is responsible for including the validated request (which is now stored in the registry). Aside from some abstraction that may seem complete useless in my code below, is it safe to use data stored in the registry (which is a valid http request) to include the file? I cannot see anything wrong with it even though Zend Studio is complaining about an "unsafe use of variable in call include()/require()." I just think thats because Zend Studio does not know where the variable is coming from.

Code: Select all

<?php
 
class Singleton
{
    private static $instance;
    public $file;
 
    private function __construct ($file)
    {
        $this->file = $file;
    }
 
    public static function init ($file = null)
    {
        if (! isset(self::$instance)) {
            self::$instance = new self($file);
        }
        return self::$instance;
    }
}
 
class Includer
{
 
    public function __construct ()
    {
        $config = Singleton::init();
        require_once ($config->file);
    }
}
 
$valid_files = array("myfile" , "yourfile" , "foo" , "bar");
$requested_file = $_GET['file'];
if (in_array($requested_file, $valid_files)) {
    $file = $requested_file . ".php";
    $config = Singleton::init($file);
    new Includer();
}
?>
So, is there anything unsafe about line 28?

Thank you,
John

Re: using a registry to include files

Posted: Mon Aug 11, 2008 4:58 pm
by Mordred
The code analysis tool cannot grasp your code (particularly that you've checked the filename) so it complains. The code looks okay to me, so ignore.

Re: using a registry to include files

Posted: Mon Aug 11, 2008 6:10 pm
by SidewinderX
Thanks Mordred, thats what I assumed.

Re: using a registry to include files

Posted: Mon Aug 11, 2008 7:46 pm
by Christopher
I would recommend not using globals wrapped in globals, and instead use objects:

Code: Select all

// use a config object to load the configurtion
$config = new Config_Ini('/path/to/config.ini');    // or XML or DB or whatever
$config->valid_files = array("myfile" , "yourfile" , "foo" , "bar");   // or set it directly
 
// somewhere later do something like
$fc = new FrontController($request, $config);
$fc->dispatch();

Re: using a registry to include files

Posted: Tue Aug 12, 2008 5:03 pm
by SidewinderX
arborint wrote:I would recommend not using globals wrapped in globals
By globals wrapped in globals I am assuming you mean an http request wrapped in a singleton? This was a concern of mine, but if the data is properly validated, I cannot think of a reason not to use it. I am interested to know why you would not recommend it.

Re: using a registry to include files

Posted: Wed Aug 13, 2008 12:51 am
by Christopher
Maybe I am missing something, but this:

Code: Select all

$config = Singleton::init($file);
new Includer();
just seems like a crazy Rube Goldberg device to do this:

Code: Select all

require_once $file;

Re: using a registry to include files

Posted: Wed Aug 13, 2008 3:21 pm
by alex.barylski
I was wondering the same thing...

Why use a singleton to store nothing but a simple config value???

Your code was extremely confusing to me...so while it might make sense to you now...in 9 months time you'll be scratching your head wondering what the hell.

No reason to use a singleton IMHO.

Re: using a registry to include files

Posted: Wed Aug 13, 2008 3:46 pm
by SidewinderX
Oh. I completely concur with your point. The code above was just an example, not my actual implementation, hence the "aside from some abstraction that may seem complete useless in my code below."