From 55e9f738c96fadade440b709c168fd12d81579e1 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 17 Nov 2022 12:27:21 +1100 Subject: [PATCH] Fix #8897 Ignore conditional headers as per RFC7232 (#8899) * 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 --- .../eclipse/jetty/server/ResourceService.java | 34 +++++++++++++------ .../server/handler/ResourceHandlerTest.java | 19 +++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 694209ca4b2b..0d96ce36f476 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -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) { @@ -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: } @@ -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) @@ -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(); @@ -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) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index 66423100afe0..23d868dd0b49 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -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 {