This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/11.0.x by this push:
     new 4d15612c66 Return 400 for content and range mismatch
4d15612c66 is described below

commit 4d15612c6681312124951e47f5707f55369d886c
Author: remm <r...@apache.org>
AuthorDate: Wed Mar 5 11:31:02 2025 +0100

    Return 400 for content and range mismatch
    
    Still hacked together since the existing DefaultServlet
    executePartialPut method signature is not appropriate.
    As suggested in PR#810.
---
 .../apache/catalina/servlets/DefaultServlet.java   | 21 +++++++--
 .../catalina/servlets/LocalStrings.properties      |  1 +
 .../catalina/servlets/TestDefaultServletPut.java   |  5 ++
 webapps/docs/changelog.xml                         | 53 +++-------------------
 4 files changed, 29 insertions(+), 51 deletions(-)

diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java 
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 8273b475bd..dfba149fc5 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -625,10 +625,12 @@ public class DefaultServlet extends HttpServlet {
                 resourceInputStream = req.getInputStream();
             } else {
                 tempContentFile = executePartialPut(req, range, path);
-                resourceInputStream = new FileInputStream(tempContentFile);
+                if (tempContentFile != null) {
+                    resourceInputStream = new FileInputStream(tempContentFile);
+                }
             }
 
-            if (resources.write(path, resourceInputStream, true)) {
+            if (resourceInputStream != null && resources.write(path, 
resourceInputStream, true)) {
                 if (resource.exists()) {
                     resp.setStatus(HttpServletResponse.SC_NO_CONTENT);
                 } else {
@@ -636,7 +638,8 @@ public class DefaultServlet extends HttpServlet {
                 }
             } else {
                 try {
-                    resp.sendError(HttpServletResponse.SC_CONFLICT);
+                    resp.sendError(resourceInputStream != null ?
+                            HttpServletResponse.SC_CONFLICT : 
HttpServletResponse.SC_BAD_REQUEST);
                 } catch (IllegalStateException e) {
                     // Already committed, ignore
                 }
@@ -700,12 +703,22 @@ public class DefaultServlet extends HttpServlet {
 
             // Append data in request input stream to contentFile
             randAccessContentFile.seek(range.getStart());
+            long received = 0;
             int numBytesRead;
             byte[] transferBuffer = new byte[BUFFER_SIZE];
             try (BufferedInputStream requestBufInStream = new 
BufferedInputStream(req.getInputStream(), BUFFER_SIZE)) {
                 while ((numBytesRead = 
requestBufInStream.read(transferBuffer)) != -1) {
+                    received += numBytesRead;
+                    if (received > range.getEnd() - range.getStart()) {
+                        throw new 
IllegalStateException(sm.getString("defaultServlet.wrongByteCountForRange",
+                                String.valueOf(received), 
String.valueOf(range.getEnd() - range.getStart())));
+                    }
                     randAccessContentFile.write(transferBuffer, 0, 
numBytesRead);
                 }
+                if (received < range.getEnd() - range.getStart()) {
+                    throw new 
IllegalStateException(sm.getString("defaultServlet.wrongByteCountForRange",
+                            String.valueOf(received), 
String.valueOf(range.getEnd() - range.getStart())));
+                }
             }
 
         } catch (IOException | RuntimeException | Error e) {
@@ -713,7 +726,7 @@ public class DefaultServlet extends HttpServlet {
             if (!contentFile.delete()) {
                 log(sm.getString("defaultServlet.deleteTempFileFailed", 
contentFile.getAbsolutePath()));
             }
-            throw e;
+            return null;
         }
 
         return contentFile;
diff --git a/java/org/apache/catalina/servlets/LocalStrings.properties 
b/java/org/apache/catalina/servlets/LocalStrings.properties
index b849c68d54..c0c92ea88d 100644
--- a/java/org/apache/catalina/servlets/LocalStrings.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings.properties
@@ -56,6 +56,7 @@ defaultServlet.resource.name=Name
 defaultServlet.resource.size=Size
 defaultServlet.skipfail=Read failed because only [{0}] bytes were available 
but needed to skip [{1}] bytes to reach the start of the requested range
 defaultServlet.unknownBomConfig=Unrecognised value of [{0}] provided for 
useBomIfPresent initialization parameter
+defaultServlet.wrongByteCountForRange=An invalid amount [{0}] of bytes were 
received for range with length [{1}]
 defaultServlet.xslError=XSL transformer error
 
 webdavservlet.dataSourceStore.error=Error processing [{0}] on dead properties 
for path [{1}]
diff --git a/test/org/apache/catalina/servlets/TestDefaultServletPut.java 
b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
index d19b95684d..55a04b79d6 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServletPut.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
@@ -86,6 +86,11 @@ public class TestDefaultServletPut extends TomcatBaseTest {
         // 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 });
+        // 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 });
 
         return parameterSets;
     }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6ce8b4775c..afdf326ee2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -113,6 +113,10 @@
         a 400 response. Instead will consider it as a failed match since strong
         etags are required for <code>If-Range</code>. (remm)
       </fix>
+      <fix>
+        Return 400 if the amount of content sent for a partial PUT is
+        inconsistent with the range that was specified. (remm)
+      </fix>
     </changelog>
   </subsection>
 </section>
@@ -782,54 +786,9 @@
         the StandardContext. (markt)
       </fix>
       <fix>
-        Stop after <code>INITIALIZED</code> state should be a noop since it is
-        possible for subcomponents to be in <code>FAILED</code> after init.
-        (remm)
-      </fix>
-      <fix>
-        Fix incorrect web resource cache size calculations when there are
-        concurrent <code>PUT</code> and <code>DELETE</code> requests for the
-        same resource. (markt)
-      </fix>
-      <add>
-        Add debug logging for the web resource cache so the current size can be
-        tracked as resources are added and removed. (markt)
-      </add>
-      <update>
-        Replace legacy WebDAV <code>opaquelocktoken:</code> scheme for lock
-        tokens with <code>urn:uuid:</code> as recommended by RFC 4918, and
-        remove <code>secret</code> init parameter. (remm)
-      </update>
-      <fix>
-        Concurrent reads and writes (e.g. <code>GET</code> and <code>PUT</code>
-        / <code>DELETE</code>) for the same path caused corruption of the
-        <code>FileResource</code> where some of the fields were set as if the
-        file exists and some were set as if it does not. This resulted in
-        inconsistent metadata. (markt)
-      </fix>
-      <fix>
-        <bug>69415</bug>: Ensure that the <code>ExpiresFilter</code> only sets
-        cache headers on <code>GET</code> and <code>HEAD</code> requests. Also
-        skip requests where the application has set <code>Cache-Control:
-        no-store</code>. (markt)
-      </fix>
-      <fix>
-        <bug>69419</bug>: Improve the performance of
-        <code>ServletRequest.getAttribute()</code> when there are multiple
-        levels of nested includes. Based on a patch provided by John
-        Engebretson. (markt)
-      </fix>
-      <fix>
-        <bug>69426</bug>: Restore providing a value (rather than null) for
-        <code>Class.getProtectionDomain().getCodeSource().getLocation()</code>
-        as a number of libraries and JRE features depend on this being non-null
-        even when a SecurityManager is not is use. (markt)
+        Return 400 if the amount of content sent for a partial PUT is
+        inconsistent with the range that was specified. (remm)
       </fix>
-      <add>
-        All applications to send an early hints informational response by
-        calling <code>HttpServletResponse.sendError()</code> with a status code
-        of 103. (schultz)
-      </add>
     </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