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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
