search

larskuhtz / wai-cors

A Haskell implementation of Cross-Origin resource sharing (CORS) for Wai
MIT License
26 stars 13 forks source link

give `simpleHeaders` to `simpleCorsResourcePolicy` #26

Open 2mol opened 5 years ago

2mol commented 5 years ago

It makes more sense to give simpleHeaders explicitely to the simpleCorsResourcePolicy, and by extension to simpleCors.

The reason for this is that the preflight request chrome and firefox make vie OPTIONS includes content-type, resulting in a 400. As the documentation states, all simpleHeaders except content-type are implicit when passing the empty list to corsRequestHeaders.

I hope this is ok, let me know if I misunderstood the intention regarding simpleCors.

guaraqe commented 4 years ago

I had the same problems, and the fix above solved it. In my opinions this gives a better out-of-the-box experience.

larskuhtz commented 4 years ago

I hesitate to take this change without carefully reading the related sections in the CORS standard. The behavior of the server guides the browser to reject unsafe scenarios. Thus, a more liberal policy is always more convenient, but, generally, not safer.

I am happy to merge this when I am convinced that it is in accordance with the standard, doesn't affect security for other users of the library, and doesn't break existing uses. In case of doubt, I prefer some inconvenience for some use cases.

larskuhtz commented 4 years ago

Unfortunately, I am not working with CORS on a daily basis and re-familiarizing myself with the standard always takes some time, which is the reason why this package is maintained rather conservatively.

larskuhtz commented 4 years ago

I appreciate that people use this package and file issues and submit PRs. And am sorry, that I am slow in keeping up with responding.

larskuhtz commented 4 years ago

In concrete, I am concerned that the change in the PR may conflict with the following paragraph from section 6.2 of the CORS standard:

  1. If each of the header field-names is a simple header and none is Content-Type, this step may be skipped.

    Add one or more Access-Control-Allow-Headers headers consisting of (a subset of) the list of headers.

    Note: If a header field name is a simple header and is not Content-Type, it is not required to be listed. Content-Type is to be listed as only a subset of its values makes it qualify as simple header.

    Note: Since the list of headers can be unbounded, simply returning supported headers from Access-Control-Allow-Headers can be enough.

2mol commented 4 years ago

I appreciate that people use this package and file issues and submit PRs. And am sorry, that I am slow in keeping up with responding.

No need to apologise! I for one appreciate that you are careful and methodical about these changes, I agree with your points about being conservative instead of convenient.

I have to admit I have to spend more time parsing that section 6.2 to understand it :)