Skip to content

Commit 9410998

Browse files
committed
Add caching headers to If-Unmodified-Since responses
Conditional requests using "If-Unmodified-Since" headers are generally used as precondition checks for state-changing methods (POST, PUT, DELETE). See https://datatracker.ietf.org/doc/html/rfc7232#section-3.4 The spec also allows for idempotent methods like GET and HEAD. Prior to this commit, the "If-Unmodified-Since" processing done in `checkNotModified` (see `ServletWebRequest` and `DefaultServerWebExchange`) would only focus on the state changing methods and not take into account the safe methods. For those cases, the "ETag" and "Last-Modified" would be missing from the response. This commit ensures that such headers are added as expected in these cases. Fixes gh-29362
1 parent 12cc8a9 commit 9410998

File tree

4 files changed

+52
-15
lines changed

4 files changed

+52
-15
lines changed

spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ public boolean checkNotModified(@Nullable String etag, long lastModifiedTimestam
220220
if (this.notModified && response != null) {
221221
response.setStatus(HttpStatus.PRECONDITION_FAILED.value());
222222
}
223+
if (SAFE_METHODS.contains(getRequest().getMethod())) {
224+
if (StringUtils.hasLength(etag) && response.getHeader(HttpHeaders.ETAG) == null) {
225+
response.setHeader(HttpHeaders.ETAG, padEtagIfNecessary(etag));
226+
}
227+
response.setDateHeader(HttpHeaders.LAST_MODIFIED, lastModifiedTimestamp);
228+
}
223229
return this.notModified;
224230
}
225231

spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
* Default implementation of {@link ServerWebExchange}.
5757
*
5858
* @author Rossen Stoyanchev
59+
* @author Brian Clozel
5960
* @since 5.0
6061
*/
6162
public class DefaultServerWebExchange implements ServerWebExchange {
@@ -261,6 +262,12 @@ public boolean checkNotModified(@Nullable String etag, Instant lastModified) {
261262
if (this.notModified) {
262263
getResponse().setStatusCode(HttpStatus.PRECONDITION_FAILED);
263264
}
265+
if (SAFE_METHODS.contains(getRequest().getMethod())) {
266+
if (StringUtils.hasLength(etag) && getResponseHeaders().getETag() == null) {
267+
getResponseHeaders().setETag(padEtagIfNecessary(etag));
268+
}
269+
getResponseHeaders().setLastModified(lastModified.toEpochMilli());
270+
}
264271
return this.notModified;
265272
}
266273

spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.time.ZonedDateTime;
2424
import java.util.Date;
2525

26+
import org.junit.jupiter.api.Test;
2627
import org.junit.jupiter.params.ParameterizedTest;
2728
import org.junit.jupiter.params.provider.ValueSource;
2829

@@ -286,9 +287,9 @@ void checkNotModifiedMultipleETags(String method) {
286287
assertThat(servletResponse.getHeader("ETag")).isEqualTo(etag);
287288
}
288289

289-
@ParameterizedHttpMethodTest
290-
void checkNotModifiedTimestampWithLengthPart(String method) {
291-
setUpRequest(method);
290+
@Test
291+
void checkNotModifiedTimestampWithLengthPart() {
292+
setUpRequest("GET");
292293

293294
long epochTime = ZonedDateTime.parse(CURRENT_TIME, RFC_1123_DATE_TIME).toInstant().toEpochMilli();
294295
servletRequest.setMethod("GET");
@@ -299,47 +300,57 @@ void checkNotModifiedTimestampWithLengthPart(String method) {
299300
assertThat(servletResponse.getDateHeader("Last-Modified") / 1000).isEqualTo(epochTime / 1000);
300301
}
301302

302-
@ParameterizedHttpMethodTest
303-
void checkModifiedTimestampWithLengthPart(String method) {
304-
setUpRequest(method);
303+
@Test
304+
void checkModifiedTimestampWithLengthPart() {
305+
setUpRequest("GET");
305306

306307
long epochTime = ZonedDateTime.parse(CURRENT_TIME, RFC_1123_DATE_TIME).toInstant().toEpochMilli();
307-
servletRequest.setMethod("GET");
308308
servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774");
309309

310310
assertThat(request.checkNotModified(epochTime)).isFalse();
311311
assertThat(servletResponse.getStatus()).isEqualTo(200);
312312
assertThat(servletResponse.getDateHeader("Last-Modified") / 1000).isEqualTo(epochTime / 1000);
313313
}
314314

315-
@ParameterizedHttpMethodTest
316-
void checkNotModifiedTimestampConditionalPut(String method) {
317-
setUpRequest(method);
315+
@Test
316+
void checkNotModifiedTimestampConditionalPut() {
317+
setUpRequest("PUT");
318318

319319
long currentEpoch = currentDate.getTime();
320320
long oneMinuteAgo = currentEpoch - (1000 * 60);
321-
servletRequest.setMethod("PUT");
322321
servletRequest.addHeader("If-UnModified-Since", currentEpoch);
323322

324323
assertThat(request.checkNotModified(oneMinuteAgo)).isFalse();
325324
assertThat(servletResponse.getStatus()).isEqualTo(200);
326325
assertThat(servletResponse.getHeader("Last-Modified")).isNull();
327326
}
328327

329-
@ParameterizedHttpMethodTest
330-
void checkNotModifiedTimestampConditionalPutConflict(String method) {
331-
setUpRequest(method);
328+
@Test
329+
void checkNotModifiedTimestampConditionalPutConflict() {
330+
setUpRequest("PUT");
332331

333332
long currentEpoch = currentDate.getTime();
334333
long oneMinuteAgo = currentEpoch - (1000 * 60);
335-
servletRequest.setMethod("PUT");
336334
servletRequest.addHeader("If-UnModified-Since", oneMinuteAgo);
337335

338336
assertThat(request.checkNotModified(currentEpoch)).isTrue();
339337
assertThat(servletResponse.getStatus()).isEqualTo(412);
340338
assertThat(servletResponse.getHeader("Last-Modified")).isNull();
341339
}
342340

341+
@ParameterizedHttpMethodTest
342+
void checkNotModifiedConditionalGet(String method) {
343+
setUpRequest(method);
344+
long currentEpoch = currentDate.getTime();
345+
long oneMinuteAgo = currentEpoch - (1000 * 60);
346+
servletRequest.addHeader("If-UnModified-Since", currentEpoch);
347+
348+
assertThat(request.checkNotModified(oneMinuteAgo)).isFalse();
349+
assertThat(servletResponse.getStatus()).isEqualTo(200);
350+
assertThat(servletResponse.getDateHeader("Last-Modified") / 1000).isEqualTo(oneMinuteAgo / 1000);
351+
}
352+
353+
343354
private void setUpRequest(String method) {
344355
this.servletRequest.setMethod(method);
345356
this.servletRequest.setRequestURI("https://example.org");

spring-web/src/test/java/org/springframework/web/server/adapter/DefaultServerWebExchangeCheckNotModifiedTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,4 +307,17 @@ void checkNotModifiedTimestampConditionalPutConflict() throws Exception {
307307
assertThat(exchange.getResponse().getHeaders().getLastModified()).isEqualTo(-1);
308308
}
309309

310+
@Test
311+
void checkNotModifiedTimestampConditionalGet() throws Exception {
312+
String eTag = "\"Test\"";
313+
Instant oneMinuteAgo = currentDate.minusSeconds(60);
314+
MockServerHttpRequest request = MockServerHttpRequest.get("/").ifUnmodifiedSince(currentDate.toEpochMilli()).build();
315+
MockServerWebExchange exchange = MockServerWebExchange.from(request);
316+
317+
assertThat(exchange.checkNotModified(eTag, oneMinuteAgo)).isFalse();
318+
assertThat(exchange.getResponse().getStatusCode()).isNull();
319+
assertThat(exchange.getResponse().getHeaders().getLastModified()).isEqualTo(oneMinuteAgo.toEpochMilli());
320+
assertThat(exchange.getResponse().getHeaders().getETag()).isEqualTo(eTag);
321+
}
322+
310323
}

0 commit comments

Comments
 (0)