Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
reta merged 1 commit into from
Jul 28, 2026
+118 −24
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment thread
reta marked this conversation as resolved.
if (a != null) {
attachments.add(a);
Expand All @@ -87,6 +91,7 @@ public boolean hasNext() throws IOException {
return hasNext(true);
}
public Iterator<Attachment> iterator() {
// CHECKSTYLE:OFF
return new Iterator<Attachment>() {
int current;
boolean removed;
Expand All @@ -98,6 +103,9 @@ public boolean hasNext() {

// check if there is another attachment
try {
if (attachments.size() > maxAttachmentCount) {
throw new IOException("The message contains more attachments than are permitted");
}
Attachment a = deserializer.readNext();
if (a == null) {
return false;
Expand Down Expand Up @@ -125,8 +133,8 @@ public void remove() {
attachments.remove(--current);
removed = true;
}

};
// CHECKSTYLE:ON
}

public int size() {
Expand All @@ -136,10 +144,16 @@ public int size() {
}

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);
}

public boolean addAll(Collection<? extends Attachment> arg0) {
if (attachments.size() + arg0.size() > maxAttachmentCount) {
throw new RuntimeException(new IOException("The message contains more attachments than are permitted"));
}
return attachments.addAll(arg0);
}

Expand Down
124 changes: 101 additions & 23 deletions core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand All @@ -40,6 +41,7 @@

import org.xml.sax.helpers.DefaultHandler;

import jakarta.activation.DataHandler;
import jakarta.activation.DataSource;
import jakarta.activation.URLDataSource;
import org.apache.cxf.helpers.IOUtils;
Expand Down Expand Up @@ -711,33 +713,29 @@ public void testManyAttachmentHeaders() throws Exception {
assertThrows("Failure expected on too many attachment headers", IOException.class,
() -> ad.initializeAttachments());
}


@Test
public void testManyAttachments() throws Exception {
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");
public void testManyAttachmentsDataHandlerIterator() throws Exception {
prepareAttachments();

// Add many attachments
IntStream.range(0, 100000).forEach(i -> {
sb.append("------=_Part_34950_1098328613.1263781527359\n")
.append("Content-Type: text/xml\n")
.append("Content-Transfer-Encoding: binary\n")
.append("Content-Id: <b86a5f2d-e7af-4e5e-b71a-9f6f2307cab0>\n")
.append('\n')
.append("<message>\n")
.append("------=_Part_34950_1098328613.1263781527359--\n");
});
AttachmentDeserializer ad = new AttachmentDeserializer(msg);
ad.initializeAttachments();

// Force it to load the attachments
final LazyAttachmentCollection attachments = (LazyAttachmentCollection) msg.getAttachments();
assertThrows("Failure expected on too many attachments", RuntimeException.class,
() -> {
// Exercise iterator() path
for (Map.Entry<String, DataHandler> entry : attachments.createDataHandlerMap().entrySet()) {
// Do nothing, just force loading
}
});
}

@Test
public void testManyAttachmentsLoadAll() throws Exception {
prepareAttachments();

msg = new MessageImpl();
msg.setContent(InputStream.class, new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8)));
msg.put(Message.CONTENT_TYPE, "multipart/related");
AttachmentDeserializer ad = new AttachmentDeserializer(msg);
ad.initializeAttachments();

Expand All @@ -746,6 +744,60 @@ public void testManyAttachments() throws Exception {
() -> msg.getAttachments().size());
}

@Test
public void testManyAttachmentsIterator() throws Exception {
prepareAttachments();

AttachmentDeserializer ad = new AttachmentDeserializer(msg);
ad.initializeAttachments();

// Iterate over attachments
assertThrows("Failure expected on too many attachments", RuntimeException.class,
() -> {
// Exercise iterator() path
for (Attachment attachment : msg.getAttachments()) {
// Do nothing, just force loading
}
}
);

// Iterate over attachments
final LazyAttachmentCollection attachments = (LazyAttachmentCollection) msg.getAttachments();
assertThrows("Failure expected on too many attachments", IOException.class,
() -> {
// Exercise iterator() path
while (attachments.hasNext()) {
// Do nothing, just force loading
}
}
);

assertThrows("Failure expected on too many attachments", RuntimeException.class,
() -> attachments.add(new AttachmentImpl("contentId")));

assertThrows("Failure expected on too many attachments", RuntimeException.class,
() -> attachments.addAll(List.of(new AttachmentImpl("contentId"))));
}

@Test
public void testManyAttachmentsHasNext() throws Exception {
prepareAttachments();

AttachmentDeserializer ad = new AttachmentDeserializer(msg);
ad.initializeAttachments();

// Iterate over attachments
final LazyAttachmentCollection attachments = (LazyAttachmentCollection) msg.getAttachments();
assertThrows("Failure expected on too many attachments", IOException.class,
() -> {
// Exercise iterator() path
while (attachments.hasNext()) {
// Do nothing, just force loading
}
}
);
}

@Test
public void testChangingMaxAttachmentCount() throws Exception {
StringBuilder sb = new StringBuilder(1000);
Expand Down Expand Up @@ -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 -> {
Comment thread
reta marked this conversation as resolved.
sb.append("------=_Part_34950_1098328613.1263781527359\n")
.append("Content-Type: text/xml\n")
.append("Content-Transfer-Encoding: binary\n")
.append("Content-Id: <b86a5f2d-e7af-4e5e-b71a-9f6f2307cab0>\n")
.append('\n')
.append("<message>\n")
.append("------=_Part_34950_1098328613.1263781527359--\n");
});

msg = new MessageImpl();
msg.setContent(InputStream.class, new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8)));
msg.put(Message.CONTENT_TYPE, "multipart/related");
}
}
2 changes: 2 additions & 0 deletions systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import jakarta.xml.ws.Service;
import jakarta.xml.ws.soap.MTOM;
import jakarta.xml.ws.soap.SOAPBinding;
import org.apache.cxf.attachment.AttachmentDeserializer;
import org.apache.cxf.ext.logging.Logging;
import org.apache.cxf.ext.logging.LoggingInInterceptor;
import org.apache.cxf.ext.logging.LoggingOutInterceptor;
Expand Down Expand Up @@ -111,6 +112,7 @@ public static class Server extends AbstractBusTestServerBase {
protected void run() {
EndpointImpl endpointImpl = (EndpointImpl)Endpoint.publish(ADDRESS, new MTOMServer());
endpointImpl.getProperties().put(Message.CONTENT_TRANSFER_ENCODING, "base64");
endpointImpl.getBus().getProperties().put(AttachmentDeserializer.ATTACHMENT_MAX_COUNT, "100");
}
public static void main(String[] args) {
try {
Expand Down
Loading