Skip to content

Commit

Permalink
Fix #8897 Ignore conditional headers as per RFC7232 (#8899)
Browse files Browse the repository at this point in the history
* Ignore date based headers if etag ones are present.
* Also avoid parsing dates unless necessary.
* Check a resource has a lastModified date

Signed-off-by: Greg Wilkins <[email protected]>
  • Loading branch information
gregw authored Nov 17, 2022
1 parent 3569f65 commit 55e9f73
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
String ifm = null;
String ifnm = null;
String ifms = null;
long ifums = -1;
String ifums = null;

if (request instanceof Request)
{
Expand All @@ -518,7 +518,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
ifms = field.getValue();
break;
case IF_UNMODIFIED_SINCE:
ifums = DateParser.parseDate(field.getValue());
ifums = field.getValue();
break;
default:
}
Expand All @@ -530,7 +530,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
ifm = request.getHeader(HttpHeader.IF_MATCH.asString());
ifnm = request.getHeader(HttpHeader.IF_NONE_MATCH.asString());
ifms = request.getHeader(HttpHeader.IF_MODIFIED_SINCE.asString());
ifums = request.getDateHeader(HttpHeader.IF_UNMODIFIED_SINCE.asString());
ifums = request.getHeader(HttpHeader.IF_UNMODIFIED_SINCE.asString());
}

if (_etags)
Expand Down Expand Up @@ -585,7 +585,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
}

// Handle if modified since
if (ifms != null)
if (ifms != null && ifnm == null)
{
//Get jetty's Response impl
String mdlm = content.getLastModifiedValue();
Expand All @@ -595,19 +595,31 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
return false;
}

long ifmsl = request.getDateHeader(HttpHeader.IF_MODIFIED_SINCE.asString());
if (ifmsl != -1 && content.getResource().lastModified() / 1000 <= ifmsl / 1000)
long ifmsl = DateParser.parseDate(ifms);
if (ifmsl != -1)
{
sendStatus(response, HttpServletResponse.SC_NOT_MODIFIED, content::getETagValue);
return false;
long lm = content.getResource().lastModified();
if (lm != -1 && lm / 1000 <= ifmsl / 1000)
{
sendStatus(response, HttpServletResponse.SC_NOT_MODIFIED, content::getETagValue);
return false;
}
}
}

// Parse the if[un]modified dates and compare to resource
if (ifums != -1 && content.getResource().lastModified() / 1000 > ifums / 1000)
if (ifums != null && ifm == null)
{
response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED);
return false;
long ifumsl = DateParser.parseDate(ifums);
if (ifumsl != -1)
{
long lm = content.getResource().lastModified();
if (lm != -1 && lm / 1000 > ifumsl / 1000)
{
response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED);
return false;
}
}
}
}
catch (IllegalArgumentException iae)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,25 @@ public void testIfModifiedSince() throws Exception
assertThat(response.getStatus(), equalTo(304));
}

@Test
public void testIfModifiedSinceIgnored() throws Exception
{
HttpTester.Response response = HttpTester.parseResponse(
_local.getResponse("GET /resource/simple.txt HTTP/1.0\r\n\r\n"));
assertThat(response.getStatus(), equalTo(200));
assertThat(response.get(LAST_MODIFIED), Matchers.notNullValue());
assertThat(response.getContent(), containsString("simple text"));
String lastModified = response.get(LAST_MODIFIED);

response = HttpTester.parseResponse(_local.getResponse(
"GET /resource/simple.txt HTTP/1.0\r\n" +
"If-Modified-Since: " + lastModified + "\r\n" +
"If-None-Match: XYZ\r\n" +
"\r\n"));

assertThat(response.getStatus(), equalTo(200));
}

@Test
public void testBigFile() throws Exception
{
Expand Down

0 comments on commit 55e9f73

Please sign in to comment.