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