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 490c5871d6 Fix off by one validation logic for partial PUT ranges
490c5871d6 is described below

commit 490c5871d62d6503c7ce3577970b4b14eb08515d
Author: remm <r...@apache.org>
AuthorDate: Mon Apr 14 13:05:03 2025 +0200

    Fix off by one validation logic for partial PUT ranges
    
    The testcase was also wrong, so this includes a fix.
    Submitted by Chenjp with PR#843
---
 .../apache/catalina/servlets/DefaultServlet.java   |  9 ++++---
 .../catalina/servlets/TestDefaultServletPut.java   | 29 +++++++++++-----------
 webapps/docs/changelog.xml                         |  4 +++
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java 
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 64954a6cf8..d2eea67d17 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -735,17 +735,18 @@ public class DefaultServlet extends HttpServlet {
             int numBytesRead;
             byte[] transferBuffer = new byte[BUFFER_SIZE];
             try (BufferedInputStream requestBufInStream = new 
BufferedInputStream(req.getInputStream(), BUFFER_SIZE)) {
+                long rangeBytes = range.getEnd() - range.getStart() + 1L;
                 while ((numBytesRead = 
requestBufInStream.read(transferBuffer)) != -1) {
                     received += numBytesRead;
-                    if (received > range.getEnd() - range.getStart()) {
+                    if (received > rangeBytes) {
                         throw new 
IllegalStateException(sm.getString("defaultServlet.wrongByteCountForRange",
-                                String.valueOf(received), 
String.valueOf(range.getEnd() - range.getStart())));
+                                String.valueOf(received), 
String.valueOf(rangeBytes)));
                     }
                     randAccessContentFile.write(transferBuffer, 0, 
numBytesRead);
                 }
-                if (received < range.getEnd() - range.getStart()) {
+                if (received < rangeBytes) {
                     throw new 
IllegalStateException(sm.getString("defaultServlet.wrongByteCountForRange",
-                            String.valueOf(received), 
String.valueOf(range.getEnd() - range.getStart())));
+                            String.valueOf(received), 
String.valueOf(rangeBytes)));
                 }
             }
 
diff --git a/test/org/apache/catalina/servlets/TestDefaultServletPut.java 
b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
index 55a04b79d6..0255819497 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServletPut.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
@@ -41,9 +41,9 @@ import org.apache.tomcat.util.buf.ByteChunk;
 public class TestDefaultServletPut extends TomcatBaseTest {
 
     private static final String START_TEXT= "Starting text";
-    private static final String START_LEN = 
Integer.toString(START_TEXT.length());
+    private static final int START_LEN = START_TEXT.length();
     private static final String PATCH_TEXT= "Ending *";
-    private static final String PATCH_LEN = 
Integer.toString(PATCH_TEXT.length());
+    private static final int PATCH_LEN = PATCH_TEXT.length();
     private static final String END_TEXT= "Ending * text";
 
     @Parameterized.Parameters(name = "{index} rangeHeader [{0}]")
@@ -52,23 +52,23 @@ 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 });
+                "Content-Range: bytes 0-" + (PATCH_LEN-1) + "/" + 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 });
+                "Content-Range: ByTeS 0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.TRUE, END_TEXT, Boolean.TRUE });
         // Full PUT
         parameterSets.add(new Object[] {
                 "", null, PATCH_TEXT, Boolean.TRUE });
         // Invalid range
         parameterSets.add(new Object[] {
-                "Content-Range: apples 0-" + PATCH_LEN + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
+                "Content-Range: apples 0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
-                "Content-Range: bytes00-" + PATCH_LEN + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
+                "Content-Range: bytes00-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
-                "Content-Range: bytes0-" + PATCH_LEN + "/" + START_LEN + CRLF, 
Boolean.FALSE, START_TEXT, Boolean.TRUE });
+                "Content-Range: bytes0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
-                "Content-Range: bytes=0-" + PATCH_LEN + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
+                "Content-Range: bytes=0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
-                "Content-Range: bytes@0-" + PATCH_LEN + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
+                "Content-Range: bytes@0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.FALSE, START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
                 "Content-Range: bytes 9-7/" + START_LEN + CRLF, Boolean.FALSE, 
START_TEXT, Boolean.TRUE });
         parameterSets.add(new Object[] {
@@ -82,16 +82,17 @@ public class TestDefaultServletPut extends TomcatBaseTest {
         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 });
+                "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 });
+                "Content-Range: bytes 0-" + (PATCH_LEN-1) + "/" + START_LEN + 
CRLF, Boolean.TRUE, START_TEXT, Boolean.FALSE });
         // Errors due to incorrect length
         parameterSets.add(new Object[] {
                 "Content-Range: bytes 0-1/" + PATCH_LEN + CRLF, Boolean.FALSE, 
START_TEXT, Boolean.TRUE });
-        parameterSets.add(new Object[] {
-                "Content-Range: bytes 0-19/20" + CRLF, Boolean.FALSE, 
START_TEXT, Boolean.TRUE });
-
+        parameterSets.add(new Object[] { "Content-Range: bytes 0-" + PATCH_LEN 
+ "/20" + CRLF, Boolean.FALSE,
+                START_TEXT, Boolean.TRUE });
+        parameterSets.add(new Object[] { "Content-Range: bytes 0-" + 
(PATCH_LEN - 2) + "/20" + CRLF, Boolean.FALSE,
+                START_TEXT, Boolean.TRUE });
         return parameterSets;
     }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4c8516b094..653ef0f363 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -175,6 +175,10 @@
         <bug>69643</bug>: Optimize directory listing for large amount of files.
         Patch submitted by  Loic de l'Eprevier. (remm)
       </fix>
+      <fix>
+        <pr>843</pr>: Fix off by one validation logic for partial PUT ranges
+        and associated test case. Submitted by Chenjp. (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

Reply via email to