diff --git a/core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java b/core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java index 8fdbba76604..e8cfe40d4c2 100644 --- a/core/src/main/java/org/apache/cxf/attachment/LazyAttachmentCollection.java +++ b/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(); if (a != null) { attachments.add(a); @@ -87,6 +91,7 @@ public boolean hasNext() throws IOException { return hasNext(true); } public Iterator iterator() { + // CHECKSTYLE:OFF return new Iterator() { int current; boolean removed; @@ -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; @@ -125,8 +133,8 @@ public void remove() { attachments.remove(--current); removed = true; } - }; + // CHECKSTYLE:ON } public int size() { @@ -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 arg0) { + if (attachments.size() + arg0.size() > maxAttachmentCount) { + throw new RuntimeException(new IOException("The message contains more attachments than are permitted")); + } return attachments.addAll(arg0); } diff --git a/core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java b/core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java index 857704c7878..9c106da1800 100644 --- a/core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java +++ b/core/src/test/java/org/apache/cxf/attachment/AttachmentDeserializerTest.java @@ -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; @@ -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; @@ -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("\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: \n") - .append('\n') - .append("\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 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(); @@ -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); @@ -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("\n"); + + // 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: \n") + .append('\n') + .append("\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"); + } } diff --git a/systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java b/systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java index 278be3b3741..242da079836 100644 --- a/systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java +++ b/systests/databinding/src/test/java/org/apache/cxf/systest/jaxb/MTOMBase64Test.java @@ -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; @@ -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 {