jdesrosiers / silex-cors-provider

A silex service provider that adds CORS services to silex
MIT License
78 stars 25 forks source link

Having HTTP 405 Method not allowed response when used with annotations #33

Closed alcalyn closed 7 years ago

alcalyn commented 7 years ago

I'm trying to make a PUT HTTP request:

The browser then send the preflight request:

Request:

OPTIONS /index-dev.php/api/teams/4/users/32 HTTP/1.1
Host: localhost:20000
Access-Control-Request-Method: PUT
origin: http://localhost
Connection: keep-alive

Response:

HTTP/1.1 405 Method Not Allowed
Server: nginx/1.11.8
Connection: keep-alive
Allow: PUT
Access-Control-Allow-Methods: PUT
Access-Control-Allow-Origin: http://localhost

I can see the cors headers are well set, but it keeps the 405 status code and the silex error page in body which is saying that the OPTIONS method is not allowed for this controller.

I don't want to make all my controllers match the OPTIONS method.

I can see that the cors servcie does its jobs, it adds cors headers on response, but on the same response which is actually the error response.

alcalyn commented 7 years ago

So adding the snippet code I posted in the related issue, it fixes the thing, even if it does it on any request, I think I can adapt the snippet to make it work correctly.

But I think the logic is to send a new response before running the controller instead of using the response from the controller, add cors headers and sending it with the wrong status code and body.

alcalyn commented 7 years ago

In fact this happens on controllers registered with annotations: https://github.com/danadesrosiers/silex-annotation-provider

Not working:

use DDesrosiers\SilexAnnotations\Annotations as SLX;

/**
 * @SLX\Route(
 *      @SLX\Request(method="PUT", uri="/api/my-route")
 * )
 */
public function addUser()
{
    // ...
}

Working:

$this['app.controller.my'] = function () {
    return new \App\Controller\MyController();
};

$this->put('/api/my-route', 'app.controller.my:addUser');
jdesrosiers commented 7 years ago

Thanks. I'll check it out

jdesrosiers commented 7 years ago

Sorry, I haven't had a chance to look into this yet. Hopefully I'll have time it in the next couple days. I just want to let you know I haven't forgotten.

jdesrosiers commented 7 years ago

The way the silex-cor-provider works is that parses all of your routes and automatically creates and registers the necessary OPTIONS routes that are needed for CORS. Your code is pretty much a lower level (and less complete) version of what the silex-cor-provider already does.

The problem you are running into might be that the silex-annotation-provider is somehow creating routes after silex-cors-provider generates OPTIONS routes. I wasn't able to reproduce the problem, so it's hard to say without seeing it fail.

I have an idea for a refactor that is similar to the code that is working for you. I'll work on it and ask you to try it with your failing case when I'm done.

jdesrosiers commented 7 years ago

I just figured out how to reproduce your problem. The problem is the order in which you register your service providers. You must register silex-annotation-provider before silex-cors-provider. Providers are executed in the order they are registered, so if silex-annotation-provider doesn't run first, there are no routes for silex-cor-provider to generate OPTIONS methods for.

jdesrosiers commented 7 years ago

The new OPTIONS implementation is available on master if you want to try it out. This one should work no matter what order providers are registered.

alcalyn commented 7 years ago

Whoa ok I'll test to change the order of providers, just curiousity, then I'll test with your update

alcalyn commented 7 years ago

Hi, I tried your new implementation, everything works: the wildcard, and whatever if annotations are registered before/after cors provider. Thanks.