diff --git a/http/base/pom.xml b/http/base/pom.xml index 7a7014dcb7..53ac6be716 100644 --- a/http/base/pom.xml +++ b/http/base/pom.xml @@ -133,12 +133,6 @@ 1.1.0 provided - - commons-fileupload - commons-fileupload - 1.6.0 - provided - org.apache.felix org.apache.felix.http.wrappers diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/FileCountLimitExceededException.java b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/FileCountLimitExceededException.java new file mode 100644 index 0000000000..171a1ec092 --- /dev/null +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/FileCountLimitExceededException.java @@ -0,0 +1,55 @@ +/* + * 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.felix.http.base.internal.dispatch; + +import java.io.IOException; + +/** + * This exception is thrown if a request contains more files than the specified + * limit. + */ +public class FileCountLimitExceededException extends IOException { + + private static final long serialVersionUID = 6904179610227521789L; + + /** + * The limit that was exceeded. + */ + private final long limit; + + /** + * Creates a new instance. + * + * @param message The detail message + * @param limit The limit that was exceeded + */ + public FileCountLimitExceededException(final String message, final long limit) { + super(message); + this.limit = limit; + } + + /** + * Gets the limit that was exceeded. + * + * @return The limit that was exceeded by the request + */ + public long getLimit() { + return limit; + } +} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/MultipartConfig.java b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/MultipartConfig.java index b6c114bd3a..154e10d42d 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/MultipartConfig.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/MultipartConfig.java @@ -18,8 +18,6 @@ import java.util.Objects; -import org.apache.commons.fileupload.disk.DiskFileItemFactory; - public final class MultipartConfig { public static final MultipartConfig DEFAULT_CONFIG = new MultipartConfig(null, null, -1, -1, -1); @@ -63,7 +61,7 @@ public MultipartConfig(final Integer threshold, } else { - this.multipartThreshold = DiskFileItemFactory.DEFAULT_SIZE_THRESHOLD; + this.multipartThreshold = 10240; } this.multipartLocation = location; if ( maxFileSize > 0 || maxFileSize == -1 ) { diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestMultipartWrapper.java b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestMultipartWrapper.java index 507160d9b0..711699a368 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestMultipartWrapper.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestMultipartWrapper.java @@ -16,43 +16,21 @@ */ package org.apache.felix.http.base.internal.dispatch; -import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import org.apache.commons.fileupload.FileItem; -import org.apache.commons.fileupload.FileUpload; -import org.apache.commons.fileupload.FileUploadBase; -import org.apache.commons.fileupload.FileUploadException; -import org.apache.commons.fileupload.RequestContext; -import org.apache.commons.fileupload.disk.DiskFileItemFactory; import org.apache.felix.http.base.internal.context.ExtServletContext; import jakarta.servlet.DispatcherType; +import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.Part; final class ServletRequestMultipartWrapper extends ServletRequestWrapper { - /** - * Constant for HTTP POST method. - */ - private static final String POST_METHOD = "POST"; - - private final MultipartConfig multipartConfig; - - private Collection parts; - private Map partsParameterMap; + private long maxFileCount; public ServletRequestMultipartWrapper(final HttpServletRequest req, final ExtServletContext servletContext, @@ -61,235 +39,47 @@ public ServletRequestMultipartWrapper(final HttpServletRequest req, final boolean asyncSupported, final MultipartConfig multipartConfig) { - super(req, servletContext, requestInfo, type, asyncSupported); - - this.multipartConfig = multipartConfig; - } - - private RequestContext getMultipartContext() { - final RequestContext multipartContext; - if (!POST_METHOD.equalsIgnoreCase(this.getMethod())) { - multipartContext = null; - } else { - multipartContext = new RequestContext() { - - @Override - public InputStream getInputStream() throws IOException { - return ServletRequestMultipartWrapper.this.getInputStream(); - } + super(req, servletContext, requestInfo, type, asyncSupported); - @Override - public String getContentType() { - return ServletRequestMultipartWrapper.this.getContentType(); - } + // adapt the multipart configuration for jetty + MultipartConfigElement mce = new MultipartConfigElement( + multipartConfig.multipartLocation, + multipartConfig.multipartMaxFileSize, + multipartConfig.multipartMaxRequestSize, + multipartConfig.multipartThreshold + ); - @Override - public int getContentLength() { - return ServletRequestMultipartWrapper.this.getContentLength(); - } + // Override the multipart configuration for the current request + setAttribute("org.eclipse.jetty.multipartConfig", mce); - @Override - public String getCharacterEncoding() { - return ServletRequestMultipartWrapper.this.getCharacterEncoding(); - } - }; - } - return multipartContext; + this.maxFileCount = multipartConfig.multipartMaxFileCount; } - private Collection checkMultipart() throws IOException, ServletException { - if ( parts == null ) { - final RequestContext multipartContext = getMultipartContext(); - if ( multipartContext != null && FileUploadBase.isMultipartContent(multipartContext) ) { - if ( this.multipartConfig == null) { - throw new IllegalStateException("Multipart not enabled for servlet."); - } - - handleMultipart(multipartContext); - - } else { - throw new ServletException("Not a multipart request"); - } + /** + * Enforces the non-standard "maxFileCount" configuration + * + * @return the parts the collection that was checked + */ + private Collection checkMultipart() throws IOException, ServletException { + Collection parts = getOriginalParts(); + long filePartCount = parts.stream().filter(p -> p.getSubmittedFileName() != null).count(); + if (filePartCount > maxFileCount) { + throw new FileCountLimitExceededException("Request exceeds maximum file part count", maxFileCount); } return parts; } - private void handleMultipart(final RequestContext multipartContext) throws IOException { - // Create a new file upload handler - final FileUpload upload = new FileUpload(); - upload.setSizeMax(this.multipartConfig.multipartMaxRequestSize); - upload.setFileSizeMax(this.multipartConfig.multipartMaxFileSize); - upload.setFileItemFactory(new DiskFileItemFactory(this.multipartConfig.multipartThreshold, - new File(this.multipartConfig.multipartLocation))); - upload.setFileCountMax(this.multipartConfig.multipartMaxFileCount); - // Parse the request - List items = null; - try { - items = upload.parseRequest(multipartContext); - } catch (final FileUploadException fue) { - throw new IOException("Error parsing multipart request", fue); - } - this.parts = new ArrayList<>(); - for(final FileItem item : items) { - this.parts.add(new PartImpl(item)); - } - } - @Override - @SuppressWarnings({ "unchecked", "rawtypes" }) public Collection getParts() throws IOException, ServletException { - return (Collection)checkMultipart(); - + // enforce the non-standard "maxFileCount" condition + return checkMultipart(); } @Override public Part getPart(String name) throws IOException, ServletException { - Collection parts = this.checkMultipart(); - for(final Part p : parts) { - if ( p.getName().equals(name) ) { - return p; - } - } - return null; - } - - private Map getPartsParameterMap() { - if ( this.partsParameterMap == null ) { - try { - final Collection parts = this.checkMultipart(); - final Map params = new HashMap<>(); - for(final PartImpl p : parts) { - if (p.getFileItem().isFormField()) { - String[] current = params.get(p.getName()); - if (current == null) { - current = new String[] {p.getFileItem().getString()}; - } else { - String[] newCurrent = new String[current.length + 1]; - System.arraycopy( current, 0, newCurrent, 0, current.length ); - newCurrent[current.length] = p.getFileItem().getString(); - current = newCurrent; - } - params.put(p.getName(), current); - } - } - this.partsParameterMap = params; - } catch (final IOException | ServletException ignore) { - // ignore all exceptions and use default - } - if ( this.partsParameterMap == null ) { - // use map from container implementation as default - this.partsParameterMap = super.getParameterMap(); - } - } - return this.partsParameterMap; - } - - @Override - public String getParameter(final String name) { - final String[] values = this.getParameterValues(name); - if (values != null && values.length > 0) { - return values[0]; - } - return null; - } - - @Override - public Map getParameterMap() { - final RequestContext multipartContext = getMultipartContext(); - if ( multipartContext != null && FileUploadBase.isMultipartContent(multipartContext) && this.multipartConfig != null) { - return this.getPartsParameterMap(); - } - return super.getParameterMap(); - } - - @Override - public Enumeration getParameterNames() { - final Map params = this.getParameterMap(); - return Collections.enumeration(params.keySet()); - } - - @Override - public String[] getParameterValues(final String name) { - final Map params = this.getParameterMap(); - return params.get(name); + // enforce the non-standard "maxFileCount" condition + checkMultipart(); + return getOriginalPart(name); } - private static final class PartImpl implements Part { - - private final FileItem item; - - public PartImpl(final FileItem item) { - this.item = item; - } - - @Override - public InputStream getInputStream() throws IOException { - return item.getInputStream(); - } - - @Override - public String getContentType() { - return item.getContentType(); - } - - @Override - public String getName() { - return item.getFieldName(); - } - - @Override - public String getSubmittedFileName() { - return item.getName(); - } - - @Override - public long getSize() { - return item.getSize(); - } - - @Override - public void write(final String fileName) throws IOException { - try { - item.write(new File(fileName)); - } catch (final IOException e) { - throw e; - } catch (final Exception e) { - throw new IOException(e); - } - } - - @Override - public void delete() throws IOException { - item.delete(); - } - - @Override - public String getHeader(final String name) { - return item.getHeaders().getHeader(name); - } - - @Override - public Collection getHeaders(final String name) { - final List values = new ArrayList<>(); - final Iterator iter = item.getHeaders().getHeaders(name); - while ( iter.hasNext() ) { - values.add(iter.next()); - } - return values; - } - - @Override - public Collection getHeaderNames() { - final List names = new ArrayList<>(); - final Iterator iter = item.getHeaders().getHeaderNames(); - while ( iter.hasNext() ) { - names.add(iter.next()); - } - return names; - } - - public FileItem getFileItem() { - return this.item; - } - } } diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestWrapper.java b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestWrapper.java index 4eaa39684b..3a37edb805 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestWrapper.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletRequestWrapper.java @@ -381,6 +381,20 @@ public boolean isAsyncSupported() return this.asyncSupported; } + /** + * Subclass may call this to get the original implementation + */ + protected Collection getOriginalParts() throws IOException, ServletException { + return super.getParts(); + } + + /** + * Subclass may call this to get the original implementation + */ + protected Part getOriginalPart(String name) throws IOException, ServletException { + return super.getPart(name); + } + @Override public Collection getParts() throws IOException, ServletException { throw new ServletException("No Multipart-Support available"); diff --git a/http/itest/src/test/java/org/apache/felix/http/itest/BaseIntegrationTest.java b/http/itest/src/test/java/org/apache/felix/http/itest/BaseIntegrationTest.java index 1a242e5298..da95e6b998 100644 --- a/http/itest/src/test/java/org/apache/felix/http/itest/BaseIntegrationTest.java +++ b/http/itest/src/test/java/org/apache/felix/http/itest/BaseIntegrationTest.java @@ -163,7 +163,6 @@ public Option[] config() { mavenBundle("org.apache.sling", "org.apache.sling.commons.log", "6.0.4").classifier("all"), mavenBundle("org.apache.sling", "org.apache.sling.commons.johnzon", "1.2.16").startLevel(START_LEVEL_SYSTEM_BUNDLES), mavenBundle("commons-io", "commons-io", "2.22.0").startLevel(START_LEVEL_SYSTEM_BUNDLES), - mavenBundle("commons-fileupload", "commons-fileupload", "1.6.0").startLevel(START_LEVEL_SYSTEM_BUNDLES), mavenBundle("org.apache.felix", "org.apache.felix.configadmin").version("1.9.22").startLevel(START_LEVEL_SYSTEM_BUNDLES), mavenBundle("org.apache.felix", "org.apache.felix.http.servlet-api", System.getProperty("http.servlet.api.version")).startLevel(START_LEVEL_SYSTEM_BUNDLES), diff --git a/http/jetty/pom.xml b/http/jetty/pom.xml index 8cdfc3e927..229f589f2a 100644 --- a/http/jetty/pom.xml +++ b/http/jetty/pom.xml @@ -492,18 +492,13 @@ org.apache.felix org.apache.felix.http.base - 5.1.18 + 6.0.0-SNAPSHOT org.apache.felix org.apache.felix.http.wrappers 1.0.8 - - commons-fileupload - commons-fileupload - 1.6.0 - junit diff --git a/http/jetty/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java b/http/jetty/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java index 60f8665ac8..76fa6899a9 100644 --- a/http/jetty/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java +++ b/http/jetty/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java @@ -76,7 +76,6 @@ public Option[] configuration() throws IOException { // update pax logging for SLF4J 2 mavenBundle().groupId("org.ops4j.pax.logging").artifactId("pax-logging-api").version("2.3.0"), optionalRemoteDebug(), - mavenBundle().groupId("commons-fileupload").artifactId("commons-fileupload").version("1.6.0"), mavenBundle().groupId("commons-io").artifactId("commons-io").version("2.19.0"), mavenBundle().groupId("org.apache.felix").artifactId("org.apache.felix.http.servlet-api").version("6.1.0"), testBundle("bundle.filename"), diff --git a/http/jetty/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java b/http/jetty/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java index cadaacf425..3bb8e23ff9 100644 --- a/http/jetty/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java +++ b/http/jetty/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java @@ -57,7 +57,6 @@ protected Option[] additionalOptions() throws IOException { // Minimum additional jetty dependency bundles mavenBundle().groupId("commons-io").artifactId("commons-io").version("2.19.0"), - mavenBundle().groupId("commons-fileupload").artifactId("commons-fileupload").version("1.6.0"), mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-alpn-java-server").version(jettyVersion), mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-alpn-server").version(jettyVersion), mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-http").version(jettyVersion), diff --git a/http/jetty12/pom.xml b/http/jetty12/pom.xml index e0b89c7806..7e9a0c27d0 100644 --- a/http/jetty12/pom.xml +++ b/http/jetty12/pom.xml @@ -739,11 +739,6 @@ org.apache.felix.http.wrappers 6.1.0 - - commons-fileupload - commons-fileupload - 1.6.0 - junit diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java index 5db5425879..81dc088ff7 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/AbstractJettyTestSupport.java @@ -76,7 +76,6 @@ public Option[] configuration() throws IOException { // update pax logging for SLF4J 2 mavenBundle().groupId("org.ops4j.pax.logging").artifactId("pax-logging-api").version("2.3.0"), optionalRemoteDebug(), - mavenBundle().groupId("commons-fileupload").artifactId("commons-fileupload").version("1.6.0"), mavenBundle().groupId("commons-io").artifactId("commons-io").version("2.19.0"), mavenBundle().groupId("org.apache.felix").artifactId("org.apache.felix.http.servlet-api").version("6.1.0"), testBundle("bundle.filename"), diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java index 495f05adc9..c10379bb62 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/LightClassifierIT.java @@ -57,7 +57,6 @@ protected Option[] additionalOptions() throws IOException { // Minimum additional jetty dependency bundles mavenBundle().groupId("commons-io").artifactId("commons-io").version("2.19.0"), - mavenBundle().groupId("commons-fileupload").artifactId("commons-fileupload").version("1.6.0"), mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-alpn-java-server").version(jettyVersion), mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-alpn-server").version(jettyVersion), mavenBundle().groupId("org.eclipse.jetty.compression").artifactId("jetty-compression-common").version(jettyVersion),