Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Retrieve allowed method for route #148

Open
piotr-cz opened this issue Nov 6, 2017 · 6 comments
Open

Feature request: Retrieve allowed method for route #148

piotr-cz opened this issue Nov 6, 2017 · 6 comments

Comments

@piotr-cz
Copy link

piotr-cz commented Nov 6, 2017

I'm preparing CORS middleware for REST API, which is appending list of allowed methods to a preflight request route.

As I'm using Slim framework v3, this is not possible append middleware headers in the notAllowed error handler (which is aware of allowed methods) as error handlers don't trigger middlewares.

Suggested way to do it to collect all methods for with same endpoint in the application using strict comparison which doesn't patterns.

My workaround is to create a fake request, dispatch route and collect allowed methods from router:

$methods = [];

$noopRouteInfo = $this->router->dispatch($request->withMethod('NOOP'));

if ($noopRouteInfo[0] === Dispatcher::METHOD_NOT_ALLOWED) {
    $methods = $noopRouteInfo[1];
}

unset ($noopRouteInfo);

What I'm looking for is a helper function on dispatcher

$methods = $dispatcher->getMethods($request);
@nikic
Copy link
Owner

nikic commented Dec 19, 2017

Sounds like a reasonable feature, and the code is basically all there already: https://github.com/nikic/FastRoute/blob/master/src/Dispatcher/RegexBasedAbstract.php#L59 Adding a method to the Dispatcher interface would be a BC break though :/

@designermonkey
Copy link

designermonkey commented Mar 18, 2018

Wouldn't you introduce another interface to allow addition of methods though? That would be the more SOLID way of doing it. Something like MethodCollectingDispatcher or similar?

@lcobucci
Copy link
Collaborator

As @nikic said, this is quite interesting to be added. @burzum is working on extracting an object for the dispatch result, this would be a nice thing to consider.

@kktsvetkov
Copy link

I have a question regarding the method and URI. Look at the 405 definition:

The HyperText Transfer Protocol (HTTP) 405 Method Not Allowed response status code indicates that the server knows the request method, but the target resource doesn't support this method.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405

It seems that you have to first find the target resource and then check what methods can be used on it. In RegexBasedAbstract::dispatch() however, it seems this happens in reverse as the routes are organized by HTTP method first. In other words, it is checking what routes are allowed for a method, while the intention behind the definition of 405 is to first identify the resource and then to check if the method is applicable.

Have you considered such a way of dispatching -- where first you try to find a match against the $uri, and then check what methods are allowed for it? It seems that with this approach you will have to traverse the route collection only once as there will be no need to check multiple times -- like when checking for HEAD or for fallback routes or for the allowed method. Once there is a match you will know what methods are associated with that route and can use them for the allowed method header. This will also solve that initial comment request about fetching what methods are associated with a route.

@lcobucci
Copy link
Collaborator

Have you considered such a way of dispatching -- where first you try to find a match against the $uri, and then check what methods are allowed for it?

@kktsvetkov that's kind of the direction I'm going with #244. There are some small important things to be addressed (the pattern matching gets a bit more complicated)

@kktsvetkov
Copy link

@lcobucci @nikic can we just extract the code for the allowed methods on a separate allowedMethods() method, without actually adding the same method in the Dispatcher interface?

Or perhaps have a new interface just for allowedMethods() and then have RegexBasedAbstract implement both interfaces ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants