Skip to content

Fixing CORS attribute - #544

Merged
habbes merged 1 commit into
OData:mainfrom
giulianob:cors-attribute
Apr 4, 2022
Merged

Fixing CORS attribute#544
habbes merged 1 commit into
OData:mainfrom
giulianob:cors-attribute

Conversation

@giulianob

Copy link
Copy Markdown
Contributor

Registering selectors with CORS when controller/action use CORS attributes. Fixes #534.

Registering selectors with CORS when controller/action use CORS attributes. Fixes OData#534.
@habbes

habbes commented Apr 4, 2022

Copy link
Copy Markdown
Contributor

Hi @marabooy check this out

@habbes
habbes requested review from marabooy and xuzhg April 4, 2022 05:54


// Check if CORS attribute is specified on action. New selectors need to be registered with CORS support.
bool acceptPreflight = action.Controller.Attributes.OfType<IDisableCorsAttribute>().Any() ||

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@giulianob Based on this statement, how does a controller action with neither EnableCors nor DisableCors return a value of true for AcceptCorsPreflight of HttpMethodMetadata instance?
RE: https://github.com/OData/AspNetCoreOData/pull/544/files#diff-b4918c8cb621def5a0806e8f4e36a80bf254f78da5f1bb2b27f2eb9a1afd1de7R123

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gathogojr the Get action in the tests does have an [EnableCors] attribute, see: https://github.com/OData/AspNetCoreOData/pull/544/files#diff-b4918c8cb621def5a0806e8f4e36a80bf254f78da5f1bb2b27f2eb9a1afd1de7R152

What doesn't have the attribute is the TestController.Index action. And for that one, the expected result is false.

@gathogojr gathogojr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EnableCors attribute does not work on OData controllers

4 participants