bramus / router

A lightweight and simple object oriented PHP Router
MIT License
1.09k stars 245 forks source link

Unhandled notice exception instead of 404 handling #190

Open idashevskii opened 2 years ago

idashevskii commented 2 years ago

Notice: this issue is applicable for any HTTP method. Lets use PUT for example. The Router throws 'Undefined index: PUT' PHP notice when there is attempt to call PUT method, and no PUT routers were registered. Snipet to reproduce:

$router = new \Bramus\Router\Router();
$router->set404(function(){
    echo 'Not found';
});
$router->run();

Expected: 404 handler triggered without PHP notices. Actual: PHP notice

idashevskii commented 2 years ago

Already fixed in https://github.com/bramus/router/commit/968d04f86dafa116abc5f84a9689902393e00ac5 but not yet released

idashevskii commented 2 years ago

Tested with https://github.com/bramus/router/commit/968d04f86dafa116abc5f84a9689902393e00ac5 and it still not works. There is no PHP notice, but 404 handler was not triggered. Here is my observation

  1. $match argument is unused:

    public function trigger404($match = null)
  2. Since 1-arg is unused, no need to pass it. So no needs in isset check, which is root reasoe of the issue.

    if (isset($this->afterRoutes[$this->requestedMethod])) {
    $this->trigger404($this->afterRoutes[$this->requestedMethod]);
    }

    My suggestion is to revert/adjust https://github.com/bramus/router/commit/968d04f86dafa116abc5f84a9689902393e00ac5 and just to remove transferred but unused argument to trigger404.

bramus commented 2 years ago

I’m open to a PR that would fix this.

liviocmachado commented 2 years ago

Sorry for the question but do you know when the fix will be available?