This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 8f98152d57 Partial put - enhance content range verification (#778)
8f98152d57 is described below
commit 8f98152d57da91e5ab5a9cb5c47a50f25faed7c4
Author: Chenjp <ch...@msn.com>
AuthorDate: Thu Nov 28 01:36:53 2024 +0800
Partial put - enhance content range verification (#778)
* enhance content range verification
ContentRange - add isValid(), more length check.
ContentRange - parse(): make sure the not-null return of parse() is valid.
force content-range unit in lowercase internally.
TestDefaultServletPut - partial put
* isValid: remove unnecessary condition.
---------
Co-authored-by: Mark Thomas <ma...@apache.org>
---
.../apache/catalina/servlets/DefaultServlet.java | 10 ++-------
.../tomcat/util/http/parser/ContentRange.java | 26 ++++++++++++++++++++--
.../catalina/servlets/TestDefaultServletPut.java | 4 ++++
webapps/docs/changelog.xml | 5 +++++
4 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 1187514d1c..a82c7dc60d 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -1249,12 +1249,6 @@ public class DefaultServlet extends HttpServlet {
contentType.contains("/javascript");
}
- private static boolean validate(ContentRange range) {
- // bytes is the only range unit supported
- return (range != null) && ("bytes".equals(range.getUnits())) &&
(range.getStart() >= 0) &&
- (range.getEnd() >= 0) && (range.getStart() <= range.getEnd())
&& (range.getLength() > 0);
- }
-
private static boolean validate(Ranges ranges, long length) {
List<long[]> rangeContext = new ArrayList<>();
for (Ranges.Entry range : ranges.getEntries()) {
@@ -1435,8 +1429,8 @@ public class DefaultServlet extends HttpServlet {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
-
- if (!validate(contentRange)) {
+ // bytes is the only range unit supported
+ if (!"bytes".equals(contentRange.getUnits())) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
diff --git a/java/org/apache/tomcat/util/http/parser/ContentRange.java
b/java/org/apache/tomcat/util/http/parser/ContentRange.java
index d77a0e4c41..62a67c4221 100644
--- a/java/org/apache/tomcat/util/http/parser/ContentRange.java
+++ b/java/org/apache/tomcat/util/http/parser/ContentRange.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.http.parser;
import java.io.IOException;
import java.io.StringReader;
+import java.util.Locale;
public class ContentRange {
@@ -28,13 +29,21 @@ public class ContentRange {
public ContentRange(String units, long start, long end, long length) {
- this.units = units;
+ // Units are lower case (RFC 9110, section 14.1)
+ if (units == null) {
+ this.units = null;
+ } else {
+ this.units = units.toLowerCase(Locale.ENGLISH);
+ }
this.start = start;
this.end = end;
this.length = length;
}
+ /**
+ * @return rangeUnits in lower case.
+ */
public String getUnits() {
return units;
}
@@ -103,6 +112,19 @@ public class ContentRange {
return null;
}
- return new ContentRange(units, start, end, length);
+ ContentRange contentRange = new ContentRange(units, start, end,
length);
+ if (!contentRange.isValid()) {
+ // Invalid content range
+ return null;
+ }
+ return contentRange;
+ }
+
+
+ /**
+ * @return <code>true</code> if the content range is valid, per RFC 9110
section 14.4
+ */
+ public boolean isValid() {
+ return start >= 0 && end >= start && length > end;
}
}
diff --git a/test/org/apache/catalina/servlets/TestDefaultServletPut.java
b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
index 74262979c3..d19b95684d 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServletPut.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
@@ -53,6 +53,8 @@ public class TestDefaultServletPut extends TomcatBaseTest {
// Valid partial PUT
parameterSets.add(new Object[] {
"Content-Range: bytes 0-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.TRUE, END_TEXT, Boolean.TRUE });
+ parameterSets.add(new Object[] {
+ "Content-Range: ByTeS 0-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.TRUE, END_TEXT, Boolean.TRUE });
// Full PUT
parameterSets.add(new Object[] {
"", null, PATCH_TEXT, Boolean.TRUE });
@@ -79,6 +81,8 @@ public class TestDefaultServletPut extends TomcatBaseTest {
"Content-Range: bytes 0-5/" + CRLF, Boolean.FALSE, START_TEXT,
Boolean.TRUE });
parameterSets.add(new Object[] {
"Content-Range: bytes 0-5/0x5" + CRLF, Boolean.FALSE,
START_TEXT, Boolean.TRUE });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes 0-" + PATCH_LEN + "/" + PATCH_LEN +
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
// Valid partial PUT but partial PUT is disabled
parameterSets.add(new Object[] {
"Content-Range: bytes 0-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.TRUE, START_TEXT, Boolean.FALSE });
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a3ad282a16..3f7a2b7820 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -202,6 +202,11 @@
the requested ranges overlap. Based on pull request <pr>782</pr>
provided by Chenjp. (markt)
</fix>
+ <fix>
+ The default servlet now rejects partial PUT requests than contain
+ overlapping Content-Range values. Provided by Chenjp in pull request
+ <pr>778</pr>. (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
