This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new 7f0df68d29 69602: Allow weak etags in If-Range header 7f0df68d29 is described below commit 7f0df68d292bcb576a48b975f169010b911d728c Author: remm <r...@apache.org> AuthorDate: Fri Feb 28 23:40:42 2025 +0100 69602: Allow weak etags in If-Range header Adjust with the specification language, '"' in the first three chars should detect if this should be a date or an etag. Trim first. --- .../apache/catalina/servlets/DefaultServlet.java | 38 +++++++++++++--------- .../servlets/TestDefaultServletRangeRequests.java | 36 ++++++++++++++++++-- webapps/docs/changelog.xml | 5 +++ 3 files changed, 60 insertions(+), 19 deletions(-) diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 0e4a33e50c..9fb75069f1 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -2376,40 +2376,46 @@ public class DefaultServlet extends HttpServlet { // If-Range is not present return true; } - String headerValue = headerEnum.nextElement(); + String headerValue = headerEnum.nextElement().trim(); if (headerEnum.hasMoreElements()) { // Multiple If-Range headers response.sendError(HttpServletResponse.SC_BAD_REQUEST); return false; } - long headerValueTime = -1L; - try { - headerValueTime = request.getDateHeader("If-Range"); - } catch (IllegalArgumentException e) { - // Ignore - } - - if (headerValueTime == -1L) { - // Not HTTP-date so this should be a single strong etag - if (headerValue.length() < 2 || headerValue.charAt(0) != '"' || + if (headerValue.length() > 2 && (headerValue.charAt(0) == '"' || headerValue.charAt(2) == '"')) { + boolean weakETag = headerValue.startsWith("W/\""); + if ((!weakETag && headerValue.charAt(0) != '"') || headerValue.charAt(headerValue.length() - 1) != '"' || - headerValue.indexOf('"', 1) != headerValue.length() - 1) { - // Not a single, strong entity tag + headerValue.indexOf('"', weakETag ? 3 : 1) != headerValue.length() - 1) { + // Not a single entity tag response.sendError(HttpServletResponse.SC_BAD_REQUEST); return false; } // If the ETag the client gave does not match the entity // etag, then the entire entity is returned. - if (resourceETag != null && resourceETag.startsWith("\"") && resourceETag.equals(headerValue.trim())) { + if (resourceETag != null && resourceETag.equals(headerValue)) { return true; } else { return false; } } else { - // unit of HTTP date is second, ignore millisecond part. - return resourceLastModified >= headerValueTime && resourceLastModified < headerValueTime + 1000; + long headerValueTime = -1L; + try { + headerValueTime = request.getDateHeader("If-Range"); + } catch (IllegalArgumentException e) { + // Ignore + } + if (headerValueTime >= 0) { + // unit of HTTP date is second, ignore millisecond part. + return resourceLastModified >= headerValueTime && resourceLastModified < headerValueTime + 1000; + } else { + // Not a single entity tag and not a valid date either + response.sendError(HttpServletResponse.SC_BAD_REQUEST); + return false; + } } + } /** diff --git a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java index af6031a159..4fb3952531 100644 --- a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java +++ b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java @@ -16,7 +16,10 @@ */ package org.apache.catalina.servlets; +import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileInputStream; +import java.security.MessageDigest; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -30,9 +33,12 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.apache.catalina.Context; +import org.apache.catalina.Wrapper; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.catalina.util.IOTools; import org.apache.tomcat.util.buf.ByteChunk; +import org.apache.tomcat.util.buf.HexUtils; import org.apache.tomcat.util.http.FastHttpDateFormat; @RunWith(Parameterized.class) @@ -47,6 +53,14 @@ public class TestDefaultServletRangeRequests extends TomcatBaseTest { long len = index.length(); String strLen = Long.toString(len); String lastModified = FastHttpDateFormat.formatDate(index.lastModified()); + String weakETag = "W/\"" + strLen + "-" + Long.toString(index.lastModified()) + "\""; + String strongETag = "\"\""; + try (FileInputStream is = new FileInputStream(index)) { + ByteArrayOutputStream os = new ByteArrayOutputStream(); + IOTools.flow(is, os); + strongETag = "\"" + HexUtils.toHexString(MessageDigest.getInstance("SHA-1").digest(os.toByteArray())) + "\""; + } catch (Exception e) { + } List<Object[]> parameterSets = new ArrayList<>(); @@ -98,11 +112,24 @@ public class TestDefaultServletRangeRequests extends TomcatBaseTest { parameterSets.add(new Object[] { "bytes=0-9", lastModified, Integer.valueOf(206), "10", "0-9/" + len }); // Nonsense data (request rejected) + parameterSets.add(new Object[] { "bytes=0-9", "a-b-c", Integer.valueOf(400), null, "" }); + parameterSets.add(new Object[] { "bytes=0-9", "W\"123\"", Integer.valueOf(400), null, "" }); + parameterSets.add(new Object[] { "bytes=0-9", "\"123\"W", Integer.valueOf(400), null, "" }); + // More than one (request rejected) parameterSets.add(new Object[] { - "bytes=0-9", "a-b-c", Integer.valueOf(400), null, ""}); + "bytes=0-9", "\"46273648\", " + weakETag, Integer.valueOf(400), null, "" }); // Different date (return whole entity) parameterSets.add(new Object[] { - "bytes=0-9", FastHttpDateFormat.formatDate(1000), Integer.valueOf(200), strLen, ""}); + "bytes=0-9", FastHttpDateFormat.formatDate(1000), Integer.valueOf(200), strLen, "" }); + // Valid weak etag + parameterSets.add(new Object[] { + "bytes=0-9", weakETag, Integer.valueOf(206), "10", "0-9/" + len }); + // Invalid strong etag + parameterSets.add(new Object[] { + "bytes=0-9", "\"46273648\"", Integer.valueOf(200), strLen, "" }); + // Valid strong etag + parameterSets.add(new Object[] { + "bytes=0-9", strongETag, Integer.valueOf(206), "10", "0-9/" + len }); return parameterSets; } @@ -126,7 +153,10 @@ public class TestDefaultServletRangeRequests extends TomcatBaseTest { File appDir = new File("test/webapp"); Context ctxt = tomcat.addContext("", appDir.getAbsolutePath()); - Tomcat.addServlet(ctxt, "default", DefaultServlet.class.getName()); + Wrapper wrapper = Tomcat.addServlet(ctxt, "default", DefaultServlet.class.getName()); + if (ifRangeHeader != null && ifRangeHeader.startsWith("\"")) { + wrapper.addInitParameter("useStrongETags", "true"); + } ctxt.addServletMappingDecoded("/", "default"); tomcat.start(); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 7c3c7baf41..8b39743e1d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -170,6 +170,11 @@ Avoid scenarios where temporary files used for partial PUT would not be deleted. (remm) </fix> + <fix> + <bug>69602</bug>: Fix regression in releases from 12-2024 that were too + strict and rejected weak etags in the <code>If-Range</code> header. + (remm) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org