Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id B9142200D14 for ; Tue, 3 Oct 2017 15:05:45 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id B7C60160BD5; Tue, 3 Oct 2017 13:05:45 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 845F41609D2 for ; Tue, 3 Oct 2017 15:05:44 +0200 (CEST) Received: (qmail 45940 invoked by uid 500); 3 Oct 2017 13:05:43 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 45931 invoked by uid 99); 3 Oct 2017 13:05:43 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 03 Oct 2017 13:05:43 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 0532A81904; Tue, 3 Oct 2017 13:05:39 +0000 (UTC) Date: Tue, 03 Oct 2017 13:05:40 +0000 To: "commits@cxf.apache.org" Subject: [cxf] 01/02: CXF-7507 - Put a configurable limit on the size of attachment headers MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: coheigea@apache.org In-Reply-To: <150703593909.30940.15070690663769145338@gitbox.apache.org> References: <150703593909.30940.15070690663769145338@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: cxf X-Git-Refname: refs/heads/3.1.x-fixes X-Git-Reftype: branch X-Git-Rev: 896bd961cbbb6b8569700e5b70229f78f94ad9dd X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20171003130541.0532A81904@gitbox.apache.org> archived-at: Tue, 03 Oct 2017 13:05:45 -0000 This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch 3.1.x-fixes in repository https://gitbox.apache.org/repos/asf/cxf.git commit 896bd961cbbb6b8569700e5b70229f78f94ad9dd Author: Colm O hEigeartaigh AuthorDate: Tue Oct 3 11:53:04 2017 +0100 CXF-7507 - Put a configurable limit on the size of attachment headers # Conflicts: # core/src/main/java/org/apache/cxf/message/MessageUtils.java # rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/MessageContextImpl.java # rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/multipart/Attachment.java --- .../cxf/attachment/AttachmentDeserializer.java | 24 ++++++++ .../apache/cxf/attachment/ContentDisposition.java | 2 +- .../attachment/HeaderSizeExceededException.java | 40 ++++++++++++++ .../java/org/apache/cxf/message/MessageUtils.java | 24 +++++++- .../apache/cxf/jaxrs/ext/MessageContextImpl.java | 9 ++- .../apache/cxf/jaxrs/ext/multipart/Attachment.java | 8 +-- .../cxf/systest/jaxrs/JAXRSMultipartTest.java | 64 ++++++++++++++++++++++ .../apache/cxf/systest/jaxrs/MultipartServer.java | 1 + 8 files changed, 165 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/cxf/attachment/AttachmentDeserializer.java b/core/src/main/java/org/apache/cxf/attachment/AttachmentDeserializer.java index e526bb9..56dbb13 100644 --- a/core/src/main/java/org/apache/cxf/attachment/AttachmentDeserializer.java +++ b/core/src/main/java/org/apache/cxf/attachment/AttachmentDeserializer.java @@ -29,17 +29,20 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.activation.DataSource; +import org.apache.cxf.common.logging.LogUtils; import org.apache.cxf.common.util.StringUtils; import org.apache.cxf.helpers.HttpHeaderHelper; import org.apache.cxf.helpers.IOUtils; import org.apache.cxf.io.CachedOutputStream; import org.apache.cxf.message.Attachment; import org.apache.cxf.message.Message; +import org.apache.cxf.message.MessageUtils; public class AttachmentDeserializer { public static final String ATTACHMENT_PART_HEADERS = AttachmentDeserializer.class.getName() + ".headers"; @@ -49,6 +52,12 @@ public class AttachmentDeserializer { public static final String ATTACHMENT_MAX_SIZE = "attachment-max-size"; + /** + * The maximum MIME Header Length. The default is 300. + */ + public static final String ATTACHMENT_MAX_HEADER_SIZE = "attachment-max-header-size"; + public static final int DEFAULT_MAX_HEADER_SIZE = 300; + public static final int THRESHOLD = 1024 * 100; //100K (byte unit) private static final Pattern CONTENT_TYPE_BOUNDARY_PATTERN = Pattern.compile("boundary=\"?([^\";]*)"); @@ -58,6 +67,8 @@ public class AttachmentDeserializer { private static final Pattern INPUT_STREAM_BOUNDARY_PATTERN = Pattern.compile("^--(\\S*)$", Pattern.MULTILINE); + private static final Logger LOG = LogUtils.getL7dLogger(AttachmentDeserializer.class); + private boolean lazyLoading = true; private int pbAmount = 2048; @@ -79,6 +90,8 @@ public class AttachmentDeserializer { private Set loaded = new HashSet(); private List supportedTypes; + private int maxHeaderLength = DEFAULT_MAX_HEADER_SIZE; + public AttachmentDeserializer(Message message) { this(message, Collections.singletonList("multipart/related")); } @@ -86,6 +99,10 @@ public class AttachmentDeserializer { public AttachmentDeserializer(Message message, List supportedTypes) { this.message = message; this.supportedTypes = supportedTypes; + + // Get the maximum Header length from configuration + maxHeaderLength = MessageUtils.getContextualInteger(message, ATTACHMENT_MAX_HEADER_SIZE, + DEFAULT_MAX_HEADER_SIZE); } public void initializeAttachments() throws IOException { @@ -279,6 +296,7 @@ public class AttachmentDeserializer { new DelegatingInputStream(new MimeBodyPartInputStream(stream, boundary, pbAmount), this); createCount++; + return AttachmentUtil.createAttachment(partStream, headers); } @@ -327,6 +345,7 @@ public class AttachmentDeserializer { StringBuilder buffer = new StringBuilder(128); StringBuilder b = new StringBuilder(128); Map> heads = new TreeMap>(String.CASE_INSENSITIVE_ORDER); + // loop until we hit the end or a null line while (readLine(in, b)) { // lines beginning with white space get special handling @@ -373,6 +392,11 @@ public class AttachmentDeserializer { // just add to the buffer buffer.append((char)c); } + + if (buffer.length() > maxHeaderLength) { + LOG.fine("The attachment header size has exceeded the configured parameter: " + maxHeaderLength); + throw new HeaderSizeExceededException(); + } } // no characters found...this was either an eof or a null line. diff --git a/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java b/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java index 2632e57..7d2cf6b 100644 --- a/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java +++ b/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java @@ -28,7 +28,7 @@ import java.util.regex.Pattern; public class ContentDisposition { private static final String CD_HEADER_PARAMS_EXPRESSION = - "(([\\w-]+( )?\\*?=( )?\"[^\"]+\")|([\\w-]+( )?\\*?=( )?[^;]+))"; + "[\\w-]++( )?\\*?=( )?((\"[^\"]++\")|([^;]+))"; private static final Pattern CD_HEADER_PARAMS_PATTERN = Pattern.compile(CD_HEADER_PARAMS_EXPRESSION); diff --git a/core/src/main/java/org/apache/cxf/attachment/HeaderSizeExceededException.java b/core/src/main/java/org/apache/cxf/attachment/HeaderSizeExceededException.java new file mode 100644 index 0000000..69b484e --- /dev/null +++ b/core/src/main/java/org/apache/cxf/attachment/HeaderSizeExceededException.java @@ -0,0 +1,40 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cxf.attachment; + +public class HeaderSizeExceededException extends RuntimeException { + private static final long serialVersionUID = -8976580055837650080L; + + public HeaderSizeExceededException() { + super(); + } + + public HeaderSizeExceededException(String message) { + super(message); + } + + public HeaderSizeExceededException(String message, Throwable cause) { + super(message, cause); + } + + public HeaderSizeExceededException(Throwable cause) { + super(cause); + } +} diff --git a/core/src/main/java/org/apache/cxf/message/MessageUtils.java b/core/src/main/java/org/apache/cxf/message/MessageUtils.java index 5bcfe23..75459a7 100644 --- a/core/src/main/java/org/apache/cxf/message/MessageUtils.java +++ b/core/src/main/java/org/apache/cxf/message/MessageUtils.java @@ -19,8 +19,11 @@ package org.apache.cxf.message; +import java.util.logging.Logger; + import org.w3c.dom.Node; +import org.apache.cxf.common.logging.LogUtils; import org.apache.cxf.common.util.PropertyUtils; @@ -29,6 +32,8 @@ import org.apache.cxf.common.util.PropertyUtils; */ public final class MessageUtils { + private static final Logger LOG = LogUtils.getL7dLogger(MessageUtils.class); + /** * Prevents instantiation. */ @@ -139,7 +144,24 @@ public final class MessageUtils { } return defaultValue; } - + + public static int getContextualInteger(Message m, String key, int defaultValue) { + if (m != null) { + Object o = m.getContextualProperty(key); + if (o instanceof String) { + try { + int i = Integer.parseInt((String)o); + if (i > 0) { + return i; + } + } catch (NumberFormatException ex) { + LOG.warning("Incorrect integer value of " + o + " specified for: " + key); + } + } + } + return defaultValue; + } + public static Object getContextualProperty(Message m, String propPreferred, String propDefault) { Object prop = null; if (m != null) { diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/MessageContextImpl.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/MessageContextImpl.java index a1ec581..05f914d 100644 --- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/MessageContextImpl.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/MessageContextImpl.java @@ -45,6 +45,7 @@ import javax.ws.rs.ext.Providers; import org.apache.cxf.attachment.AttachmentDeserializer; import org.apache.cxf.attachment.AttachmentImpl; import org.apache.cxf.attachment.AttachmentUtil; +import org.apache.cxf.attachment.HeaderSizeExceededException; import org.apache.cxf.endpoint.Endpoint; import org.apache.cxf.helpers.CastUtils; import org.apache.cxf.interceptor.AttachmentOutInterceptor; @@ -64,6 +65,7 @@ import org.apache.cxf.message.MessageImpl; import org.apache.cxf.message.MessageUtils; public class MessageContextImpl implements MessageContext { + private Message m; public MessageContextImpl(Message m) { this.m = m; @@ -78,6 +80,8 @@ public class MessageContextImpl implements MessageContext { } catch (CacheSizeExceededException e) { m.getExchange().put("cxf.io.cacheinput", Boolean.FALSE); throw new WebApplicationException(e, 413); + } catch (HeaderSizeExceededException e) { + throw new WebApplicationException(e, 413); } } if (keyValue.equals("WRITE-" + Message.ATTACHMENTS)) { @@ -269,7 +273,9 @@ public class MessageContextImpl implements MessageContext { m.getExchange().getInMessage().get(AttachmentDeserializer.ATTACHMENT_MEMORY_THRESHOLD)); inMessage.put(AttachmentDeserializer.ATTACHMENT_MAX_SIZE, m.getExchange().getInMessage().get(AttachmentDeserializer.ATTACHMENT_MAX_SIZE)); - inMessage.setContent(InputStream.class, + inMessage.put(AttachmentDeserializer.ATTACHMENT_MAX_HEADER_SIZE, + m.getExchange().getInMessage().get(AttachmentDeserializer.ATTACHMENT_MAX_HEADER_SIZE)); + inMessage.setContent(InputStream.class, m.getExchange().getInMessage().get("org.apache.cxf.multipart.embedded.input")); inMessage.put(Message.CONTENT_TYPE, m.getExchange().getInMessage().get("org.apache.cxf.multipart.embedded.ctype").toString()); @@ -282,6 +288,7 @@ public class MessageContextImpl implements MessageContext { try { Map> headers = CastUtils.cast((Map)inMessage.get(AttachmentDeserializer.ATTACHMENT_PART_HEADERS)); + Attachment first = new Attachment(AttachmentUtil.createAttachment( inMessage.getContent(InputStream.class), headers), diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/multipart/Attachment.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/multipart/Attachment.java index 64a1729..30d4d48 100644 --- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/multipart/Attachment.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/multipart/Attachment.java @@ -93,7 +93,7 @@ public class Attachment implements Transferable { public Attachment(String mediaType, Object object) { this(UUID.randomUUID().toString(), mediaType, object); } - + public Attachment(String id, String mediaType, Object object) { this.object = object; if (id != null) { @@ -110,8 +110,8 @@ public class Attachment implements Transferable { headers.putSingle("Content-ID", id); headers.putSingle("Content-Type", "application/octet-stream"); } - - Attachment(MultivaluedMap headers, DataHandler handler, Object object) { + + public Attachment(MultivaluedMap headers, DataHandler handler, Object object) { this.headers = headers; this.handler = handler; this.object = object; @@ -128,7 +128,7 @@ public class Attachment implements Transferable { } public MediaType getContentType() { - String value = handler != null && handler.getContentType() != null ? handler.getContentType() + String value = handler != null && handler.getContentType() != null ? handler.getContentType() : headers.getFirst("Content-Type"); return value == null ? MediaType.TEXT_PLAIN_TYPE : JAXRSUtils.toMediaType(value); } diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSMultipartTest.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSMultipartTest.java index 3086011..99ae714 100644 --- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSMultipartTest.java +++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSMultipartTest.java @@ -39,6 +39,7 @@ import javax.activation.DataHandler; import javax.imageio.ImageIO; import javax.mail.util.ByteArrayDataSource; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.xml.bind.JAXBContext; @@ -53,6 +54,7 @@ import org.apache.cxf.jaxrs.client.JAXRSClientFactoryBean; import org.apache.cxf.jaxrs.client.WebClient; import org.apache.cxf.jaxrs.ext.multipart.Attachment; import org.apache.cxf.jaxrs.ext.multipart.ContentDisposition; +import org.apache.cxf.jaxrs.ext.multipart.InputStreamDataSource; import org.apache.cxf.jaxrs.ext.multipart.MultipartBody; import org.apache.cxf.jaxrs.impl.MetadataMap; import org.apache.cxf.jaxrs.provider.json.JSONProvider; @@ -937,6 +939,67 @@ public class JAXRSMultipartTest extends AbstractBusClientServerTestBase { } } + // The large Content Disposition header will be rejected here + @Test + public void testLargeHeader() throws Exception { + InputStream is1 = + getClass().getResourceAsStream("/org/apache/cxf/systest/jaxrs/resources/java.jpg"); + String address = "http://localhost:" + PORT + "/bookstore/books/image"; + WebClient client = WebClient.create(address); + client.type("multipart/mixed").accept("multipart/mixed"); + WebClient.getConfig(client).getRequestContext().put("support.type.as.multipart", + "true"); + + StringBuilder sb = new StringBuilder(); + sb.append("form-data;"); + for (int i = 0; i < 10000; i++) { + sb.append("aaaaaaaaaa"); + } + + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.putSingle("Content-ID", "root"); + headers.putSingle("Content-Type", "application/octet-stream"); + headers.putSingle("Content-Disposition", sb.toString()); + DataHandler handler = new DataHandler(new InputStreamDataSource(is1, "application/octet-stream")); + + Attachment att = new Attachment(headers, handler, null); + Response response = client.post(att); + assertEquals(response.getStatus(), 413); + + client.close(); + } + + // The Content Disposition header will be accepted here, even though it is larger than the default, + // as we have configured a larger value on the service side + @Test + public void testLargerThanDefaultHeader() throws Exception { + InputStream is1 = + getClass().getResourceAsStream("/org/apache/cxf/systest/jaxrs/resources/java.jpg"); + String address = "http://localhost:" + PORT + "/bookstore/books/image"; + WebClient client = WebClient.create(address); + client.type("multipart/mixed").accept("multipart/mixed"); + WebClient.getConfig(client).getRequestContext().put("support.type.as.multipart", + "true"); + + StringBuilder sb = new StringBuilder(); + sb.append("form-data;"); + for (int i = 0; i < 35; i++) { + sb.append("aaaaaaaaaa"); + } + + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.putSingle("Content-ID", "root"); + headers.putSingle("Content-Type", "application/octet-stream"); + headers.putSingle("Content-Disposition", sb.toString()); + DataHandler handler = new DataHandler(new InputStreamDataSource(is1, "application/octet-stream")); + + Attachment att = new Attachment(headers, handler, null); + Response response = client.post(att); + assertEquals(response.getStatus(), 200); + + client.close(); + } + private void doAddBook(String address, String resourceName, int status) throws Exception { doAddBook("multipart/related", address, resourceName, status); } @@ -1018,4 +1081,5 @@ public class JAXRSMultipartTest extends AbstractBusClientServerTestBase { } return str; } + } diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/MultipartServer.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/MultipartServer.java index 0bf1718..0fa7b9a 100644 --- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/MultipartServer.java +++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/MultipartServer.java @@ -40,6 +40,7 @@ public class MultipartServer extends AbstractBusTestServerBase { Map props = new HashMap(); props.put(AttachmentDeserializer.ATTACHMENT_MAX_SIZE, String.valueOf(1024 * 10)); props.put(AttachmentDeserializer.ATTACHMENT_MEMORY_THRESHOLD, String.valueOf(1024 * 5)); + props.put(AttachmentDeserializer.ATTACHMENT_MAX_HEADER_SIZE, String.valueOf(400)); sf.setProperties(props); //default lifecycle is per-request, change it to singleton sf.setResourceProvider(MultipartStore.class, -- To stop receiving notification emails like this one, please contact "commits@cxf.apache.org" .