You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Chenjp (via GitHub)" <gi...@apache.org> on 2024/11/19 03:09:27 UTC
[PR] send 416 error to overlapping ranges request [tomcat]
Chenjp opened a new pull request, #782:
URL: https://github.com/apache/tomcat/pull/782
request ranges validation - overlap detection added.
* invalid ranges - overlapping:
```
D:\git\github.com>curl http://localhost:55464/index.html -i -H "Range: bytes=10-40,35-50"
HTTP/1.1 416
Accept-Ranges: bytes
Content-Range: bytes */957
Content-Type: text/html;charset=utf-8
Content-Language: zh-CN
Content-Length: 753
Date: Tue, 19 Nov 2024 03:03:56 GMT
```
* valid ranges:
```
D:\git\github.com>curl http://localhost:55464/index.html -i -H "Range: bytes=10-40,50-60"
HTTP/1.1 206
Accept-Ranges: bytes
ETag: W/"957-1724403887512"
Last-Modified: Fri, 23 Aug 2024 09:04:47 GMT
Content-Type: multipart/byteranges; boundary=CATALINA_MIME_BOUNDARY
Content-Length: 194
Date: Tue, 19 Nov 2024 03:04:39 GMT
--CATALINA_MIME_BOUNDARY
Content-Range: bytes 10-40/957
censed to the Apache Software F
--CATALINA_MIME_BOUNDARY
Content-Range: bytes 50-60/957
(ASF) unde
--CATALINA_MIME_BOUNDARY--
```
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on code in PR #782:
URL: https://github.com/apache/tomcat/pull/782#discussion_r1854564015
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1231,10 +1231,25 @@ private static boolean validate(ContentRange range) {
(range.getEnd() >= 0) && (range.getStart() <= range.getEnd()) && (range.getLength() > 0);
}
- private static boolean validate(Ranges.Entry range, long length) {
- long start = getStart(range, length);
- long end = getEnd(range, length);
- return (start >= 0) && (end >= 0) && (start <= end);
+ private static boolean validate(Ranges ranges, long length) {
+ List<long[]> rangeContext = new ArrayList<long[]>();
+ for (Ranges.Entry range : ranges.getEntries()) {
+ long start = getStart(range, length);
+ long end = getEnd(range, length);
+ if (start < 0 || start > end) {
+ // illegal entry
+ return false;
+ }
+ // see rfc9110 #status.416
+ for (long[] e : rangeContext) {
+ if (Long.min(end, e[1]) - Long.max(start, e[0]) >= 0) {
Review Comment:
Simplify? Maybe `Long.min(...) < Long.max(...)`?
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1231,10 +1231,25 @@ private static boolean validate(ContentRange range) {
(range.getEnd() >= 0) && (range.getStart() <= range.getEnd()) && (range.getLength() > 0);
}
- private static boolean validate(Ranges.Entry range, long length) {
- long start = getStart(range, length);
- long end = getEnd(range, length);
- return (start >= 0) && (end >= 0) && (start <= end);
+ private static boolean validate(Ranges ranges, long length) {
+ List<long[]> rangeContext = new ArrayList<long[]>();
+ for (Ranges.Entry range : ranges.getEntries()) {
+ long start = getStart(range, length);
+ long end = getEnd(range, length);
+ if (start < 0 || start > end) {
Review Comment:
I like this optimisation
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf closed pull request #782: send 416 error to overlapping ranges request
URL: https://github.com/apache/tomcat/pull/782
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "gmshake (via GitHub)" <gi...@apache.org>.
gmshake commented on PR #782:
URL: https://github.com/apache/tomcat/pull/782#issuecomment-2485163715
Nice catch!
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "Chenjp (via GitHub)" <gi...@apache.org>.
Chenjp commented on PR #782:
URL: https://github.com/apache/tomcat/pull/782#issuecomment-2495274861
> I'd like to see similar protection for `Content-Range` when used with partial `PUT`.
@markt-asf done with PR #778
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "Chenjp (via GitHub)" <gi...@apache.org>.
Chenjp commented on PR #782:
URL: https://github.com/apache/tomcat/pull/782#issuecomment-2495275038
> I'd like to see similar protection for `Content-Range` when used with partial `PUT`.
@markt-asf done with PR #778
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #782:
URL: https://github.com/apache/tomcat/pull/782#issuecomment-2504291618
Tx. Applied a variation manually.
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "Chenjp (via GitHub)" <gi...@apache.org>.
Chenjp commented on code in PR #782:
URL: https://github.com/apache/tomcat/pull/782#discussion_r1857791659
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1231,10 +1231,25 @@ private static boolean validate(ContentRange range) {
(range.getEnd() >= 0) && (range.getStart() <= range.getEnd()) && (range.getLength() > 0);
}
- private static boolean validate(Ranges.Entry range, long length) {
- long start = getStart(range, length);
- long end = getEnd(range, length);
- return (start >= 0) && (end >= 0) && (start <= end);
+ private static boolean validate(Ranges ranges, long length) {
+ List<long[]> rangeContext = new ArrayList<long[]>();
+ for (Ranges.Entry range : ranges.getEntries()) {
+ long start = getStart(range, length);
+ long end = getEnd(range, length);
+ if (start < 0 || start > end) {
+ // illegal entry
+ return false;
+ }
+ // see rfc9110 #status.416
+ for (long[] e : rangeContext) {
+ if (Long.min(end, e[1]) - Long.max(start, e[0]) >= 0) {
Review Comment:
Replace with another ***more readable way***:
```java
long start = getStart(range, length);
long end = getEnd(range, length);
if (start < 0 || start > end) {
// illegal entry
return false;
}
// see rfc9110 #status.416
// invalid if range entries is overlap (equivalent to intersection is not empty).
for (long[] r : rangeContext) {
long s2 = r[0];
long e2 = r[1];
// Given [s1,e1] and [s2,e2]: if intersection is empty, then { s1>e2 || s2>e1 }
// logically equivalent to: If not { s1>e2 || s2>e1 }, then intersection is not empty.
if (start <= e2 && s2 <= end) {
// isOverlap
return false;
}
}
```
```math
$$\begin{align}
[s_1, e_1] \cap [s_2, e_2] = \emptyset \to {(s_1 \gt e_2 \lor s_2 \gt e_1)} \\\\
\iff \lnot{(s_1 \gt e_2 \lor s_2 \gt e_1)} \to [s_1, e_1] \cap [s_2, e_2] \neq \emptyset \\\\
\iff {s_1 \le e_2 \land s_2 \le e_1} \to [s_1, e_1] \cap [s_2, e_2] \neq \emptyset \\\\
\iff s_1 \le e_2 \land s_2 \le e_1 \to {isOverlap}
\end{align}$$
```
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] send 416 error to overlapping ranges request [tomcat]
Posted by "Chenjp (via GitHub)" <gi...@apache.org>.
Chenjp commented on code in PR #782:
URL: https://github.com/apache/tomcat/pull/782#discussion_r1857791659
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1231,10 +1231,25 @@ private static boolean validate(ContentRange range) {
(range.getEnd() >= 0) && (range.getStart() <= range.getEnd()) && (range.getLength() > 0);
}
- private static boolean validate(Ranges.Entry range, long length) {
- long start = getStart(range, length);
- long end = getEnd(range, length);
- return (start >= 0) && (end >= 0) && (start <= end);
+ private static boolean validate(Ranges ranges, long length) {
+ List<long[]> rangeContext = new ArrayList<long[]>();
+ for (Ranges.Entry range : ranges.getEntries()) {
+ long start = getStart(range, length);
+ long end = getEnd(range, length);
+ if (start < 0 || start > end) {
+ // illegal entry
+ return false;
+ }
+ // see rfc9110 #status.416
+ for (long[] e : rangeContext) {
+ if (Long.min(end, e[1]) - Long.max(start, e[0]) >= 0) {
Review Comment:
Replace with another ***more readable way***:
```java
long start = getStart(range, length);
long end = getEnd(range, length);
if (start < 0 || start > end) {
// illegal entry
return false;
}
// see rfc9110 #status.416
// invalid if range entries is overlap (equivalent to intersection is not empty).
for (long[] r : rangeContext) {
long s2 = r[0];
long e2 = r[1];
// Given [s1,e1] and [s2,e2]: if intersection is empty, then { s1>e2 || s2>e1 }
// logically equivalent to: If not { s1>e2 || s2>e1 }, then intersection is not empty.
if (start <= e2 && s2 <= end) {
// isOverlap
return false;
}
}
```
```math
$$\begin{align}
[s_1, e_1] \cap [s_2, e_2] = \emptyset \to {(s_1 \gt e_2 \lor s_2 \gt e_1)} \\\\
\iff \lnot{(s_1 \gt e_2 \lor s_2 \gt e_1)} \to [s_1, e_1] \cap [s_2, e_2] \neq \emptyset \\\\
\iff {s_1 \ge e_2 \land s_2 \ge e_1} \to [s_1, e_1] \cap [s_2, e_2] \neq \emptyset \\\\
\iff s_1 \ge e_2 \land s_2 \ge e_1 \to {isOverlap}
\end{align}$$
```
--
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-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org