From 4ee9e21a4f5e177f3815c89d309af5a005bd88c3 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 11 Oct 2019 11:55:02 -0400 Subject: [PATCH] Make Router::getModuleClass throw exceptions - Add new MethodNotAllowedModule - Add new Module->determineClass catch blocks - Update Module and Router tests --- src/App/Module.php | 54 ++++++++------- src/App/Router.php | 19 +++-- src/Module/HTTPException/MethodNotAllowed.php | 15 ++++ .../{ => HTTPException}/PageNotFound.php | 2 +- tests/src/App/ModuleTest.php | 4 +- tests/src/App/RouterTest.php | 69 ++++++++++++++++--- 6 files changed, 122 insertions(+), 41 deletions(-) create mode 100644 src/Module/HTTPException/MethodNotAllowed.php rename src/Module/{ => HTTPException}/PageNotFound.php (85%) diff --git a/src/App/Module.php b/src/App/Module.php index 9a24c55544..33a9b2fc2f 100644 --- a/src/App/Module.php +++ b/src/App/Module.php @@ -7,7 +7,10 @@ use Friendica\BaseObject; use Friendica\Core; use Friendica\LegacyModule; use Friendica\Module\Home; -use Friendica\Module\PageNotFound; +use Friendica\Module\HTTPException\MethodNotAllowed; +use Friendica\Module\HTTPException\PageNotFound; +use Friendica\Network\HTTPException\MethodNotAllowedException; +use Friendica\Network\HTTPException\NotFoundException; use Psr\Log\LoggerInterface; /** @@ -144,38 +147,43 @@ class Module { $printNotAllowedAddon = false; + $module_class = null; /** * ROUTING * * From the request URL, routing consists of obtaining the name of a BaseModule-extending class of which the * post() and/or content() static methods can be respectively called to produce a data change or an output. **/ - $module_class = $router->getModuleClass($args->getCommand()); - - // Then we try addon-provided modules that we wrap in the LegacyModule class - if (!$module_class && Core\Addon::isEnabled($this->module) && file_exists("addon/{$this->module}/{$this->module}.php")) { - //Check if module is an app and if public access to apps is allowed or not - $privateapps = $config->get('config', 'private_addons', false); - if ((!local_user()) && Core\Hook::isAddonApp($this->module) && $privateapps) { - $printNotAllowedAddon = true; - } else { - include_once "addon/{$this->module}/{$this->module}.php"; - if (function_exists($this->module . '_module')) { - LegacyModule::setModuleFile("addon/{$this->module}/{$this->module}.php"); - $module_class = LegacyModule::class; + try { + $module_class = $router->getModuleClass($args->getCommand()); + } catch (MethodNotAllowedException $e) { + $module_class = MethodNotAllowed::class; + } catch (NotFoundException $e) { + // Then we try addon-provided modules that we wrap in the LegacyModule class + if (Core\Addon::isEnabled($this->module) && file_exists("addon/{$this->module}/{$this->module}.php")) { + //Check if module is an app and if public access to apps is allowed or not + $privateapps = $config->get('config', 'private_addons', false); + if ((!local_user()) && Core\Hook::isAddonApp($this->module) && $privateapps) { + $printNotAllowedAddon = true; + } else { + include_once "addon/{$this->module}/{$this->module}.php"; + if (function_exists($this->module . '_module')) { + LegacyModule::setModuleFile("addon/{$this->module}/{$this->module}.php"); + $module_class = LegacyModule::class; + } } } - } - /* Finally, we look for a 'standard' program module in the 'mod' directory - * We emulate a Module class through the LegacyModule class - */ - if (!$module_class && file_exists("mod/{$this->module}.php")) { - LegacyModule::setModuleFile("mod/{$this->module}.php"); - $module_class = LegacyModule::class; - } + /* Finally, we look for a 'standard' program module in the 'mod' directory + * We emulate a Module class through the LegacyModule class + */ + if (!$module_class && file_exists("mod/{$this->module}.php")) { + LegacyModule::setModuleFile("mod/{$this->module}.php"); + $module_class = LegacyModule::class; + } - $module_class = !isset($module_class) ? PageNotFound::class : $module_class; + $module_class = $module_class ?: PageNotFound::class; + } return new Module($this->module, $module_class, $this->isBackend, $printNotAllowedAddon); } diff --git a/src/App/Router.php b/src/App/Router.php index 1bdfdcab1d..f723321ac6 100644 --- a/src/App/Router.php +++ b/src/App/Router.php @@ -8,7 +8,8 @@ use FastRoute\Dispatcher; use FastRoute\RouteCollector; use FastRoute\RouteParser\Std; use Friendica\Core\Hook; -use Friendica\Network\HTTPException\InternalServerErrorException; +use Friendica\Core\L10n; +use Friendica\Network\HTTPException; /** * Wrapper for FastRoute\Router @@ -57,7 +58,7 @@ class Router * * @return self The router instance with the loaded routes * - * @throws InternalServerErrorException In case of invalid configs + * @throws HTTPException\InternalServerErrorException In case of invalid configs */ public function addRoutes(array $routes) { @@ -71,7 +72,7 @@ class Router } elseif ($this->isRoute($config)) { $routeCollector->addRoute($config[1], $route, $config[0]); } else { - throw new InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'"); + throw new HTTPException\InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'"); } } @@ -96,7 +97,7 @@ class Router } elseif ($this->isRoute($config)) { $routeCollector->addRoute($config[1], $route, $config[0]); }else { - throw new InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'"); + throw new HTTPException\InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'"); } } }); @@ -155,7 +156,11 @@ class Router * * @param string $cmd The path component of the request URL without the query string * - * @return string|null A Friendica\BaseModule-extending class name if a route rule matched + * @return string A Friendica\BaseModule-extending class name if a route rule matched + * + * @throws HTTPException\InternalServerErrorException + * @throws HTTPException\MethodNotAllowedException If a rule matched but the method didn't + * @throws HTTPException\NotFoundException If no rule matched */ public function getModuleClass($cmd) { @@ -171,6 +176,10 @@ class Router $routeInfo = $dispatcher->dispatch($this->httpMethod, $cmd); if ($routeInfo[0] === Dispatcher::FOUND) { $moduleClass = $routeInfo[1]; + } elseif ($routeInfo[0] === Dispatcher::METHOD_NOT_ALLOWED) { + throw new HTTPException\MethodNotAllowedException(L10n::t('Method not allowed for this module. Allowed method(s): %s', implode(', ', $routeInfo[1]))); + } else { + throw new HTTPException\NotFoundException(L10n::t('Page not found.')); } return $moduleClass; diff --git a/src/Module/HTTPException/MethodNotAllowed.php b/src/Module/HTTPException/MethodNotAllowed.php new file mode 100644 index 0000000000..8d2d280a59 --- /dev/null +++ b/src/Module/HTTPException/MethodNotAllowed.php @@ -0,0 +1,15 @@ +determineModule(new App\Arguments(), []); + $moduleNew = $module->determineModule(new App\Arguments()); $this->assertNotSame($moduleNew, $module); } diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index 2ed8574c43..2fcc02ff51 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -4,13 +4,15 @@ namespace Friendica\Test\src\App; use Friendica\App\Router; use Friendica\Module; +use Friendica\Network\HTTPException\MethodNotAllowedException; +use Friendica\Network\HTTPException\NotFoundException; use PHPUnit\Framework\TestCase; class RouterTest extends TestCase { public function testGetModuleClass() { - $router = new Router(['GET']); + $router = new Router(['REQUEST_METHOD' => 'GET']); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute(['GET'], '/', 'IndexModuleClassName'); @@ -22,23 +24,70 @@ class RouterTest extends TestCase $routeCollector->addRoute(['POST', 'PUT', 'PATCH', 'DELETE', 'HEAD'], '/unsupported', 'UnsupportedMethodModuleClassName'); $this->assertEquals('IndexModuleClassName', $router->getModuleClass('/')); - $this->assertEquals('TestModuleClassName', $router->getModuleClass('/test')); - $this->assertNull($router->getModuleClass('/tes')); - $this->assertEquals('TestSubModuleClassName', $router->getModuleClass('/test/sub')); - $this->assertEquals('OptionalModuleClassName', $router->getModuleClass('/optional')); $this->assertEquals('OptionalModuleClassName', $router->getModuleClass('/optional/option')); - $this->assertNull($router->getModuleClass('/optional/opt')); - $this->assertEquals('VariableModuleClassName', $router->getModuleClass('/variable/123abc')); - $this->assertNull($router->getModuleClass('/variable')); - $this->assertEquals('OptionalVariableModuleClassName', $router->getModuleClass('/optionalvariable')); $this->assertEquals('OptionalVariableModuleClassName', $router->getModuleClass('/optionalvariable/123abc')); + } + + public function testGetModuleClassNotFound() + { + $this->expectException(NotFoundException::class); + + $router = new Router(['REQUEST_METHOD' => 'GET']); + + $router->getModuleClass('/unsupported'); + } + + public function testGetModuleClassNotFoundTypo() + { + $this->expectException(NotFoundException::class); + + $router = new Router(['REQUEST_METHOD' => 'GET']); + + $routeCollector = $router->getRouteCollector(); + $routeCollector->addRoute(['GET'], '/test', 'TestModuleClassName'); + + $router->getModuleClass('/tes'); + } + + public function testGetModuleClassNotFoundOptional() + { + $this->expectException(NotFoundException::class); + + $router = new Router(['REQUEST_METHOD' => 'GET']); + + $routeCollector = $router->getRouteCollector(); + $routeCollector->addRoute(['GET'], '/optional[/option]', 'OptionalModuleClassName'); + + $router->getModuleClass('/optional/opt'); + } + + public function testGetModuleClassNotFoundVariable() + { + $this->expectException(NotFoundException::class); + + $router = new Router(['REQUEST_METHOD' => 'GET']); + + $routeCollector = $router->getRouteCollector(); + $routeCollector->addRoute(['GET'], '/variable/{var}', 'VariableModuleClassName'); + + $router->getModuleClass('/variable'); + } + + public function testGetModuleClassMethodNotAllowed() + { + $this->expectException(MethodNotAllowedException::class); + + $router = new Router(['REQUEST_METHOD' => 'POST']); + + $routeCollector = $router->getRouteCollector(); + $routeCollector->addRoute(['GET'], '/test', 'TestModuleClassName'); - $this->assertNull($router->getModuleClass('/unsupported')); + $router->getModuleClass('/test'); } public function dataRoutes() -- 2.39.5