Skip to content

Commit

Permalink
Add caching headers to If-Unmodified-Since responses
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bclozel committed Oct 21, 2022
1 parent 12cc8a9 commit 9410998
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 15 deletions.
Expand Up @@ -220,6 +220,12 @@ public boolean checkNotModified(@Nullable String etag, long lastModifiedTimestam
if (this.notModified && response != null) {
response.setStatus(HttpStatus.PRECONDITION_FAILED.value());
}
if (SAFE_METHODS.contains(getRequest().getMethod())) {
if (StringUtils.hasLength(etag) && response.getHeader(HttpHeaders.ETAG) == null) {
response.setHeader(HttpHeaders.ETAG, padEtagIfNecessary(etag));
}
response.setDateHeader(HttpHeaders.LAST_MODIFIED, lastModifiedTimestamp);
}
return this.notModified;
}

Expand Down
Expand Up @@ -56,6 +56,7 @@
* Default implementation of {@link ServerWebExchange}.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 5.0
*/
public class DefaultServerWebExchange implements ServerWebExchange {
Expand Down Expand Up @@ -261,6 +262,12 @@ public boolean checkNotModified(@Nullable String etag, Instant lastModified) {
if (this.notModified) {
getResponse().setStatusCode(HttpStatus.PRECONDITION_FAILED);
}
if (SAFE_METHODS.contains(getRequest().getMethod())) {
if (StringUtils.hasLength(etag) && getResponseHeaders().getETag() == null) {
getResponseHeaders().setETag(padEtagIfNecessary(etag));
}
getResponseHeaders().setLastModified(lastModified.toEpochMilli());
}
return this.notModified;
}

Expand Down
Expand Up @@ -23,6 +23,7 @@
import java.time.ZonedDateTime;
import java.util.Date;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

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

@ParameterizedHttpMethodTest
void checkNotModifiedTimestampWithLengthPart(String method) {
setUpRequest(method);
@Test
void checkNotModifiedTimestampWithLengthPart() {
setUpRequest("GET");

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

@ParameterizedHttpMethodTest
void checkModifiedTimestampWithLengthPart(String method) {
setUpRequest(method);
@Test
void checkModifiedTimestampWithLengthPart() {
setUpRequest("GET");

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

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

@ParameterizedHttpMethodTest
void checkNotModifiedTimestampConditionalPut(String method) {
setUpRequest(method);
@Test
void checkNotModifiedTimestampConditionalPut() {
setUpRequest("PUT");

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

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

@ParameterizedHttpMethodTest
void checkNotModifiedTimestampConditionalPutConflict(String method) {
setUpRequest(method);
@Test
void checkNotModifiedTimestampConditionalPutConflict() {
setUpRequest("PUT");

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

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

@ParameterizedHttpMethodTest
void checkNotModifiedConditionalGet(String method) {
setUpRequest(method);
long currentEpoch = currentDate.getTime();
long oneMinuteAgo = currentEpoch - (1000 * 60);
servletRequest.addHeader("If-UnModified-Since", currentEpoch);

assertThat(request.checkNotModified(oneMinuteAgo)).isFalse();
assertThat(servletResponse.getStatus()).isEqualTo(200);
assertThat(servletResponse.getDateHeader("Last-Modified") / 1000).isEqualTo(oneMinuteAgo / 1000);
}


private void setUpRequest(String method) {
this.servletRequest.setMethod(method);
this.servletRequest.setRequestURI("https://example.org");
Expand Down
Expand Up @@ -307,4 +307,17 @@ void checkNotModifiedTimestampConditionalPutConflict() throws Exception {
assertThat(exchange.getResponse().getHeaders().getLastModified()).isEqualTo(-1);
}

@Test
void checkNotModifiedTimestampConditionalGet() throws Exception {
String eTag = "\"Test\"";
Instant oneMinuteAgo = currentDate.minusSeconds(60);
MockServerHttpRequest request = MockServerHttpRequest.get("/").ifUnmodifiedSince(currentDate.toEpochMilli()).build();
MockServerWebExchange exchange = MockServerWebExchange.from(request);

assertThat(exchange.checkNotModified(eTag, oneMinuteAgo)).isFalse();
assertThat(exchange.getResponse().getStatusCode()).isNull();
assertThat(exchange.getResponse().getHeaders().getLastModified()).isEqualTo(oneMinuteAgo.toEpochMilli());
assertThat(exchange.getResponse().getHeaders().getETag()).isEqualTo(eTag);
}

}

0 comments on commit 9410998

Please sign in to comment.