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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to