From 67955dfb3533306a63702da80de38be4a6529340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Wed, 15 Apr 2020 11:23:33 +0200 Subject: [PATCH] Prevent duplicated Vary headers in CORS processing Closes gh-24829 --- .../web/cors/DefaultCorsProcessor.java | 16 ++++++++++---- .../cors/reactive/DefaultCorsProcessor.java | 18 +++++++++++++--- .../web/cors/DefaultCorsProcessorTests.java | 14 ++++++++++++- .../reactive/DefaultCorsProcessorTests.java | 21 ++++++++++++++++++- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java index 9607ef684b..94e8fa5426 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ package org.springframework.web.cors; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import javax.servlet.http.HttpServletRequest; @@ -60,9 +61,16 @@ public class DefaultCorsProcessor implements CorsProcessor { public boolean processRequest(@Nullable CorsConfiguration config, HttpServletRequest request, HttpServletResponse response) throws IOException { - response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN); - response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD); - response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS); + Collection varyHeaders = response.getHeaders(HttpHeaders.VARY); + if (!varyHeaders.contains(HttpHeaders.ORIGIN)) { + response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN); + } + if (!varyHeaders.contains(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD)) { + response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD); + } + if (!varyHeaders.contains(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS)) { + response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS); + } if (!CorsUtils.isCorsRequest(request)) { return true; diff --git a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java index 840eddb80a..20f960d232 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -59,13 +59,25 @@ public class DefaultCorsProcessor implements CorsProcessor { ServerHttpRequest request = exchange.getRequest(); ServerHttpResponse response = exchange.getResponse(); - response.getHeaders().addAll(HttpHeaders.VARY, VARY_HEADERS); + HttpHeaders responseHeaders = response.getHeaders(); + + List varyHeaders = responseHeaders.get(HttpHeaders.VARY); + if (varyHeaders == null) { + responseHeaders.addAll(HttpHeaders.VARY, VARY_HEADERS); + } + else { + for (String header : VARY_HEADERS) { + if (!varyHeaders.contains(header)) { + responseHeaders.add(HttpHeaders.VARY, header); + } + } + } if (!CorsUtils.isCorsRequest(request)) { return true; } - if (response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN) != null) { + if (responseHeaders.getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN) != null) { logger.trace("Skip: response already contains \"Access-Control-Allow-Origin\""); return true; } diff --git a/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java b/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java index e655cc7158..1945509603 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -401,4 +401,16 @@ public class DefaultCorsProcessorTests { assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN); } + @Test + public void preventDuplicatedVaryHeaders() throws Exception { + this.request.setMethod(HttpMethod.GET.name()); + this.response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN); + this.response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD); + this.response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS); + + this.processor.processRequest(this.conf, this.request, this.response); + assertThat(this.response.getHeaders(HttpHeaders.VARY)).containsOnlyOnce(HttpHeaders.ORIGIN, + HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java b/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java index 8e317268f1..1360430d6e 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -421,6 +421,25 @@ public class DefaultCorsProcessorTests { assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); } + @Test + public void preventDuplicatedVaryHeaders() { + MockServerHttpRequest request = MockServerHttpRequest + .method(HttpMethod.GET, "http://domain1.example/test.html") + .header(HttpHeaders.ORIGIN, "http://domain1.example") + .build(); + ServerWebExchange exchange = MockServerWebExchange.from(request); + ServerHttpResponse response = exchange.getResponse(); + HttpHeaders responseHeaders = response.getHeaders(); + responseHeaders.add(VARY, ORIGIN); + responseHeaders.add(VARY, ACCESS_CONTROL_REQUEST_METHOD); + responseHeaders.add(VARY, ACCESS_CONTROL_REQUEST_HEADERS); + + this.processor.process(this.conf, exchange); + + assertThat(responseHeaders.get(VARY)).containsOnlyOnce(ORIGIN, + ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS); + } + private ServerWebExchange actualRequest() { return MockServerWebExchange.from(corsRequest(HttpMethod.GET));