This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f79b5424c498d1cfde1aed6de85285b31484af06 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Jul 1 11:17:45 2019 +0100 Ignore Range headers that use unknown units as per RFC 7233. Based on a pull request by zhanhb. Also cleaned up the parseRange() return values to differentiate between "invalid header, send an error response" and "ignore the Range header" --- .../apache/catalina/servlets/DefaultServlet.java | 38 ++++++++++++++-------- .../servlets/TestDefaultServletRangeRequests.java | 11 +++++-- webapps/docs/changelog.xml | 5 +++ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 4502649..261bd58 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -909,7 +909,7 @@ public class DefaultServlet extends HttpServlet { } } - ArrayList<Range> ranges = null; + ArrayList<Range> ranges = FULL; long contentLength = -1L; if (resource.isDirectory()) { @@ -935,6 +935,9 @@ public class DefaultServlet extends HttpServlet { // Parse range specifier ranges = parseRange(request, response, resource); + if (ranges == null) { + return; + } // ETag header response.setHeader("ETag", eTag); @@ -1013,12 +1016,7 @@ public class DefaultServlet extends HttpServlet { conversionRequired = false; } - if (resource.isDirectory() || - isError || - ( (ranges == null || ranges.isEmpty()) - && request.getHeader("Range") == null ) || - ranges == FULL ) { - + if (resource.isDirectory() || isError || ranges == FULL ) { // Set the appropriate output headers if (contentType != null) { if (debug > 0) @@ -1438,7 +1436,8 @@ public class DefaultServlet extends HttpServlet { * @param request The servlet request we are processing * @param response The servlet response we are creating * @param resource The resource - * @return a list of ranges + * @return a list of ranges, {@code null} if the range header was invalid or + * {@code #FULL} if the Range header should be ignored. * @throws IOException an IO error occurred */ protected ArrayList<Range> parseRange(HttpServletRequest request, @@ -1482,26 +1481,39 @@ public class DefaultServlet extends HttpServlet { long fileLength = resource.getContentLength(); if (fileLength == 0) { - return null; + // Range header makes no sense for a zero length resource. Tomcat + // therefore opts to ignore it. + return FULL; } // Retrieving the range header (if any is specified String rangeHeader = request.getHeader("Range"); if (rangeHeader == null) { - return null; + // No Range header is the same as ignoring any Range header + return FULL; } Ranges ranges = Ranges.parseRanges(new StringReader(rangeHeader)); - // bytes is the only range unit supported (and I don't see the point - // of adding new ones). - if (ranges == null || !ranges.getUnits().equals("bytes")) { + if (ranges == null) { + // The Range header is present but not formatted correctly. + // Could argue for a 400 response but 416 is more specific. + // There is also the option to ignore the (invalid) Range header. + // RFC7233#4.4 notes that many servers do ignore the Range header in + // these circumstances but Tomcat has always returned a 416. response.addHeader("Content-Range", "bytes */" + fileLength); response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); return null; } + // bytes is the only range unit supported (and I don't see the point + // of adding new ones). + if (!ranges.getUnits().equals("bytes")) { + // RFC7233#3.1 Servers must ignore range units they don't understand + return FULL; + } + // TODO: Remove the internal representation and use Ranges // Convert to internal representation ArrayList<Range> result = new ArrayList<>(); diff --git a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java index 1f9f189..da5d477 100644 --- a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java +++ b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java @@ -66,8 +66,9 @@ public class TestDefaultServletRangeRequests extends TomcatBaseTest { parameterSets.add(new Object[] { "bytes10-", Integer.valueOf(416), "", "*/" + len }); parameterSets.add(new Object[] { "bytes-10", Integer.valueOf(416), "", "*/" + len }); // Unknown types - parameterSets.add(new Object[] { "unknown=1-2", Integer.valueOf(416), "", "*/" + len }); - parameterSets.add(new Object[] { "bytesX=1-2", Integer.valueOf(416), "", "*/" + len }); + parameterSets.add(new Object[] { "unknown=1-2", Integer.valueOf(200), strLen, "" }); + parameterSets.add(new Object[] { "bytesX=1-2", Integer.valueOf(200), strLen, "" }); + parameterSets.add(new Object[] { "Xbytes=1-2", Integer.valueOf(200), strLen, "" }); // Valid range parameterSets.add(new Object[] { "bytes=0-9", Integer.valueOf(206), "10", "0-9/" + len }); parameterSets.add(new Object[] { "bytes=-100", Integer.valueOf(206), "100", (len - 100) + "-" + (len - 1) + "/" + len }); @@ -116,7 +117,11 @@ public class TestDefaultServletRangeRequests extends TomcatBaseTest { } if (responseRangeExpected.length() > 0) { - String responseRange = responseHeaders.get("Content-Range").get(0); + String responseRange = null; + List<String> headerValues = responseHeaders.get("Content-Range"); + if (headerValues != null && headerValues.size() == 1) { + responseRange = headerValues.get(0); + } Assert.assertEquals("bytes " + responseRangeExpected, responseRange); } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 98908b6..f6c6d7b 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -50,6 +50,11 @@ <fix> Improve parsing of Range request headers. (markt) </fix> + <fix> + Range headers that specify a range unit Tomcat does not recognise should + be ignored rather than triggering a 416 response. Based on a pull + request by zhanhb. (markt) + </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