rmaucher commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1878573149
########## java/org/apache/catalina/servlets/DefaultServlet.java: ########## @@ -726,10 +727,79 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws */ protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { + String ifMatchHeader = request.getHeader("If-Match"); + String ifUnmodifiedSinceHeader = request.getHeader("If-Unmodified-Since"); + String ifNoneMatchHeader = request.getHeader("If-None-Match"); + String ifModifiedSinceHeader = request.getHeader("If-Modified-Since"); + String ifRangeHeader = request.getHeader("If-Range"); + String rangeHeader = request.getHeader("Range"); - return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && - checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); - + // RFC9110 #13.3.2 defines preconditions evaluation order + int next = 1; + while (next < 6) { Review Comment: I have the impression it works, but the whole thing looks weird. ########## java/org/apache/catalina/servlets/DefaultServlet.java: ########## @@ -2092,36 +2139,51 @@ protected boolean checkSendfile(HttpServletRequest request, HttpServletResponse protected boolean checkIfMatch(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { - String headerValue = request.getHeader("If-Match"); - if (headerValue != null) { + String resourceETag = generateETag(resource); + if (resourceETag == null) { + // if a current representation for the target resource is not present + response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); + return false; + } - boolean conditionSatisfied; + int headerCount = Collections.list(request.getHeaders("If-Match")).size(); Review Comment: This check should be simplified with !headerValues.hasMoreElements() ########## java/org/apache/catalina/servlets/DefaultServlet.java: ########## @@ -2176,15 +2240,40 @@ protected boolean checkIfModifiedSince(HttpServletRequest request, HttpServletRe protected boolean checkIfNoneMatch(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { - String headerValue = request.getHeader("If-None-Match"); - if (headerValue != null) { + String resourceETag = generateETag(resource); + int headerCount = Collections.list(request.getHeaders("If-None-Match")).size(); Review Comment: Same. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org