amphp / http-server-router

A router for Amp's HTTP Server.
MIT License
38 stars 6 forks source link

Support to handle Options request using middleware stack #13

Closed barbosa89 closed 5 months ago

barbosa89 commented 5 months ago

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 commented 5 months ago

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.

barbosa89 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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.