how to build a good router for php mvc

Question

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

score 10 · Accepted Answer · edited Nov 06 '13 at 17:23

10

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
}

edited Nov 06 '13 at 17:23

Martin Bean

38,379
25
128
201

answered May 24 '12 at 13:52

damianb

1,224
7
16

what would you advise how should I change it? – lgt May 24 '12 at 13:54
I would advise, like I said, that you whitelist what can be used as your Request class's `_controller` property. I'll edit my answer with a quick example. – damianb May 24 '12 at 13:56
@lgt There ya go, take a look. You should also strip out any nul bytes in any user-provided inputs as well - do a str_replace and replace any instances of `"\0"` with an empty string for safety. – damianb May 24 '12 at 14:23

m1lt0n · Answer 2 · 2012-05-24T11:30:40.110

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()).

score 0 · Answer 3 · answered May 24 '12 at 09:47

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.

thanks but I still do not have an idea how to handle a request and route it to the right controller — lgt, May 24 '12 at 10:43

score 0 · Answer 4 · answered Oct 24 '13 at 09:44

0

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

answered Oct 24 '13 at 09:44

kta

19,412
7
65
47

Sounds like a good idea, but there is something to be said about exploring and experimenting. Hey, would you mind taking a look at my question. It's worth 300 points! https://stackoverflow.com/questions/42172228/is-this-how-an-mvc-router-class-typically-works – Anthony Rutledge Feb 15 '17 at 17:35

how to build a good router for php mvc

4 Answers4

Linked