From d9b97ccf4202d9ed88b0559ee994031c493dc4d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Mon, 28 Nov 2016 04:20:34 +0100 Subject: [PATCH] Bugfixes --- src/Pecee/Http/Input/Input.php | 14 +- src/Pecee/Http/Input/InputFile.php | 5 +- .../Http/Middleware/BaseCsrfVerifier.php | 2 +- src/Pecee/Http/Request.php | 2 +- .../SimpleRouter/Route/LoadableRoute.php | 9 +- src/Pecee/SimpleRouter/Route/Route.php | 128 +++++------------- .../SimpleRouter/Route/RouteController.php | 3 - src/Pecee/SimpleRouter/Route/RouteGroup.php | 4 + src/Pecee/SimpleRouter/Route/RouteUrl.php | 9 +- test/RouterRouteTest.php | 30 ++-- 10 files changed, 74 insertions(+), 132 deletions(-) diff --git a/src/Pecee/Http/Input/Input.php b/src/Pecee/Http/Input/Input.php index 4c8fd71..6f3a9c4 100644 --- a/src/Pecee/Http/Input/Input.php +++ b/src/Pecee/Http/Input/Input.php @@ -51,18 +51,17 @@ class Input } /* Parse get requests */ - $this->file = $this->parseFiles(); + if (count($_FILES) > 0) { + $this->file = $this->parseFiles(); + } } public function parseFiles() { - if (count($_FILES) === 0) { - return []; - } - + $files = $_FILES; $list = []; - foreach ($_FILES as $key => $value) { + foreach ($files as $key => $value) { // Handle array input if (is_array($value['name']) === false) { @@ -95,7 +94,8 @@ class Input $path = $original[$property]; - foreach (array_values($index) as $i) { + $tmp = array_values($index); + foreach ($tmp as $i) { $path = $path[$i]; } diff --git a/src/Pecee/Http/Input/InputFile.php b/src/Pecee/Http/Input/InputFile.php index 6fdec72..6ec141b 100644 --- a/src/Pecee/Http/Input/InputFile.php +++ b/src/Pecee/Http/Input/InputFile.php @@ -42,14 +42,13 @@ class InputFile implements IInputItem 'error' => null, ], $values); - $input = new static($values['index']); - $input->setError($values['error']) + return (new static($values['index'])) + ->setError($values['error']) ->setSize($values['size']) ->setType($values['type']) ->setTmpName($values['tmp_name']) ->setFilename($values['name']); - return $input; } /** diff --git a/src/Pecee/Http/Middleware/BaseCsrfVerifier.php b/src/Pecee/Http/Middleware/BaseCsrfVerifier.php index 046251b..7a96ba9 100644 --- a/src/Pecee/Http/Middleware/BaseCsrfVerifier.php +++ b/src/Pecee/Http/Middleware/BaseCsrfVerifier.php @@ -58,7 +58,7 @@ class BaseCsrfVerifier implements IMiddleware public function handle(Request $request, ILoadableRoute &$route = null) { - if ($this->skip($request) === false && in_array($request->getMethod(), ['post', 'put', 'delete']) === true) { + if ($this->skip($request) === false && in_array($request->getMethod(), ['post', 'put', 'delete'], false) === true) { $token = $request->getInput()->get(static::POST_KEY, null, 'post'); diff --git a/src/Pecee/Http/Request.php b/src/Pecee/Http/Request.php index 8215a27..eb2fe26 100644 --- a/src/Pecee/Http/Request.php +++ b/src/Pecee/Http/Request.php @@ -215,7 +215,7 @@ class Request public function __isset($name) { - return $this->data[$name] ?? null; + return array_key_exists($name, $this->data); } public function __set($name, $value = null) diff --git a/src/Pecee/SimpleRouter/Route/LoadableRoute.php b/src/Pecee/SimpleRouter/Route/LoadableRoute.php index 8696bfa..a6a40a5 100644 --- a/src/Pecee/SimpleRouter/Route/LoadableRoute.php +++ b/src/Pecee/SimpleRouter/Route/LoadableRoute.php @@ -7,8 +7,6 @@ use Pecee\SimpleRouter\Exceptions\HttpException; abstract class LoadableRoute extends Route implements ILoadableRoute { - const PARAMETERS_REGEX_MATCH = '%s([\w\-\_]*?)\%s{0,1}%s'; - /** * @var */ @@ -57,12 +55,10 @@ abstract class LoadableRoute extends Route implements ILoadableRoute $parameters = []; - if (preg_match($this->regex, $request->getHost() . $url, $parameters) !== false) { + if (preg_match($this->regex, $request->getHost() . $url, $parameters) > 0) { /* Remove global match */ - if (count($parameters) > 1) { - $this->parameters = array_slice($parameters, 1); - } + $this->parameters = array_slice($parameters, 1); return true; } @@ -147,6 +143,7 @@ abstract class LoadableRoute extends Route implements ILoadableRoute } } + /** @noinspection AliasFunctionsUsageInspection */ $url .= join('/', $unknownParams); return rtrim($url, '/') . '/'; diff --git a/src/Pecee/SimpleRouter/Route/Route.php b/src/Pecee/SimpleRouter/Route/Route.php index 03b8093..ff62281 100644 --- a/src/Pecee/SimpleRouter/Route/Route.php +++ b/src/Pecee/SimpleRouter/Route/Route.php @@ -6,6 +6,8 @@ use Pecee\SimpleRouter\Exceptions\NotFoundHttpException; abstract class Route implements IRoute { + const PARAMETERS_REGEX_MATCH = '%s([\w\-\_]*?)(\%s{0,1})%s'; + const REQUEST_TYPE_GET = 'get'; const REQUEST_TYPE_POST = 'post'; const REQUEST_TYPE_PUT = 'put'; @@ -88,113 +90,49 @@ abstract class Route implements IRoute } } - protected function generateRouteRegEx($route, $parameterRegex = '[\w]+') { - - /* Make regular expression based on route */ - - $route = rtrim($route, '/') . '/'; - - $parameterNames = []; - $regex = ''; - $lastCharacter = ''; - $isParameter = false; - $parameter = ''; - $routeLength = strlen($route) - 1; - - for ($i = $routeLength; $i >= 0; $i--) { - - $character = strrev($route)[$i]; - - if ($character === '{') { - - /* Strip "/" and "\" from regex */ - - if (substr($regex, strlen($regex) - 1) === '/') { - $regex = substr($regex, 0, -2); - } - - $isParameter = true; - - } elseif ($isParameter === true) { - - if($character === '}') { - - $required = true; - - /* Check for optional parameter and use custom parameter regex if it exists */ - - if (is_array($this->where) === true && isset($this->where[$parameter])) { - $parameterRegex = $this->where[$parameter]; - } - - if ($lastCharacter === '?') { - - $parameter = substr($parameter, 0, -1); - $regex .= '(?:\/(?P<' . $parameter . '>' . $parameterRegex . ')[^\/]?)?'; - $required = false; - - } else { - - $regex .= '\/(?P<' . $parameter . '>' . $parameterRegex . ')[^\/]?'; - } - - $parameterNames[] = [ - 'name' => $parameter, - 'required' => $required, - ]; - - $parameter = ''; - $isParameter = false; - - } else { - $parameter .= $character; - } - - - } else if ($character === '/') { - $regex .= '\\' . $character; - } else { - $regex .= str_replace('.', '\\.', $character); - } - - $lastCharacter = $character; - } - - $this->regex = $regex; - - return [ - 'regex' => $regex, - 'parameters' => $parameterNames - ]; - } - protected function parseParameters($route, $url, $parameterRegex = '[\w]+') { - $result = $this->generateRouteRegEx($route, $parameterRegex); + $regex = sprintf(static::PARAMETERS_REGEX_MATCH, $this->paramModifiers[0], $this->paramOptionalSymbol, $this->paramModifiers[1]); - $parameterNames = $result['parameters']; + if (preg_match_all('/' . $regex . '/is', $route, $parameters)) { - if (preg_match('/^' . $result['regex'] . '\/?$/is', $url, $matches)) { + $parameterNamesRegex = []; + $parameterNames = $parameters[1]; + $parameterRequired = []; - $parameters = []; + $urlParts = preg_split('/\{[^}]+\}/is', rtrim($route, '/')); - $max = count($parameterNames) - 1; + foreach($urlParts as $key => $t) { - for ($i = $max; $i >= 0; $i--) { + $regex = ''; - $name = $parameterNames[$i]['name']; - $required = $parameterNames[$i]['required']; + if($key < (count($parameters[1]))) { - $param = isset($matches[$name]) ? $matches[$name] : null; + $name = $parameters[1][$key]; + $regex = isset($this->where[$name]) ? $this->where[$name] : $parameterRegex; + $regex = sprintf('(?P<%s>%s)', $name, $regex) . $parameters[2][$key]; - if ($required === true && isset($this->parameters[$name]) === true && trim($param) === '') { - $param = $this->parameters[$name]; } - $parameters[$name] = $param; + $urlParts[$key] = preg_quote($t, '/') . $regex; } - return $parameters; + $urlRegex = join('', $urlParts); + + } else { + $urlRegex = preg_quote($route, '/'); + } + + if(preg_match('/^' . $urlRegex . '(\/?)$/is', $url, $matches) > 0) { + + $values = []; + + /* Only take matched parameters with name */ + foreach($parameters[1] as $name) { + $values[$name] = $matches[$name]; + } + + return $values; } return null; @@ -419,10 +357,6 @@ abstract class Route implements IRoute $values['where'] = $this->where; } - /*if (count($this->parameters) > 0) { - $values['parameters'] = $this->parameters; - }*/ - if (count($this->middlewares) > 0) { $values['middleware'] = $this->middlewares; } diff --git a/src/Pecee/SimpleRouter/Route/RouteController.php b/src/Pecee/SimpleRouter/Route/RouteController.php index a47cf52..9d520fd 100644 --- a/src/Pecee/SimpleRouter/Route/RouteController.php +++ b/src/Pecee/SimpleRouter/Route/RouteController.php @@ -104,9 +104,6 @@ class RouteController extends LoadableRoute implements IControllerRoute $method = (!isset($path[0]) || trim($path[0]) === '') ? $this->defaultMethod : $path[0]; $this->method = $method; - //array_shift($path); - //$this->parameters = $path; - $this->parameters = array_slice($path, 1); // Set callback diff --git a/src/Pecee/SimpleRouter/Route/RouteGroup.php b/src/Pecee/SimpleRouter/Route/RouteGroup.php index acf5323..e24723b 100644 --- a/src/Pecee/SimpleRouter/Route/RouteGroup.php +++ b/src/Pecee/SimpleRouter/Route/RouteGroup.php @@ -171,6 +171,10 @@ class RouteGroup extends Route implements IGroupRoute $values['as'] = $this->name; } + if (count($this->parameters) > 0) { + $values['parameters'] = $this->parameters; + } + return array_merge($values, parent::toArray()); } diff --git a/src/Pecee/SimpleRouter/Route/RouteUrl.php b/src/Pecee/SimpleRouter/Route/RouteUrl.php index 0f2660d..9cc082f 100644 --- a/src/Pecee/SimpleRouter/Route/RouteUrl.php +++ b/src/Pecee/SimpleRouter/Route/RouteUrl.php @@ -24,12 +24,13 @@ class RouteUrl extends LoadableRoute /* Make regular expression based on route */ $parameters = $this->parseParameters($this->url, $url); - if($parameters !== null) { - $this->setParameters($parameters); - return true; + if($parameters === null) { + return false; } - return false; + $this->setParameters($parameters); + + return true; } diff --git a/test/RouterRouteTest.php b/test/RouterRouteTest.php index 23bdaff..5637f46 100644 --- a/test/RouterRouteTest.php +++ b/test/RouterRouteTest.php @@ -14,6 +14,26 @@ class RouterRouteTest extends PHPUnit_Framework_TestCase { protected $result = false; + public function testMultiParam() + { + SimpleRouter::router()->reset(); + SimpleRouter::request()->setMethod('get'); + SimpleRouter::request()->setUri('/test-param1-param2'); + + SimpleRouter::get('/test-{param1}-{param2}', function($param1, $param2) { + + if($param1 === 'param1' && $param2 === 'param2') { + $this->result = true; + } + + }); + + SimpleRouter::start(); + + $this->assertTrue($this->result); + + } + /** * Redirects to another route through 3 exception handlers. * @@ -118,16 +138,6 @@ class RouterRouteTest extends PHPUnit_Framework_TestCase SimpleRouter::start(); } - public function testMultiParam() - { - SimpleRouter::router()->reset(); - SimpleRouter::request()->setMethod('get'); - SimpleRouter::request()->setUri('/test-param1-param2'); - - SimpleRouter::get('/test-{param1}-{param2}', 'DummyController@param'); - SimpleRouter::start(); - } - public function testPathParamRegex() { SimpleRouter::router()->reset();