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

Local @CrossOrigin maxAge value should override global value #26619

Closed

Conversation

GungnirLaevatain
Copy link
Contributor

The attribute of CrossOrigin that be applied to method will override that be applied to class, like origins, but maxAge is different,so I think we would modify it

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 27, 2021
@sbrannen sbrannen self-assigned this Feb 28, 2021
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 28, 2021
@sbrannen sbrannen added this to the 5.3.5 milestone Feb 28, 2021
@sbrannen
Copy link
Member

The Javadoc for @CrossOrigin states:

The rules for combining global and local configuration are generally additive -- e.g. all global and all local origins. For those attributes where only a single value can be accepted such as allowCredentials and maxAge, the local overrides the global value.

In light of that, I have labeled this as a bug to be fixed in 5.3.5 and backported to 5.2.14.

Good catch!

@sbrannen sbrannen changed the title modify Access-Control-Max-Age effective priority Local @CrossOrigin maxAge value should override global value Feb 28, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x labels Feb 28, 2021
@sbrannen sbrannen closed this in 90de1ab Mar 2, 2021
sbrannen added a commit that referenced this pull request Mar 2, 2021
sbrannen pushed a commit that referenced this pull request Mar 2, 2021
Prior to this commit, a method-level @crossorigin maxAge value did not
override a class-level @crossorigin maxAge value. This contradicts the
Javadoc for @Crossorgin which states the following.

    For those attributes where only a single value can be accepted such
    as allowCredentials and maxAge, the local overrides the global
    value.

This commit ensures that a method-level @crossorigin maxAge value
overrides a class-level @crossorigin maxAge value.

Closes gh-26619
sbrannen added a commit that referenced this pull request Mar 2, 2021
@sbrannen
Copy link
Member

sbrannen commented Mar 2, 2021

This has been merged into master in 90de1ab, revised in 01c2e12, and backported to 5.2.x.

Thanks


For future reference, please make sure to run ./gradlew check before submitting a PR in order to catch and correct any Checkstyle violations such as the unused imports in this PR.

This was referenced Mar 17, 2021
@GungnirLaevatain GungnirLaevatain deleted the patch-1 branch June 17, 2021 05:09
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
Prior to this commit, a method-level @crossorigin maxAge value did not
override a class-level @crossorigin maxAge value. This contradicts the
Javadoc for @Crossorgin which states the following.

    For those attributes where only a single value can be accepted such
    as allowCredentials and maxAge, the local overrides the global
    value.

This commit ensures that a method-level @crossorigin maxAge value
overrides a class-level @crossorigin maxAge value.

Closes spring-projectsgh-26619
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants