Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

Make preflight request header check case-insensitive #29

Closed
wants to merge 2 commits into from
Closed

Make preflight request header check case-insensitive #29

wants to merge 2 commits into from

Conversation

Norgerman
Copy link
Contributor

In jquery ajax(maybe in old versions), the Access-Control-Request-Headers in OPTIONS request value is lower case and will contains accept header which is already in SimpleRequestHeaders

…request headers

Signed-off-by: Norgerman <xyn0410@gmail.com>
@dnfclas
Copy link

dnfclas commented Aug 25, 2015

Hi @Norgerman, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link

dnfclas commented Aug 25, 2015

@Norgerman, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@Eilon Eilon changed the title Make perflight request header check case-insensitive Make preflight request header check case-insensitive Sep 8, 2015
@Eilon
Copy link
Contributor

Eilon commented Sep 8, 2015

Logged a bug to track this: #32

@kichalla can you review this PR?

@@ -97,7 +97,8 @@ public virtual void EvaluatePreflightRequest(HttpContext context, CorsPolicy pol

if (!policy.AllowAnyHeader &&
requestHeaders != null &&
!requestHeaders.All(header => policy.Headers.Contains(header, StringComparer.Ordinal)))
!requestHeaders.All(header => CorsConstants.SimpleRequestHeaders.Contains(header, StringComparer.OrdinalIgnoreCase) ||
policy.Headers.Contains(header, StringComparer.OrdinalIgnoreCase)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change correct? This is supposed to be case-sensitive for any header other than non-simple + content-type header, right?...like can you add a test where the header in the request is "version" and the policy is "Version"...this should not match..

@kichalla
Copy link
Member

kichalla commented Sep 8, 2015

@Norgerman
Copy link
Contributor Author

I have tested this use jquery ajax in chrome, Access-Control-Request-Headers are always in lower case. So the server is not able to get Access-Control-Request-Headers values like Version or verSion, the value will always be version. Thanks for your review.

@kichalla
Copy link
Member

kichalla commented Sep 9, 2015

:shipit:

@kichalla
Copy link
Member

kichalla commented Sep 9, 2015

commit 38728a6

Thanks for the contribution! @Norgerman

@kichalla kichalla closed this Sep 9, 2015
@Norgerman
Copy link
Contributor Author

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants