From 3a7b27796ad5cfb3a9b1b03eae247101d33ee683 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Tue, 1 Aug 2017 18:29:47 +0200 Subject: [PATCH 1/3] Url matching fixes --- README.md | 61 +++++++++++++------------- src/Pecee/SimpleRouter/Route/Route.php | 6 ++- src/Pecee/SimpleRouter/Router.php | 3 ++ test/RouterUrlTest.php | 27 ++++++++++++ 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 28d5844..ef10ef5 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ If you want a great new feature or experience any issues what-so-ever, please fe - [Optional parameters](#optional-parameters) - [Regular expression constraints](#regular-expression-constraints) - [Regular expression route-match](#regular-expression-route-match) + - [Custom regex for matching parameters](#custom-regex-for-matching-parameters) - [Named routes](#named-routes) - [Generating URLs To Named Routes](#generating-urls-to-named-routes) - [Router groups](#router-groups) @@ -381,7 +382,7 @@ The example below is using the following regular expression: `/ajax/([\w]+)/?([0 **Matches:** `/ajax/abc/`, `/ajax/abc/123/` -**Doesn't match:** `/ajax/` +**Won't match:** `/ajax/` Match groups specified in the regex will be passed on as parameters: @@ -392,6 +393,35 @@ SimpleRouter::all('/ajax/abc/123', function($param1, $param2) { })->setMatch('/\/ajax\/([\w]+)\/?([0-9]+)?\/?/is'); ``` +### Custom regex for matching parameters + +By default simple-php-router uses the `\w` regular expression when matching parameters. +This decision was made with speed and reliability in mind, as this match will match both letters, number and most of the used symbols on the internet. + +However, sometimes it can be necessary to add a custom regular expression to match more advanced characters like `-` etc. + +Instead of adding a custom regular expression to all your parameters, you can simply add a global regular expression which will be used on all the parameters on the route. + +**Note:** If you the regular expression to be available across, we recommend using the global parameter on a group as demonstrated in the examples below. + +#### Example + +This example will ensure that all parameters use the `[\w\-]+` regular expression when parsing. + +```php +SimpleRouter::get('/path/{parameter}', 'VideoController@home', ['defaultParameterRegex' => '[\w\-]+']); +``` + +You can also apply this setting to a group if you need multiple routes to use your custom regular expression when parsing parameters. + +```php +SimpleRouter::group(['defaultParameterRegex' => '[\w\-]+'], function() { + + SimpleRouter::get('/path/{parameter}', 'VideoController@home'); + +}); +``` + ## Named routes Named routes allow the convenient generation of URLs or redirects for specific routes. You may specify a name for a route by chaining the name method onto the route definition: @@ -1102,35 +1132,6 @@ $router->addRoute($route); This section contains advanced tips & tricks on extending the usage for parameters. -### Custom default regex for matching parameters - -By default simple-php-router uses the `\w` regular expression when matching parameters. -This decision was made with speed and reliability in mind, as this match will match both letters, number and most of the used symbols on the internet. - -However, sometimes it can be nessesary to add a custom regular expression to match more advanced characters like `-` etc. - -Instead of adding a custom regular expression to all your parameters, you can simply add a global regular expression which will be used on all the parameters on the route. - -**Note:** If you the regular expression to be available across, we recommend using the global parameter on a group as demonstrated in the examples below. - -#### Route - -This example will ensure that all parameters use the `[\w\-]+` regular expression when parsing. - -```php -SimpleRouter::get('/path/{parameter}', 'VideoController@home', ['defaultParameterRegex' => '[\w\-]+']); -``` - -You can also apply this setting to a group if you need multiple routes to use your custom regular expression when parsing parameters. - -```php -SimpleRouter::group(['defaultParameterRegex' => '[\w\-]+'], function() { - - SimpleRouter::get('/path/{parameter}', 'VideoController@home'); - -}); -``` - ## Extending This is a simple example of an integration into a framework. diff --git a/src/Pecee/SimpleRouter/Route/Route.php b/src/Pecee/SimpleRouter/Route/Route.php index 079a4f9..c9d7ce6 100644 --- a/src/Pecee/SimpleRouter/Route/Route.php +++ b/src/Pecee/SimpleRouter/Route/Route.php @@ -135,7 +135,7 @@ abstract class Route implements IRoute } } - $regex = sprintf('\-?\/?(?P<%s>%s)', $name, $regex) . $parameters[2][$key]; + $regex = sprintf('(?:\/|\-)' . $parameters[2][$key] . '(?P<%s>%s)', $name, $regex) . $parameters[2][$key]; } @@ -148,7 +148,9 @@ abstract class Route implements IRoute $urlRegex = preg_quote($route, '/'); } - if (preg_match('/^' . $urlRegex . '(\/?)$/', $url, $matches) > 0) { + echo $urlRegex . '\/? | ' . $url . chr(10); + + if (preg_match('/^' . $urlRegex . '\/?/', $url, $matches) > 0) { $values = []; diff --git a/src/Pecee/SimpleRouter/Router.php b/src/Pecee/SimpleRouter/Router.php index 01f898e..e404538 100644 --- a/src/Pecee/SimpleRouter/Router.php +++ b/src/Pecee/SimpleRouter/Router.php @@ -11,6 +11,7 @@ use Pecee\SimpleRouter\Route\IControllerRoute; use Pecee\SimpleRouter\Route\IGroupRoute; use Pecee\SimpleRouter\Route\ILoadableRoute; use Pecee\SimpleRouter\Route\IRoute; +use Pecee\SimpleRouter\Route\LoadableRoute; class Router { @@ -124,6 +125,8 @@ class Router $url = ($this->request->getRewriteUrl() !== null) ? $this->request->getRewriteUrl() : $this->request->getUri(); + + /* @var $route IRoute */ for ($i = $max; $i >= 0; $i--) { diff --git a/test/RouterUrlTest.php b/test/RouterUrlTest.php index 93e6108..6bf3b25 100644 --- a/test/RouterUrlTest.php +++ b/test/RouterUrlTest.php @@ -8,6 +8,33 @@ require_once 'Helpers/TestRouter.php'; class RouterUrlTest extends PHPUnit_Framework_TestCase { + public function testOptionalParameters() + { + + // TestRouter::get('/aviso/legal', 'DummyController@method1'); + TestRouter::get('/aviso/{aviso}', 'DummyController@method1'); + //TestRouter::get('/pagina/{pagina}', 'DummyController@method1'); + TestRouter::get('/{pagina?}', 'DummyController@method1'); + + //TestRouter::debugNoReset('/aviso/optional', 'get'); + //$this->assertEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + + //TestRouter::debugNoReset('/pagina/optional', 'get'); + //$this->assertEquals('/pagina/{pagina}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + + //TestRouter::debugNoReset('/optional', 'get'); + //$this->assertEquals('/{pagina?}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + +// New test lines + //TestRouter::debugNoReset('/avisolegal', 'get'); + //$this->assertNotEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + + TestRouter::debugNoReset('/avisolegal', 'get'); + $this->assertEquals('/{pagina?}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + + TestRouter::router()->reset(); + } + public function testSimilarUrls() { // Match normal route on alias From 883d8a6b0eb5a60f14df5143f5ee8a61a8e65d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Tue, 1 Aug 2017 21:09:18 +0200 Subject: [PATCH 2/3] Fixes for issue #248 --- src/Pecee/SimpleRouter/Route/Route.php | 4 ++-- .../SimpleRouter/Route/RouteController.php | 2 +- src/Pecee/SimpleRouter/Router.php | 3 --- test/RouterUrlTest.php | 22 +++++++++---------- 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/Pecee/SimpleRouter/Route/Route.php b/src/Pecee/SimpleRouter/Route/Route.php index c9d7ce6..ae06128 100644 --- a/src/Pecee/SimpleRouter/Route/Route.php +++ b/src/Pecee/SimpleRouter/Route/Route.php @@ -110,6 +110,8 @@ abstract class Route implements IRoute $parameters = []; + $url = '/' . ltrim($url, '/'); + if (preg_match_all('/' . $regex . '/', $route, $parameters)) { $urlParts = preg_split('/((\-?\/?)\{[^}]+\})/', rtrim($route, '/')); @@ -148,8 +150,6 @@ abstract class Route implements IRoute $urlRegex = preg_quote($route, '/'); } - echo $urlRegex . '\/? | ' . $url . chr(10); - if (preg_match('/^' . $urlRegex . '\/?/', $url, $matches) > 0) { $values = []; diff --git a/src/Pecee/SimpleRouter/Route/RouteController.php b/src/Pecee/SimpleRouter/Route/RouteController.php index e01854a..b32fa23 100644 --- a/src/Pecee/SimpleRouter/Route/RouteController.php +++ b/src/Pecee/SimpleRouter/Route/RouteController.php @@ -53,7 +53,7 @@ class RouteController extends LoadableRoute implements IControllerRoute if (strpos($name, '.') !== false) { $found = array_search(substr($name, strrpos($name, '.') + 1), $this->names, false); if ($found !== false) { - $method = $found; + $method = (string)$found; } } diff --git a/src/Pecee/SimpleRouter/Router.php b/src/Pecee/SimpleRouter/Router.php index e404538..01f898e 100644 --- a/src/Pecee/SimpleRouter/Router.php +++ b/src/Pecee/SimpleRouter/Router.php @@ -11,7 +11,6 @@ use Pecee\SimpleRouter\Route\IControllerRoute; use Pecee\SimpleRouter\Route\IGroupRoute; use Pecee\SimpleRouter\Route\ILoadableRoute; use Pecee\SimpleRouter\Route\IRoute; -use Pecee\SimpleRouter\Route\LoadableRoute; class Router { @@ -125,8 +124,6 @@ class Router $url = ($this->request->getRewriteUrl() !== null) ? $this->request->getRewriteUrl() : $this->request->getUri(); - - /* @var $route IRoute */ for ($i = $max; $i >= 0; $i--) { diff --git a/test/RouterUrlTest.php b/test/RouterUrlTest.php index 6bf3b25..91e42ab 100644 --- a/test/RouterUrlTest.php +++ b/test/RouterUrlTest.php @@ -10,24 +10,22 @@ class RouterUrlTest extends PHPUnit_Framework_TestCase public function testOptionalParameters() { - - // TestRouter::get('/aviso/legal', 'DummyController@method1'); + TestRouter::get('/aviso/legal', 'DummyController@method1'); TestRouter::get('/aviso/{aviso}', 'DummyController@method1'); - //TestRouter::get('/pagina/{pagina}', 'DummyController@method1'); + TestRouter::get('/pagina/{pagina}', 'DummyController@method1'); TestRouter::get('/{pagina?}', 'DummyController@method1'); - //TestRouter::debugNoReset('/aviso/optional', 'get'); - //$this->assertEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + TestRouter::debugNoReset('/aviso/optional', 'get'); + $this->assertEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); - //TestRouter::debugNoReset('/pagina/optional', 'get'); - //$this->assertEquals('/pagina/{pagina}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + TestRouter::debugNoReset('/pagina/optional', 'get'); + $this->assertEquals('/pagina/{pagina}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); - //TestRouter::debugNoReset('/optional', 'get'); - //$this->assertEquals('/{pagina?}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + TestRouter::debugNoReset('/optional', 'get'); + $this->assertEquals('/{pagina?}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); -// New test lines - //TestRouter::debugNoReset('/avisolegal', 'get'); - //$this->assertNotEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); + TestRouter::debugNoReset('/avisolegal', 'get'); + $this->assertNotEquals('/aviso/{aviso}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); TestRouter::debugNoReset('/avisolegal', 'get'); $this->assertEquals('/{pagina?}/', TestRouter::router()->getRequest()->getLoadedRoute()->getUrl()); From 3c73de866ee28f1799ca3433f48f79505fcd61f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Tue, 1 Aug 2017 21:25:44 +0200 Subject: [PATCH 3/3] Added code-comments. --- src/Pecee/SimpleRouter/Route/Route.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Pecee/SimpleRouter/Route/Route.php b/src/Pecee/SimpleRouter/Route/Route.php index ae06128..f9f618f 100644 --- a/src/Pecee/SimpleRouter/Route/Route.php +++ b/src/Pecee/SimpleRouter/Route/Route.php @@ -110,6 +110,7 @@ abstract class Route implements IRoute $parameters = []; + // Ensures that hostnames/domains will work with parameters $url = '/' . ltrim($url, '/'); if (preg_match_all('/' . $regex . '/', $route, $parameters)) {