search

agrueneberg / Corser

CORS middleware for Node.js
MIT License
91 stars 11 forks source link

Chrome and Firefox behavior is different when making HTTP-DELETE CORS requests #18

Closed hamburml closed 8 years ago

hamburml commented 8 years ago

Hello!

I am using deployd (https://github.com/deployd/deployd) which is using corser for cors-requests.

I found out that Firefox 48 and Chrome 52 are different when sending Method-Options Requests. I have an API and a Client. The API and the Client do not share the same domain, so CORS Requests are used. When deleting a resource on the API delete-method is used.

Firefox sends firefox_options_request_delete access-control-request-method: DELETE

Chrome sends chrome_options_request_delete access-control-request-method: DELETE req.headers['access-control-request-headers']: (empty string)

In line https://github.com/agrueneberg/Corser/blob/master/lib/corser.js#L129 the request-headers is used to find out if the browser wants to send custom headers. When this header is not send, the requestHeaders array is empty and the following https://github.com/agrueneberg/Corser/blob/master/lib/corser.js#L143 is true. But when using Chrome the header is present. The header "" (empty string) is searched but of course can't be found.

I am not sure but I think this is either a bug from chrome or from corser.

edit

Bug is from chrome. https://bugs.chromium.org/p/chromium/issues/detail?id=633729

agrueneberg commented 8 years ago

Ouch! Thanks for figuring out that it's a bug in Chrome! Do you think replacing the condition on line 129 with req.headers.hasOwnProperty("access-control-request-headers") && req.headers["access-control-request-headers"] !== "" is an adequate workaround? Feel free to send a pull request if you want to fix it :)

hamburml commented 8 years ago

Yeah, I think this would make a perfect workaround till the bug is fixed. I never send a pull request before :D Let's see...

agrueneberg commented 8 years ago

Let me know if you need help :)

agrueneberg commented 8 years ago

I've pushed version 2.0.1 to npm. I would appreciate if you could check if updating your dependencies fixes the issue.

hamburml commented 8 years ago

Yep, error is gone and I am happy.

agrueneberg commented 8 years ago

Great, thanks again for figuring out the cause of the issue!