Adding CORS header concerns

[enriclluelles]

Hi there,

Thanks for this library. I added the handling of header whitelisting to it.

And at the same time tried to make some parts of the code it a little bit more readable. I'm a ruby guy who's only started doing clojure recently so even though I tried to be idiomatic I may haven't been.

Cheers

[r0man]

I think the Travis build is failing because the code doesn't pass the linter. Can you fix this? Make sure thatlein ci passes. Thx.

[enriclluelles]

Sorry, never ran the lein ci command locally. I changed the code to make the linting pass.

Although I gotta say, I think for a language like clojure where some constructs get nested really easily, I think that limiting line length to 80 chars can decrease legibility in some cases instead of improving it. Just my two cents

[r0man]

Thx. Released as 0.1.5.

[r0man]

Hi @enriclluelles,

I think we introduced a regression with your last PR. Previously everything that was passed to wrap-cors ended up as a header in the response, except if it needed special handling. The following configuration for example should also return a "Access-Control-Allow-Credentials" and "Access-Control-Expose-Headers" headers.

(cors/wrap-cors
 handler
 :access-control-allow-credentials "true"
 :access-control-allow-headers
 (str/join ", " ["accept" "authorization" "content-type"])
 :access-control-allow-methods [:get :delete :head :post :patch :put]
 :access-control-allow-origin #".*"
 :access-control-expose-headers
 (str/join ", " ["etag" "link" "x-total"]))

Do you have time to restore this behaviour?

Roman

[enriclluelles]

Yeah I will take a look between today and tomorrow, add a failing test and fix it. Sorry about the bug

On 23 Dec 2014, at 19:27, r0man notifications@github.com wrote:

Hi @enriclluelles,

I think we introduced a regression with your last PR. Previously everything that was passed to wrap-cors ended up as a header in the response, except if it needed special handling. The following configuration for example should also return a "Access-Control-Allow-Credentials" and "Access-Control-Expose-Headers" headers.

(cors/wrap-cors handler :access-control-allow-credentials "true" :access-control-allow-headers (str/join ", " ["accept" "authorization" "content-type"]) :access-control-allow-methods [:get :delete :head :post :patch :put] :access-control-allow-origin #".*" :access-control-expose-headers (str/join ", " ["etag" "link" "x-total"])) Do you have time to restore this behaviour?

Roman

— Reply to this email directly or view it on GitHub.

[r0man]

Cool. Thx!

On Tue, Dec 23, 2014 at 7:47 PM, Enric Lluelles notifications@github.com wrote:

Yeah I will take a look between today and tomorrow, add a failing test and fix it. Sorry about the bug

On 23 Dec 2014, at 19:27, r0man notifications@github.com wrote:

Hi @enriclluelles,

I think we introduced a regression with your last PR. Previously everything that was passed to wrap-cors ended up as a header in the response, except if it needed special handling. The following configuration for example should also return a "Access-Control-Allow-Credentials" and "Access-Control-Expose-Headers" headers.

(cors/wrap-cors handler :access-control-allow-credentials "true" :access-control-allow-headers (str/join ", " ["accept" "authorization" "content-type"]) :access-control-allow-methods [:get :delete :head :post :patch :put] :access-control-allow-origin #".*" :access-control-expose-headers (str/join ", " ["etag" "link" "x-total"])) Do you have time to restore this behaviour?

Roman

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/r0man/ring-cors/pull/7#issuecomment-67984726.

[frankiesardo]

We're currently stuck because we cannot set :access-control-allow-credentials "true".

Is there a quick workaround we can implement while this gets fixed?

[r0man]

@frankiesardo I think the regression introduced in this PR has already been fixed. I just added a test case for :access-control-allow-credentials over here https://github.com/r0man/ring-cors/commit/7418fe9764de32fb31437fa0b8f02f5b72c98d05 . What exactly is the problem? Patch with tests welcome!

[frankiesardo]

Yup it works! The error was caused by our mistake and by searching this library it seemed that this option was not supported. We've been too quick to blame others :smile: Thanks for adding the test case anyway, it makes it easier to see which keywords we can pass to wrap-cors.

[r0man]

Ok, cool. :)