Copilot commented on code in PR #3344:
URL: https://github.com/apache/cxf/pull/3344#discussion_r3661019535


##########
core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java:
##########
@@ -73,6 +73,10 @@ private void loadAll() {
      */
     public boolean hasNext(boolean shouldLoadNew) throws IOException {
         if (shouldLoadNew) {
+            if (attachments.size() > maxAttachmentCount) {
+                throw new IOException("The message contains more attachments 
than are permitted");
+            }
+
             Attachment a = deserializer.readNext();

Review Comment:
   The max-attachment-count check is off by one: when attachments.size() == 
maxAttachmentCount this path still reads and adds another attachment, allowing 
one more than configured before failing later. Check should block loading when 
the current size is already at the limit.
   
   This issue also appears in the following locations of the same file:
   - line 99
   - line 146



##########
core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java:
##########
@@ -954,4 +1006,30 @@ public void testCXF8706followUrlRejectsDisallowedScheme() 
{
             
System.clearProperty(AttachmentUtil.ATTACHMENT_XOP_FOLLOW_URLS_PROPERTY);
         }
     }
+
+    private void prepareAttachments() {
+        StringBuilder sb = new StringBuilder(1000);
+        sb.append("SomeHeader: foo\n")
+            .append("------=_Part_34950_1098328613.1263781527359\n")
+            .append("Content-Type: text/xml; charset=UTF-8\n")
+            .append("Content-Transfer-Encoding: binary\n")
+            .append("Content-Id: 
<318731183421.1263781527359.IBM.WEBSERVICES@auhpap02>\n")
+            .append('\n')
+            .append("<envelope/>\n");
+
+        // Add many attachments
+        IntStream.range(0, 100000).forEach(i -> {

Review Comment:
   prepareAttachments() builds a multipart message with 100000 attachments, and 
it's now called by multiple tests. This can significantly increase test runtime 
and memory usage (very large StringBuilder) without improving coverage, since 
exceeding the default max by 1 is sufficient to exercise the limit enforcement 
paths.



-- 
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