Bug in the router

[testt23]

Hi everybody,
I found one bug in Router.php
The problem comes when I try to create the maximum length of the string I get. Then the curly brace is exchanged with the ordinary one.

So, If u input new data in index.php like;

$router->add('{lang:[a-z]{2}+}/{controller}/{action}');
$router->add('{lang:[a-z]{2}+}/{controller}/{action}/{id:\d+}');

You take this array:

Core\Router Object
(
    [routes:protected] => Array
        (
            [/^$/i] => Array
                (
                    [controller] => Home
                    [action] => index
                )

            [/^(?P<lang>[a-z]{2)+}\/(?P<controller>[a-z-]+)\/(?P<action>[a-z-]+)$/i] => Array
                (
                )

            [/^(?P<lang>[a-z]{2)+}\/(?P<controller>[a-z-]+)\/(?P<action>[a-z-]+)\/(?P<id>\d+)$/i] => Array
                (
                )

            [/^admin\/(?P<controller>[a-z-]+)\/(?P<action>[a-z-]+)$/i] => Array
                (
                    [namespace] => Admin
                )

        )

    [params:protected] => Array
        (
        )

)

This is wrong:

[/^(?P<lang>[a-z]{2)+}\/(?P<controller>[a-z-]+)\/(?P<action>[a-z-]+)$/i] => Array

{2)+}

this is correct:

[/^(?P<lang>[a-z]{2}+)\/(?P<controller>[a-z-]+)\/(?P<action>[a-z-]+)$/i] => Array

{2}+)

The problem comes when I try to create the maximum length of the string I get. Then the curly brace is exchanged with the ordinary one.

[daveh]

This can be fixed by changing the Router's add method to this:

public function add($route, $params = [])
{
    $segments = preg_split('/(?<!\\\)\//', $route);

    foreach ($segments as $key => $segment) {

        // Convert simple variables e.g. {controller}
        if (preg_match('/^\{([a-z]+)\}$/', $segment, $matches)) {

            $segments[$key] = '(?<' . $matches[1] . '>[a-z]+)';
        }

        // Convert variables with custom regular expressions e.g. {id:\d+}
        if (preg_match('/^\{([a-z_-]+):(.*)\}$/', $segment, $matches)) {

            $segments[$key] = '(?<' . $matches[1] . '>' . $matches[2] . ')';

        }
    }

    // Join with escaped slashes
    $route = implode('\/', $segments);

    // Add start and end delimiters, and case insensitive flag
    $route = '/^' . $route . '$/iu';

    $this->routes[$route] = $params;
}

(I will be incorporating these changes when I get chance)

[testt23]

Great to now is okay, but if I want to add default parameter (language - en) I get the error:
No route matched.

What I want to do.
If the url is empty for lang:, then I want to set parameter, default parameter with lang: EN / en

If you open site.com/controller/index - it doesn't work,
but if you open site.com/de/controller/index it still works properly.

I think this is a disadvantage because we have to create a lot of routers, for example:

// Add the routes
$router->add('', ['controller' => 'Home', 'action' => 'index']);
$router->add('{lang:[a-z]{2}+}/{controller}/{action}');
$router->add('{lang:[a-z]{2}+}/{controller}/{action}/{id:\d+}');
$router->add('{controller}/{action}');
$router->add('{controller}/{action}/{id:\d+}');
$router->add('admin/{controller}/{action}', ['namespace' => 'Admin']);

So, what I make and no working.

In the file Router.php in method match() I add

                if ( empty ( $params['lang'] ) ) {
                    $params['lang'] .= 'en';
                }

And this is not working and may be is not good practice.

    public function match($url)
    {

        foreach ($this->routes as $route => $params) {

            if (preg_match($route, $url, $matches)) {
                // Get named capture group values
                foreach ($matches as $key => $match) {
                    if (is_string($key)) {
                        $params[$key] = $match;
                    }
                }
                if ( empty ( $params['lang'] ) ) {
                    $params['lang'] .= 'en';
                }

                $this->params = $params;
                return true;
            }
        }

        return false;
    }

Im student in the udemy
Thanks for your help Dave!
Chears,

EDIT: I have one more idea. In method add() we can add in preg_match for NEW default parameter. This is optional example:

// Add the routes
$router->add('', ['controller' => 'Home', 'action' => 'index']);
$router->add('{lang:[a-z]{2}:en+}/', ['controller' => 'Home', 'action' => 'index']);
$router->add('{lang:[a-z]{2}:en+}/{controller}/{action}');
$router->add('{lang:[a-z]{2}:en+}/{controller}/{action}/{id:\d+}');
$router->add('admin/{controller}/{action}', ['namespace' => 'Admin']);

And preg_match check for the parameter is is have add the default language.
I want to learn good practices and that's why I pay so much attention to Routers.

[daveh]

That's one way to do it yes - if this works for you, then no problem. As for it matching with no language, it could be because of the + at the end of this regex:

$router->add('{lang:[a-z]{2}+}/{controller}/{action}');
which means it would match "posts" for example instead of just a language code. Try removing the + to see if it makes a difference:

$router->add('{lang:[a-z]{2}}/{controller}/{action}');

[testt23]

Nothing. After remove + in add parameter is not working. I take a wrong: 404 -> No route matched.

What you think for this situation?

[daveh]

Please can you let me know the full URL that you're using that doesn't match?

[testt23]

<?php
/**
 * Composer
 */
require '../vendor/autoload.php';
ini_set('pcre.backtrack_limit', 5000000);
/**
 * Error and Exception handling
 */
error_reporting(E_ALL);
set_error_handler('Core\Error::errorHandler');
set_exception_handler('Core\Error::exceptionHandler');

/**
 * Routing
 */
$router = new Core\Router();

// Add the routes
$router->add('', ['controller' => 'Home', 'action' => 'index']);
$router->add('{lang:[a-z]{2}}/{controller}/{action}');
$router->add('{lang:[a-z]{2}}/{controller}/{action}/{id:\d+}');
$router->add('admin/{controller}/{action}', ['namespace' => 'Admin']);

$router->dispatch($_SERVER['QUERY_STRING']);

http://localhost/en/posts/index - its okay

Array
(
    [0] => en/posts/index
    [lang] => en
    [1] => en
    [controller] => posts
    [2] => posts
    [action] => index
    [3] => index
)

http://localhost/posts/index - its have error -> no route matched

Array
(
    [controller] => Home
    [action] => index
)
Array
(
)
Array
(
)
Array
(
    [namespace] => Admin
)

I use for the debug print_r() in method match

    public function match($url)
    {
        foreach ($this->routes as $route => $params) {
            if (preg_match($route, $url, $matches)) {
                print_r($matches);
                // Get named capture group values

                foreach ($matches as $key => $match) {
                    if (is_string($key)) {
                        $params[$key] = $match;
                    }
                }

                $this->params = $params;
                return true;
            }
        }

        return false;
    }

[daveh]

To match the http://localhost/posts/index URL, you need a "catch-all" route at the end, after all the others have been defined:

$router->add('{controller}/{action}');

Routes need to be added in order of the most specific first, then the least specific last.