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

Support to handle Options request using middleware stack #13

Closed
wants to merge 3 commits into from
Closed

Support to handle Options request using middleware stack #13

wants to merge 3 commits into from

Conversation

barbosa89
Copy link

Hi devs,

This pull request intends to support requests using the OPTIONS method of the HTTP protocol for the specific case in which the route is found but the method is not allowed. The use cases proposed are the following:

  1. If the path of the request with the OPTIONS method is not found, respond normally with 404.

  2. If the path of the request with the OPTIONS method is found but the method does not match, it is considered found and the middleware stack will be applied. If there is no middleware defined that handles the request with the OPTIONS method, it responds with 405. If the middleware exists, the response defined in its logic will be constructed.

In any case, I appreciate your attention and I hope to contribute to the beautiful Amp ecosystem. If there is a better way to support this feature, I will be very attentive to feedback.

@trowski
Copy link
Member

trowski commented May 10, 2024

OPTIONS is a different HTTP verb, and as such a route should be provided to handle it. You would not want a handler expecting another verb to receive an OPTIONS request.

@trowski trowski closed this May 10, 2024
@barbosa89
Copy link
Author

Thanks for your time, I suspect you didn't read the code because any request with the OPTIONS method will never be handled by controladores but by the disallowed methods handler, in any case, thank you very much, Amphp is great.

@trowski
Copy link
Member

trowski commented May 10, 2024

I was referring to the middleware stack being invoked for a method which was not defined by the router. That middleware stack may not be expecting an OPTIONS request. It is certainly something which would need documentation and would require another major version release as it would be a serious behavior change. Sorry if I was a bit terse in my first reply.

The easiest thing to do if want all routes to reply to an OPTIONS request is to wrap the Router with the middleware instead.

$requestHandler = stackMiddleware($router, new class implements Middleware {
    public function handleRequest(Request $request, RequestHandler $requestHandler): Response
    {
        if ($request->getMethod() === 'OPTIONS') {
            return new Response(HttpStatus::OK, ['Access-Control-Allow-Origin' => '*']);
        }

        return $requestHandler->handleRequest($request);
    }
});

Remember that the router needn't be the top-level request handler. You can have other request handlers or middlewares before the router to handle logic for every request.

@barbosa89
Copy link
Author

Ohh @trowski, thank you very much, a solution at a higher level than the router, it's great, thank you very much again, this is what I needed.

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

Successfully merging this pull request may close these issues.

3 participants