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
