This is an automated email from the ASF dual-hosted git repository.
markt 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 b55723bc0f Fix BZ 69731 - correct maxParameterCount tracking.
b55723bc0f is described below
commit b55723bc0fd7e8b0c34d69c410f09c2a1de8f4fb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jul 1 19:04:18 2025 +0100
Fix BZ 69731 - correct maxParameterCount tracking.
Limit was was smaller than intended for multipart uploads with non-file
parts when the parts were processed before query string parameters
https://bz.apache.org/bugzilla/show_bug.cgi?id=69731
---
java/org/apache/catalina/connector/Request.java | 50 +++++++-
.../catalina/valves/TestParameterLimitValve.java | 134 ++++++++++++++++++++-
webapps/docs/changelog.xml | 6 +
3 files changed, 184 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index ce06e6e10d..760e728e28 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -109,6 +109,7 @@ import org.apache.tomcat.util.http.ServerCookies;
import org.apache.tomcat.util.http.fileupload.FileItem;
import org.apache.tomcat.util.http.fileupload.FileUpload;
import org.apache.tomcat.util.http.fileupload.disk.DiskFileItemFactory;
+import
org.apache.tomcat.util.http.fileupload.impl.FileCountLimitExceededException;
import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
import org.apache.tomcat.util.http.fileupload.impl.SizeException;
import org.apache.tomcat.util.http.fileupload.servlet.ServletRequestContext;
@@ -2374,6 +2375,26 @@ public class Request implements HttpServletRequest {
}
}
+ /*
+ * When the request body is multipart/form-data, both the parts and
the query string count towards
+ * maxParameterCount. If parseParts() is called before
getParameterXXX() then the parts will be parsed before
+ * the query string. Otherwise, the query string will be parsed first.
+ *
+ * maxParameterCount must be respected regardless of which is parsed
first.
+ *
+ * maxParameterCount is reset from the Connector at the start of every
request.
+ *
+ * If parts are parsed first, non-file parts will be added to the
parameter map and any files will reduce
+ * maxParameterCount by 1 so that when the query string is parsed the
difference between the size of the
+ * parameter map and maxParameterCount will be the original
maxParameterCount less the number of parts. i.e. the
+ * maxParameterCount applied to the query string will be the original
maxParameterCount less the number of
+ * parts.
+ *
+ * If the query string is parsed first, all parameters will be added
to the parameter map and, ignoring
+ * maxPartCount, the part limit will be set to the original
maxParameterCount less the size of the parameter
+ * map. i.e. the maxParameterCount applied to the parts will be the
original maxParameterCount less the number
+ * of query parameters.
+ */
Parameters parameters = coyoteRequest.getParameters();
parameters.setLimit(maxParameterCount);
@@ -2474,11 +2495,14 @@ public class Request implements HttpServletRequest {
// Not possible
}
parameters.addParameter(name, value);
+ } else {
+ // Adjust the limit to account for a file part which is
not added to the parameter map.
+ maxParameterCount--;
}
}
} catch (InvalidContentTypeException e) {
partsParseException = new ServletException(e);
- } catch (SizeException e) {
+ } catch (SizeException | FileCountLimitExceededException e) {
checkSwallowInput();
partsParseException = new InvalidParameterException(e,
HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
} catch (IOException e) {
@@ -2726,11 +2750,27 @@ public class Request implements HttpServletRequest {
}
parametersParsed = true;
+ /*
+ * When the request body is multipart/form-data, both the parts and
the query string count towards
+ * maxParameterCount. If parseParts() is called before
getParameterXXX() then the parts will be parsed before
+ * the query string. Otherwise, the query string will be parsed first.
+ *
+ * maxParameterCount must be respected regardless of which is parsed
first.
+ *
+ * maxParameterCount is reset from the Connector at the start of every
request.
+ *
+ * If parts are parsed first, non-file parts will be added to the
parameter map and any files will reduce
+ * maxParameterCount by 1 so that when the query string is parsed the
difference between the size of the
+ * parameter map and maxParameterCount will be the original
maxParameterCount less the number of parts. i.e. the
+ * maxParameterCount applied to the query string will be the original
maxParameterCount less the number of
+ * parts.
+ *
+ * If the query string is parsed first, all parameters will be added
to the parameter map and, ignoring
+ * maxPartCount, the part limit will be set to the original
maxParameterCount less the size of the parameter
+ * map. i.e. the maxParameterCount applied to the parts will be the
original maxParameterCount less the number
+ * of query parameters.
+ */
Parameters parameters = coyoteRequest.getParameters();
-
- if (parts != null && maxParameterCount > 0) {
- maxParameterCount -= parts.size();
- }
parameters.setLimit(maxParameterCount);
// getCharacterEncoding() may have been overridden to search for
diff --git a/test/org/apache/catalina/valves/TestParameterLimitValve.java
b/test/org/apache/catalina/valves/TestParameterLimitValve.java
index 178ba31fe6..2152ba39b3 100644
--- a/test/org/apache/catalina/valves/TestParameterLimitValve.java
+++ b/test/org/apache/catalina/valves/TestParameterLimitValve.java
@@ -511,6 +511,138 @@ public class TestParameterLimitValve extends
TomcatBaseTest {
}
+ @Test
+ public void testMaxParameterCountLimitExceeded01_02_00_00() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 2, 0, 0, false);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded01_00_02_00() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 0, 2, 0, false);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded01_00_00_02() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 0, 0, 2, false);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded01_01_00_00() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 1, 0, 0, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded01_00_01_00() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 0, 1, 0, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded01_00_00_01() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(1, 0, 0, 1, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded02_01_01_00() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(2, 1, 1, 0, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded02_01_0_01() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(2, 1, 0, 1, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded02_00_01_01() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(2, 0, 1, 1, true);
+ }
+
+
+ @Test
+ public void testMaxParameterCountLimitExceeded03_01_01_01() throws
Exception {
+ doTestMaxParameterCountLimitExceeded(3, 1, 1, 1, true);
+ }
+
+
+ private void doTestMaxParameterCountLimitExceeded(int maxParameterCount,
int textPartCount, int filePartCount,
+ int queryStringCount, boolean okExpected) throws Exception {
+
+ Tomcat tomcat = getTomcatInstance();
+ StandardContext ctx = (StandardContext) getProgrammaticRootContext();
+
+ ParameterLimitValve parameterLimitValve = new ParameterLimitValve();
+ ctx.getPipeline().addValve(parameterLimitValve);
+ // Only looking to test maxParameterCount
+ parameterLimitValve.setUrlPatternLimits("/upload/.*=" +
Integer.toString(maxParameterCount) + ",-1,-1");
+
+ Wrapper w = Tomcat.addServlet(ctx, "multipart", new
MultipartServlet());
+ // Use defaults for Multipart
+ w.setMultipartConfigElement(new MultipartConfigElement(""));
+ ctx.addServletMappingDecoded("/upload/*", "multipart");
+
+ tomcat.start();
+
+ // Construct a simple multi-part body
+ String boundary = "--simpleBoundary";
+
+ StringBuilder content = new StringBuilder();
+ int part = 1;
+
+ for (int i = 0; i < textPartCount; i++) {
+ content.append("--").append(boundary).append(CRLF);
+ content.append("Content-Disposition: form-data;
name=\"part").append(part).append("\"").append(CRLF);
+ content.append(CRLF);
+ content.append("part value ").append(part).append(CRLF);
+ part++;
+ }
+
+ for (int i = 0; i < filePartCount; i++) {
+ content.append("--").append(boundary).append(CRLF);
+ content.append("Content-Disposition: form-data;
name=\"part").append(part).append("\"; filename=\"part")
+ .append(part).append("\"").append(CRLF);
+ content.append("Content-Type: text/plain").append(CRLF);
+ content.append(CRLF);
+ content.append("part value ").append(part).append(CRLF);
+ part++;
+ }
+
+ content.append("--").append(boundary).append("--").append(CRLF);
+
+ StringBuilder queryString = new StringBuilder();
+ for (int i = 0; i < queryStringCount; i++) {
+ if (i > 0) {
+ queryString.append("&");
+ }
+ queryString.append("param");
+ queryString.append(part);
+ queryString.append("=value");
+ queryString.append(part);
+ part++;
+ }
+
+
+ Map<String,List<String>> reqHeaders = new HashMap<>();
+ reqHeaders.put("Content-Type", List.of("multipart/form-data;
boundary=" + boundary));
+ reqHeaders.put("Content-Length",
List.of(Integer.toString(content.length())));
+
+ int rc = postUrl(content.toString().getBytes(), "http://localhost:"; +
getPort() + "/upload/endpoint?" +
+ queryString.toString(), new ByteChunk(), reqHeaders, null);
+
+ if (okExpected) {
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ } else {
+ Assert.assertTrue(Integer.toString(rc),
+ rc == HttpServletResponse.SC_BAD_REQUEST || rc ==
HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
+ }
+ }
+
private static class MultipartServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -520,8 +652,8 @@ public class TestParameterLimitValve extends TomcatBaseTest
{
resp.setContentType("text/plain");
resp.setCharacterEncoding(StandardCharsets.UTF_8);
PrintWriter pw = resp.getWriter();
- pw.println("Parameters: " + req.getParameterMap().size());
pw.println("Parts: " + req.getParts().size());
+ pw.println("Parameters: " + req.getParameterMap().size());
}
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4dae024faf..ce4d131cb4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -181,6 +181,12 @@
below the web application root with a path that was terminated with a
file separator. (remm/markt)
</fix>
+ <fix>
+ <bug>69731</bug>: Fix an issue that meant that the value of
+ <code>maxParameterCount</code> applied was smaller than intended for
+ multipart uploads with non-file parts when the parts were processed
+ before query string parameters. (markt)
+ </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
