CODEWITHSUNDEEP
phpmodel-view-controller
lgt
lgt

Reputation: 1034

how to build a good router for php mvc

I'm experimenting with php mvc and I'm stucked with the following issue. My request and router classes are really simple and I would like to extend theme to can handle controller calls from sub folders and to controller classes functions should be able to pick up url variables send it threw get and post.

my router looks as it follows

class Router{

    public static function route(Request $request){


        $controller = $request->getController().'Controller';

        $method = $request->getMethod();

        $args = $request->getArgs();


        $controllerFile = __SITE_PATH.'/controllers/'.$controller.'.php';


        if(is_readable($controllerFile)){
            require_once $controllerFile;

            $controller = new $controller;


            if(!empty($args)){
                call_user_func_array(array($controller,$method),$args);
            }else{  
                call_user_func(array($controller,$method));
            }   
            return;
        }

        throw new Exception('404 - '.$request->getController().'--Controller not found');
    }
}

and Request class

    private $_controller;


    private $_method;

    private $_args;

    public function __construct(){

        $parts = explode('/',$_SERVER['REQUEST_URI']);


        $this->_controller = ($c = array_shift($parts))? $c: 'index';
        $this->_method = ($c = array_shift($parts))? $c: 'index';

        $this->_args = (isset($parts[0])) ? $parts : array();

    }

    public function getController(){

        return $this->_controller;

    }
    public function getMethod(){

        return $this->_method;

    }
    public function getArgs(){

        return $this->_args;
    }
}

The problem is:when I try to send threw ajax, variables to a controller method this are not recognized because of its url structure. For example

index/ajax?mod_title=shop+marks&domain=example

is accepted just if it look

index/ajax/shop+mark/example

Upvotes: 5

Views: 16086

Answers (4)

damianb
damianb

Reputation: 1224

Your code contains what is known as an LFI vulnerability and is dangerous in its current state.
You should whitelist your what can be used as your $controller, as otherwise an attacker could try to specify something using NUL bytes and possibly going up a directory to include files that SHOULD NOT be ever included, such as /etc/passwd, a config file, whatever.

Your router is not safe for use; beware!

edit: example on whitelisting

$safe = array(
    'ajax',
    'somecontroller',
    'foo',
    'bar',
);
if(!in_array($this->_controller, $safe))
{
    throw new Exception(); // replace me with your own error 404 stuff
}

Upvotes: 10

kta
kta

Reputation: 20110

Choose any popular MVC to see how they implement it under the hood. In addition, spl_autoload_register and namespace are your friends.

Upvotes: 0

m1lt0n
m1lt0n

Reputation: 361

Since your Request class uses a URI segments approach for identifying controller, action and arguments, global variables such as $_GET or $_REQUEST are not taken into account from within your Request.

What you need to do is to make some additions to your Request code. Specifically:

Remove the line:

$this->_args = (isset($parts[0])) ? $parts : array();

And add the following:

$all_parts = (isset($parts[0])) ? $parts : array();
$all_parts['get'] = $_GET;
$this->_args = $all_parts;

This way, $_GET (ie variables passed via the url) variables will be available in the actions called, as they will be in $args (they will be available as $args['get'] actually, which is the array that holds the $_GET vars, so you will be able to have access to domain=example by using $args['get']['domain']).

Ofcourse, you can add one more method in your Request class (e.g. query) that might look like that:

public function query($var = null)
{
    if ($var === null)
    {
        return $_GET;
    }
    if ( ! isset($_GET[$var]) )
    {
        return FALSE;
    }
    return $_GET[$var];
}

This way, you can get a single variable from the url (e.g. $request->query('domain')) or the whole $_GET array ($request->query()).

Upvotes: 2

mabi
mabi

Reputation: 5306

That's because php will put "?mod_title=..." in the $_GET array automatically. Your getArgs() function should check for $_GET, $_POST or $_REQUEST.

If you're trying for a minimal MVC approach, have a look at rasmus' example: http://toys.lerdorf.com/archives/38-The-no-framework-PHP-MVC-framework.html

If your use case is going to get more complex, have a look at how Zend (http://framework.zend.com/manual/en/zend.controller.html) or Symfony (https://github.com/symfony/symfony/tree/master/src/Symfony/Component/Routing) do their stuff.

Upvotes: 0

Related Questions