Skip to content

Commit

Permalink
Fix incorrect cache behavior in AssetServlet (#3736)
Browse files Browse the repository at this point in the history
###### Problem:

AssetServlet is returning 304 when the `If-None-Match` header matches *OR* `If-Modified-Since` header is later or equal to the asset modified time.

This is incorrect according to [MDN's documentation](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since)

###### Solution:

Based on the documentation if the request contains the `If-None-Match` header then they are compared instead of checking `If-Modified-Since`.

Fixes #3724
  • Loading branch information
twhi21 committed Mar 17, 2021
1 parent 7a5e899 commit c5a0a0c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
Expand Up @@ -302,8 +302,14 @@ protected byte[] readResource(URL requestedResourceURL) throws IOException {
}

private boolean isCachedClientSide(HttpServletRequest req, CachedAsset cachedAsset) {
return cachedAsset.getETag().equals(req.getHeader(IF_NONE_MATCH)) ||
(req.getDateHeader(IF_MODIFIED_SINCE) >= cachedAsset.getLastModifiedTime());
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since
// Indicates that with the presense of If-None-Match If-Modified-Since should be ignored.
String ifNoneMatchHeader = req.getHeader(IF_NONE_MATCH);
if (ifNoneMatchHeader != null) {
return cachedAsset.getETag().equals(ifNoneMatchHeader);
} else {
return req.getDateHeader(IF_MODIFIED_SINCE) >= cachedAsset.getLastModifiedTime();
}
}

/**
Expand Down
Expand Up @@ -150,6 +150,25 @@ void servesFilesFromRootsWithSameName() throws Exception {
.isEqualTo("HELLO THERE 2");
}

@Test
void cacheIfModifiedSinceOverwrittenByIfNoneMatch() throws Exception{
request.setHeader(HttpHeader.IF_MODIFIED_SINCE.toString(), "Sat, 05 Nov 1955 22:57:05 GMT");
request.setURI(DUMMY_SERVLET + "index.htm");
response = HttpTester.parseResponse(SERVLET_TESTER.getResponses(request.generate()));

assertThat(response.getStatus())
.isEqualTo(200);

String eTag = response.get(HttpHeader.ETAG);

// If-None-Match should override If-Modified-Since
request.setHeader(HttpHeader.IF_NONE_MATCH.toString(), eTag);
response = HttpTester.parseResponse(SERVLET_TESTER.getResponses(request.generate()));
assertThat(response.getStatus())
.isEqualTo(304);

}

@Test
void servesFilesWithA200() throws Exception {
response = HttpTester.parseResponse(SERVLET_TESTER.getResponses(request.generate()));
Expand Down

0 comments on commit c5a0a0c

Please sign in to comment.