Skip to content

Commit d02e4fb

Browse files
sdeleuzesdeleuze
committed
Add Vary:Access-Control-Request-Method/Headers CORS headers
This commit adds these 2 Vary headers in addition to the existing Origin one to avoid caching of Access-Control-Request-Method and Access-Control-Request-Headers headers which can be an issue when allowed methods or headers are unbounded and only the requested method or headers are returned in the response. Issue: SPR-16413
1 parent 857a5b0 commit d02e4fb

File tree

5 files changed

+139
-44
lines changed

5 files changed

+139
-44
lines changed

‎spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-201/ the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import java.io.IOException;
2020
import java.nio.charset.StandardCharsets;
2121
import java.util.ArrayList;
22+
import java.util.Arrays;
2223
import java.util.List;
2324
import javax.servlet.http.HttpServletRequest;
2425
import javax.servlet.http.HttpServletResponse;
@@ -121,7 +122,8 @@ protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse r
121122
String allowOrigin = checkOrigin(config, requestOrigin);
122123
HttpHeaders responseHeaders = response.getHeaders();
123124

124-
responseHeaders.add(HttpHeaders.VARY, HttpHeaders.ORIGIN);
125+
responseHeaders.addAll(HttpHeaders.VARY, Arrays.asList(HttpHeaders.ORIGIN,
126+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
125127

126128
if (allowOrigin == null) {
127129
logger.debug("Rejecting CORS request because '" + requestOrigin + "' origin is not allowed");

‎spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.web.cors.reactive;
1818

1919
import java.util.ArrayList;
20+
import java.util.Arrays;
2021
import java.util.List;
2122

2223
import org.apache.commons.logging.Log;
@@ -107,7 +108,8 @@ protected boolean handleInternal(ServerWebExchange exchange,
107108
ServerHttpResponse response = exchange.getResponse();
108109
HttpHeaders responseHeaders = response.getHeaders();
109110

110-
response.getHeaders().add(HttpHeaders.VARY, HttpHeaders.ORIGIN);
111+
response.getHeaders().addAll(HttpHeaders.VARY, Arrays.asList(HttpHeaders.ORIGIN,
112+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
111113

112114
String requestOrigin = request.getHeaders().getOrigin();
113115
String allowOrigin = checkOrigin(config, requestOrigin);

‎spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import org.springframework.mock.web.test.MockHttpServletRequest;
2727
import org.springframework.mock.web.test.MockHttpServletResponse;
2828

29+
import static org.hamcrest.Matchers.contains;
2930
import static org.junit.Assert.*;
3031

3132
/**
@@ -65,7 +66,8 @@ public void actualRequestWithOriginHeader() throws Exception {
6566

6667
this.processor.processRequest(this.conf, this.request, this.response);
6768
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
68-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
69+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
70+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
6971
assertEquals(HttpServletResponse.SC_FORBIDDEN, this.response.getStatus());
7072
}
7173

@@ -90,7 +92,8 @@ public void actualRequestWithOriginHeaderAndAllowedOrigin() throws Exception {
9092
assertEquals("*", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
9193
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_MAX_AGE));
9294
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS));
93-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
95+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
96+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
9497
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
9598
}
9699

@@ -108,7 +111,8 @@ public void actualRequestCredentials() throws Exception {
108111
assertEquals("http://domain2.com", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
109112
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
110113
assertEquals("true", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
111-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
114+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
115+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
112116
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
113117
}
114118

@@ -124,7 +128,8 @@ public void actualRequestCredentialsWithOriginWildcard() throws Exception {
124128
assertEquals("http://domain2.com", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
125129
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
126130
assertEquals("true", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
127-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
131+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
132+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
128133
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
129134
}
130135

@@ -136,7 +141,8 @@ public void actualRequestCaseInsensitiveOriginMatch() throws Exception {
136141

137142
this.processor.processRequest(this.conf, this.request, this.response);
138143
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
139-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
144+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
145+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
140146
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
141147
}
142148

@@ -154,7 +160,8 @@ public void actualRequestExposedHeaders() throws Exception {
154160
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS));
155161
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS).contains("header1"));
156162
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS).contains("header2"));
157-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
163+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
164+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
158165
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
159166
}
160167

@@ -166,7 +173,8 @@ public void preflightRequestAllOriginsAllowed() throws Exception {
166173
this.conf.addAllowedOrigin("*");
167174

168175
this.processor.processRequest(this.conf, this.request, this.response);
169-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
176+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
177+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
170178
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
171179
}
172180

@@ -178,7 +186,8 @@ public void preflightRequestWrongAllowedMethod() throws Exception {
178186
this.conf.addAllowedOrigin("*");
179187

180188
this.processor.processRequest(this.conf, this.request, this.response);
181-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
189+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
190+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
182191
assertEquals(HttpServletResponse.SC_FORBIDDEN, this.response.getStatus());
183192
}
184193

@@ -192,7 +201,8 @@ public void preflightRequestMatchedAllowedMethod() throws Exception {
192201
this.processor.processRequest(this.conf, this.request, this.response);
193202
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
194203
assertEquals("GET,HEAD", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS));
195-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
204+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
205+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
196206
}
197207

198208
@Test
@@ -202,7 +212,8 @@ public void preflightRequestTestWithOriginButWithoutOtherHeaders() throws Except
202212

203213
this.processor.processRequest(this.conf, this.request, this.response);
204214
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
205-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
215+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
216+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
206217
assertEquals(HttpServletResponse.SC_FORBIDDEN, this.response.getStatus());
207218
}
208219

@@ -214,7 +225,8 @@ public void preflightRequestWithoutRequestMethod() throws Exception {
214225

215226
this.processor.processRequest(this.conf, this.request, this.response);
216227
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
217-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
228+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
229+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
218230
assertEquals(HttpServletResponse.SC_FORBIDDEN, this.response.getStatus());
219231
}
220232

@@ -227,7 +239,8 @@ public void preflightRequestWithRequestAndMethodHeaderButNoConfig() throws Excep
227239

228240
this.processor.processRequest(this.conf, this.request, this.response);
229241
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
230-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
242+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
243+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
231244
assertEquals(HttpServletResponse.SC_FORBIDDEN, this.response.getStatus());
232245
}
233246

@@ -249,7 +262,8 @@ public void preflightRequestValidRequestAndConfig() throws Exception {
249262
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS));
250263
assertEquals("GET,PUT", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS));
251264
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_MAX_AGE));
252-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
265+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
266+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
253267
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
254268
}
255269

@@ -270,7 +284,8 @@ public void preflightRequestCredentials() throws Exception {
270284
assertEquals("http://domain2.com", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
271285
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
272286
assertEquals("true", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS));
273-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
287+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
288+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
274289
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
275290
}
276291

@@ -289,7 +304,8 @@ public void preflightRequestCredentialsWithOriginWildcard() throws Exception {
289304
this.processor.processRequest(this.conf, this.request, this.response);
290305
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
291306
assertEquals("http://domain2.com", this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
292-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
307+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
308+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
293309
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
294310
}
295311

@@ -310,7 +326,8 @@ public void preflightRequestAllowedHeaders() throws Exception {
310326
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("Header1"));
311327
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("Header2"));
312328
assertFalse(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("Header3"));
313-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
329+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
330+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
314331
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
315332
}
316333

@@ -329,7 +346,8 @@ public void preflightRequestAllowsAllHeaders() throws Exception {
329346
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("Header1"));
330347
assertTrue(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("Header2"));
331348
assertFalse(this.response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS).contains("*"));
332-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
349+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
350+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
333351
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
334352
}
335353

@@ -345,7 +363,8 @@ public void preflightRequestWithEmptyHeaders() throws Exception {
345363
this.processor.processRequest(this.conf, this.request, this.response);
346364
assertTrue(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
347365
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS));
348-
assertEquals(HttpHeaders.ORIGIN, this.response.getHeader(HttpHeaders.VARY));
366+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
367+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
349368
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
350369
}
351370

0 commit comments

Comments
 (0)