You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "reta (via GitHub)" <gi...@apache.org> on 2026/09/06 15:29:55 UTC
[PR] Add formParamsMaxSize to FormEncodingProvider and estimate form params count before decoding / parsing [cxf]
reta opened a new pull request, #3444:
URL: https://github.com/apache/cxf/pull/3444
Add `formParamsMaxSize` to `FormEncodingProvider` and estimate form params count before decoding / parsing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add formParamsMaxSize to FormEncodingProvider and estimate form params count before decoding / parsing [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on PR #3444:
URL: https://github.com/apache/cxf/pull/3444#issuecomment-5569209192
@reta Can you check these comments please:
High: the vulnerable @FormParam path remains unchanged.
The PR limits reads only inside FormEncodingProvider. However, ordinary @FormParam processing is handled directly by JAXRSUtils.processFormParam, which still does:
FormUtils.readBody(entityStream, enc) using the uncapped overload.
FormUtils.populateMapFromString(...).
postBody.split("&") before the parameter-count check.
Relevant unchanged paths:
JAXRSUtils.java:1201
FormUtils.java:119
FormUtils.java:137
Therefore, an endpoint using @FormParam can still receive an unbounded body and reach the original split-before-check allocation. The PR's formParamsMaxSize setter is not applied to this path.
The servlet request adapter has the same issue:
HttpServletRequestFilter.java:122
Medium: the new provider-side size limit still permits excessive memory allocation.
The changed provider path now calls the capped overload, but its default is 100 MiB. readBody materializes the body into a ByteArrayOutputStream, then creates another byte array with toByteArray(), and then creates a decoded String. A request near the configured limit can therefore require substantially more than 100 MiB of transient memory before the parser rejects it.
Also, IOUtils.copy writes the input chunk before checking whether maxSize has been exceeded:
IOUtils.java:218
This is a bounded allocation compared with the original unbounded behavior, but it is not a robust low-memory default. On a 128 MiB heap, a body approaching the 100 MiB default can still cause memory pressure or OutOfMemoryError.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add formParamsMaxSize to FormEncodingProvider and estimate form params count before decoding / parsing [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta merged PR #3444:
URL: https://github.com/apache/cxf/pull/3444
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add formParamsMaxSize to FormEncodingProvider and estimate form params count before decoding / parsing [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on PR #3444:
URL: https://github.com/apache/cxf/pull/3444#issuecomment-5571424915
> The PR limits reads only inside FormEncodingProvider. However, ordinary @FormParam processing is handled directly by JAXRSUtils.processFormParam, which still does:
Thanks @coheigea . yes, I am going through though form param handling, starting with `FormEncodingProvider` only (that was really an intent), will send pull requests for other flows (sorry, have limited time). Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add formParamsMaxSize to FormEncodingProvider and estimate form params count before decoding / parsing [cxf]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3444:
URL: https://github.com/apache/cxf/pull/3444#discussion_r3946156719
##########
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/FormUtils.java:
##########
@@ -288,6 +302,18 @@ public static void populateMapFromMultipart(MultivaluedMap<String, String> param
}
}
+ /**
+ * Estimates how many parts we should expect by checking on & separator
+ */
+ private static int estimateNumberOfParts(String postBody) {
+ int count = 0;
+ int index = -1;
+ while ((index = postBody.indexOf('&', index + 1)) >= 0) {
+ ++count;
+ }
+ return count;
+ }
Review Comment:
`estimateNumberOfParts` currently counts '&' separators (not actual parts) and therefore does not match `postBody.split("&")` semantics (off-by-one for normal inputs and mismatches for trailing separators). This makes the pre-check less effective and can lead to inconsistent behavior compared to the subsequent `parts.length` check.
Consider estimating the number of parts in a way that mirrors `String#split` (including trimming trailing empty parts).
##########
systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java:
##########
@@ -342,6 +344,30 @@ public void testPostEmptyFormAsInStream() throws Exception {
assertEquals("empty form", r.readEntity(String.class));
}
+ @Test
+ public void testTooManyFormParams() throws Exception {
+ // Exceeding 500 limit
+ String params = IntStream.range(0, 501).mapToObj(i -> "param" + i + "=" + i).collect(Collectors.joining("&"));
+ String address = "http://localhost:" + PORT + "/bookstore/form";
+ WebClient wc = WebClient.create(address);
+ wc.type(MediaType.APPLICATION_FORM_URLENCODED);
+ Response r = wc.post(new ByteArrayInputStream(params.getBytes(StandardCharsets.UTF_8)));
+ assertThat("max form params limit reached", r.getStatus(), equalTo(413));
+ }
+
+ @Test
+ public void testTooLargeFormParams() throws Exception {
+ // Exceeding 100Mb limit
+ String params = IntStream.range(0, 110)
+ .mapToObj(i -> "param" + i + "="
+ + new String(Integer.toString(i)).repeat(524800)).collect(Collectors.joining("&"));
+ String address = "http://localhost:" + PORT + "/bookstore/form";
+ WebClient wc = WebClient.create(address);
+ wc.type(MediaType.APPLICATION_FORM_URLENCODED);
+ Response r = wc.post(new ByteArrayInputStream(params.getBytes(StandardCharsets.UTF_8)));
+ assertThat("max form params limit reached", r.getStatus(), equalTo(500));
Review Comment:
`testTooLargeFormParams` builds a >100MB `String` in memory (and then copies it again into a byte[]), which can make the systest slow and memory-hungry and increase flakiness in constrained CI environments. Since the goal is just to exceed the configured max body size, you can stream a large payload without allocating it all at once on the client side.
Also, the assertion message should reflect that this is a *size* limit check (not a param-count limit).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org