Skip to content

Commit

Permalink
Ensure local @crossorigin maxAge overrides global value
Browse files Browse the repository at this point in the history
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
  • Loading branch information
GungnirLaevatain authored and sbrannen committed Mar 2, 2021
1 parent 428dbc4 commit e8f685e
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ else if (!allowCredentials.isEmpty()) {
"or an empty string (\"\"): current value is [" + allowCredentials + "]");
}

if (annotation.maxAge() >= 0 && config.getMaxAge() == null) {
if (annotation.maxAge() >= 0) {
config.setMaxAge(annotation.maxAge());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
Expand All @@ -37,6 +38,7 @@
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.reactive.config.EnableWebFlux;
import org.springframework.web.testfixture.http.server.reactive.bootstrap.HttpServer;

Expand Down Expand Up @@ -257,6 +259,19 @@ void ambiguousProducesPreflightRequest(HttpServer httpServer) throws Exception {
assertThat(entity.getHeaders().getAccessControlAllowCredentials()).isTrue();
}

@ParameterizedHttpServerTest
void maxAgeWithDefaultOrigin(HttpServer httpServer) throws Exception {
startServer(httpServer);

this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
ResponseEntity<String> entity = performOptions("/classAge", this.headers, String.class);
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(entity.getHeaders().getAccessControlMaxAge()).isEqualTo(10);

entity = performOptions("/methodAge", this.headers, String.class);
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(entity.getHeaders().getAccessControlMaxAge()).isEqualTo(100);
}

@Configuration
@EnableWebFlux
Expand Down Expand Up @@ -361,4 +376,21 @@ public String baz() {
}
}

@RestController
@CrossOrigin(maxAge = 10)
private static class MaxAgeWithDefaultOriginController {

@CrossOrigin
@GetMapping(path = "/classAge")
public String classAge() {
return "classAge";
}

@CrossOrigin(maxAge = 100)
@GetMapping(path = "/methodAge")
public String methodAge() {
return "methodAge";
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ else if (!allowCredentials.isEmpty()) {
"or an empty string (\"\"): current value is [" + allowCredentials + "]");
}

if (annotation.maxAge() >= 0 && config.getMaxAge() == null) {
if (annotation.maxAge() >= 0) {
config.setMaxAge(annotation.maxAge());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,27 @@ public void preFlightRequestWithoutRequestMethodHeader() throws Exception {
assertThat(this.handlerMapping.getHandler(request)).isNull();
}

@Test
public void maxAgeWithDefaultOrigin() throws Exception {
this.handlerMapping.registerHandler(new MaxAgeWithDefaultOriginController());

this.request.setRequestURI("/classAge");
HandlerExecutionChain chain = this.handlerMapping.getHandler(request);
CorsConfiguration config = getCorsConfiguration(chain, false);
assertThat(config).isNotNull();
assertThat(config.getAllowedMethods()).containsExactly("GET");
assertThat(config.getAllowedOrigins()).containsExactly("*");
assertThat(config.getMaxAge()).isEqualTo(10);

this.request.setRequestURI("/methodAge");
chain = this.handlerMapping.getHandler(request);
config = getCorsConfiguration(chain, false);
assertThat(config).isNotNull();
assertThat(config.getAllowedMethods()).containsExactly("GET");
assertThat(config.getAllowedOrigins()).containsExactly("*");
assertThat(config.getMaxAge()).isEqualTo(100);
}


private CorsConfiguration getCorsConfiguration(HandlerExecutionChain chain, boolean isPreFlightRequest) {
if (isPreFlightRequest) {
Expand Down Expand Up @@ -425,7 +446,21 @@ public void bar() {
@RequestMapping(path = "/baz", method = RequestMethod.GET)
public void baz() {
}
}

@Controller
@CrossOrigin(maxAge = 10)
private static class MaxAgeWithDefaultOriginController {

@CrossOrigin
@RequestMapping(path = "/classAge", method = RequestMethod.GET)
public void classAge() {
}

@CrossOrigin(maxAge = 100)
@RequestMapping(path = "/methodAge", method = RequestMethod.GET)
public void methodAge() {
}
}


Expand Down

0 comments on commit e8f685e

Please sign in to comment.