Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultIsOriginAllowed should StringComparer.Ordinal be changed to StringComparer.OrdinalIgnoreCase? #2373

Closed
aspnet-hello opened this issue Jan 1, 2018 · 2 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Comments

@aspnet-hello
Copy link

From @benhysell on Friday, August 11, 2017 11:58:50 AM

https://github.com/aspnet/CORS/blob/8214954d5b0dafe191890da9a3de86f5b607ea69/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsPolicy.cs#L161

Is the only place where string comparison is cased...should it ignore case?

Ran into an issue today where I built my own policy:

services.AddCors(options =>
                {
                    options.AddPolicy("AllowRequestFromWeb",
                        builder =>
                        {
                            builder.WithOrigins(Configuration["ServerOptions:AllowedCorsOrigins"])
                                .AllowAnyHeader()
                                .AllowAnyMethod()
                                .AllowCredentials();
                        });
                });

And I'm reading the configuration in from appsettings.json.

"ServerOptions": {    
    "AllowedCorsOrigins": "http://localhost"
  }

All worked great, but I was having an odd failure with my production appsettings.json. From appsettings.json.Production

"ServerOptions": {    
    "AllowedCorsOrigins": "http://MyCoolProductionServer"
  }

My CORS were failing.

The second I changed "http://MyCoolProductionServer" to lowercase everything worked as expected.

I realize that I cased my server name in my appsettings.json.Production, my mistake...but should that comparison be case sensitive?

Copied from original issue: aspnet/CORS#124

@aspnet-hello
Copy link
Author

From @blowdart on Friday, August 11, 2017 12:20:11 PM

It's complicated. So complicated there's an RFC - https://tools.ietf.org/html/rfc4343

Basically for ASCII domains it's case insensitive, but punycode (what Internationalised domain names convert into) kinda, sorta, messed it up.

However, we ought to normalise in our checks, and then allow case sensitivity as an option.

@pranavkm
Copy link
Contributor

This was addressed as part of aspnet/CORS@feceb3d. We still don't punycode i8n names, but this issue can be closed.

@Eilon Eilon added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed repo:CORS labels Nov 26, 2018
ryanbrandenburg pushed a commit that referenced this issue Nov 27, 2018
- VisualStudio defaults to adding a `<none>` link item when right click -> add existing item for Razor files; therefore, this also includes the knowledge of the "None" item group when finding Razor files.
- Added unit and functional tests to verify the new `FallbackRazorProjectHost` behavior.
- Added new schema items to represent the `Content` and `None` item type information (pulled from the project system repo).

#2373
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

3 participants