Skip to content

Make sure LazyAttachmentCollection respects max attachment count in all cases - #3344

Merged
reta merged 1 commit into
apache:mainfrom
reta:fix.max.attachments
Jul 28, 2026
Merged

Make sure LazyAttachmentCollection respects max attachment count in all cases#3344
reta merged 1 commit into
apache:mainfrom
reta:fix.max.attachments

Conversation

@reta

@reta reta commented Jul 27, 2026

Copy link
Copy Markdown
Member

Make sure LazyAttachmentCollection respects max attachment count in all cases, Several gaps have been identified and reported where LazyAttachmentCollection usage may bypass the max attachment count verification,

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens enforcement of the configured maximum attachment count by adding additional checks in LazyAttachmentCollection and expanding test coverage to exercise multiple lazy-loading access paths that previously could bypass the limit.

Changes:

  • Add max-attachment-count checks to LazyAttachmentCollection for hasNext(...), iterator-based traversal, and mutation (add/addAll).
  • Expand AttachmentDeserializerTest with targeted tests that force attachment loading via size(), iterators, hasNext(), and createDataHandlerMap().
  • Update MTOMBase64Test server configuration to raise the attachment max count to avoid failing when many MTOM parts are produced.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java Raises attachment max-count configuration for the MTOM base64 systest scenario.
core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java Adds/reshapes tests to exercise multiple lazy attachment loading paths and validate max-count enforcement.
core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java Adds max attachment count checks across iterator/hasNext and collection mutation paths.
Comments suppressed due to low confidence (2)

core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java:103

  • Iterator-based lazy loading still permits reading one attachment beyond maxAttachmentCount (the check uses ">" before readNext()). Also, if the collection has already exceeded the limit, the early-return branch can return true without surfacing the limit violation. This should fail as soon as the configured maximum is reached/exceeded.
            public boolean hasNext() {
                if (attachments.size() > current) {
                    return true;
                }

core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java:150

  • add(Attachment) has the same off-by-one issue: when attachments.size() == maxAttachmentCount it still allows adding another element, resulting in max+1 attachments. Use ">=" to enforce the configured limit.
    public boolean add(Attachment arg0) {
        if (attachments.size() > maxAttachmentCount) {
            throw new RuntimeException(new IOException("The message contains more attachments than are permitted"));
        }
        return attachments.add(arg0);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@reta
reta merged commit e18116b into apache:main Jul 28, 2026
6 of 8 checks passed
reta added a commit that referenced this pull request Jul 28, 2026
reta added a commit that referenced this pull request Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants