From 208bf2ac71aa1eb66e68575791a5ca5b46925bf0 Mon Sep 17 00:00:00 2001 From: Sagar Miglani Date: Mon, 7 Apr 2025 14:26:32 +0530 Subject: [PATCH] SLING-12497 - Replace Commons FileUpload with Servlet API --- pom.xml | 6 -- .../parameters/MultipartRequestParameter.java | 77 ++++++++++++------- .../impl/parameters/ParameterSupport.java | 73 +++++++++--------- .../impl/parameters/RequestPartsIterator.java | 65 ++++++---------- .../engine/impl/parameters/SlingPart.java | 10 +-- 5 files changed, 115 insertions(+), 116 deletions(-) diff --git a/pom.xml b/pom.xml index 555a28bf..b4c64089 100644 --- a/pom.xml +++ b/pom.xml @@ -137,12 +137,6 @@ 2.1.2 provided - - org.apache.commons - commons-fileupload2-jakarta-servlet5 - 2.0.0-M2 - provided - org.osgi org.osgi.framework diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/MultipartRequestParameter.java b/src/main/java/org/apache/sling/engine/impl/parameters/MultipartRequestParameter.java index 21779774..c10b34e6 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/MultipartRequestParameter.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/MultipartRequestParameter.java @@ -21,8 +21,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; +import java.nio.charset.Charset; -import org.apache.commons.fileupload2.core.DiskFileItem; +import jakarta.servlet.http.Part; /** * The MultipartRequestParameter represents a request parameter @@ -34,23 +35,35 @@ */ public class MultipartRequestParameter extends AbstractRequestParameter { - private final DiskFileItem delegatee; + private final Part part; private String encodedFileName; private String cachedValue; - public MultipartRequestParameter(DiskFileItem delegatee) { - super(delegatee.getFieldName(), null); - this.delegatee = delegatee; + private byte[] cachedContent; + + public static final String FORM_DATA = "form-data"; + + public static final String ATTACHMENT = "attachment"; + + private static final String FILENAME_KEY = "filename"; + + public MultipartRequestParameter(Part part) { + this(part, null); + } + + public MultipartRequestParameter(Part part, String encoding) { + super(part.getName(), encoding); + this.part = part; } void dispose() throws IOException { - this.delegatee.delete(); + this.part.delete(); } - DiskFileItem getFileItem() { - return this.delegatee; + Part getPart() { + return this.part; } @Override @@ -60,36 +73,32 @@ void setEncoding(String encoding) { } public byte[] get() { - return this.delegatee.get(); + if (cachedContent != null) { + return cachedContent; + } + // read the content of the part into a byte array + try (InputStream in = part.getInputStream()) { + cachedContent = in.readAllBytes(); + return cachedContent; + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } } public String getContentType() { - return this.delegatee.getContentType(); + return this.part.getContentType(); } public InputStream getInputStream() throws IOException { - return this.delegatee.getInputStream(); + return this.part.getInputStream(); } public String getFileName() { - if (this.encodedFileName == null && this.delegatee.getName() != null) { - String tmpFileName = this.delegatee.getName(); - if (this.getEncoding() != null) { - try { - byte[] rawName = tmpFileName.getBytes(Util.ENCODING_DIRECT); - tmpFileName = new String(rawName, this.getEncoding()); - } catch (UnsupportedEncodingException uee) { - // might log, but actually don't care - } - } - this.encodedFileName = tmpFileName; - } - - return this.encodedFileName; + return this.part.getSubmittedFileName(); } public long getSize() { - return this.delegatee.getSize(); + return this.part.getSize(); } public String getString() { @@ -117,7 +126,7 @@ public String getString() { return this.cachedValue; } - return this.delegatee.getString(); + return new String(this.get(), getCharset()); } public String getString(String enc) throws UnsupportedEncodingException { @@ -125,7 +134,7 @@ public String getString(String enc) throws UnsupportedEncodingException { } public boolean isFormField() { - return this.delegatee.isFormField(); + return getFileName() == null; } public String toString() { @@ -135,4 +144,16 @@ public String toString() { return "File: " + this.getFileName() + " (" + this.getSize() + " bytes)"; } + + private Charset getCharset() { + final String contentType = getContentType(); + if (contentType != null) { + for (String param : contentType.split(";")) { + if (param.trim().toLowerCase().startsWith("charset=")) { + return Charset.forName(param.trim().substring("charset=".length())); + } + } + } + return Charset.forName(Util.ENCODING_DIRECT); + } } diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java index 05e3daa4..411e241d 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java @@ -29,16 +29,12 @@ import java.util.Locale; import java.util.Map; +import jakarta.servlet.MultipartConfigElement; +import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; -import org.apache.commons.fileupload2.core.DiskFileItem; -import org.apache.commons.fileupload2.core.DiskFileItemFactory; -import org.apache.commons.fileupload2.core.FileUploadException; -import org.apache.commons.fileupload2.core.RequestContext; -import org.apache.commons.fileupload2.jakarta.servlet5.JakartaServletDiskFileUpload; -import org.apache.commons.fileupload2.jakarta.servlet5.JakartaServletFileUpload; -import org.apache.commons.fileupload2.jakarta.servlet5.JakartaServletRequestContext; +import jakarta.servlet.http.Part; import org.apache.sling.api.request.RequestParameter; import org.apache.sling.api.request.RequestParameterMap; import org.apache.sling.api.resource.ResourceResolver; @@ -303,8 +299,7 @@ private ParameterMap getRequestParameterMapInternal() { } // Multipart POST - if (JakartaServletFileUpload.isMultipartContent( - new JakartaServletRequestContext(this.getServletRequest()))) { + if (isMultipartContent(this.getServletRequest())) { if (isStreamed(parameters, this.getServletRequest())) { // special case, the request is Multipart and streamed processing has been requested try { @@ -314,7 +309,7 @@ private ParameterMap getRequestParameterMapInternal() { new RequestPartsIterator(this.getServletRequest())); this.log.debug( "getRequestParameterMapInternal: Iterator available as request attribute named request-parts-iterator"); - } catch (IOException e) { + } catch (IOException | ServletException e) { this.log.error( "getRequestParameterMapInternal: Error parsing multipart streamed request", e); } @@ -393,39 +388,45 @@ private static final boolean isWWWFormEncodedContent(HttpServletRequest request) return false; } + private boolean isMultipartContent(final HttpServletRequest request) { + String contentType = request.getContentType(); + if (contentType == null) { + return false; + } + return contentType.toLowerCase(Locale.ENGLISH).startsWith("multipart/form-data"); + } + private void parseMultiPartPost(ParameterMap parameters) { // Create a new file upload handler - JakartaServletDiskFileUpload upload = new JakartaServletDiskFileUpload(); - upload.setSizeMax(ParameterSupport.maxRequestSize); - upload.setFileSizeMax(ParameterSupport.maxFileSize); - upload.setFileItemFactory(DiskFileItemFactory.builder() - .setBufferSize(ParameterSupport.fileSizeThreshold) - .setPath(ParameterSupport.location.toPath()) - .get()); - upload.setFileCountMax(ParameterSupport.maxFileCount); - RequestContext rc = new JakartaServletRequestContext(this.getServletRequest()) { - @Override - public String getCharacterEncoding() { - String enc = super.getCharacterEncoding(); - return (enc != null) ? enc : Util.ENCODING_DIRECT; - } - }; + HttpServletRequest request = this.getServletRequest(); + String encoding = request.getCharacterEncoding(); + if (encoding == null) { + encoding = Util.ENCODING_DIRECT; + } + + MultipartConfigElement multipartConfig = new MultipartConfigElement( + ParameterSupport.location.toPath().toString(), + ParameterSupport.maxFileSize, + ParameterSupport.maxRequestSize, + ParameterSupport.fileSizeThreshold); + request.setAttribute("org.eclipse.jetty.multipartConfig", multipartConfig); - // Parse the request - List /* FileItem */ items = null; try { - items = upload.parseRequest(rc); - } catch (FileUploadException fue) { - this.log.error("parseMultiPartPost: Error parsing request", fue); - } + Collection parts = request.getParts(); + long fileCount = + parts.stream().filter(p -> p.getSubmittedFileName() != null).count(); - if (items != null && items.size() > 0) { - for (Iterator ii = items.iterator(); ii.hasNext(); ) { - DiskFileItem fileItem = (DiskFileItem) ii.next(); - RequestParameter pp = new MultipartRequestParameter(fileItem); - parameters.addParameter(pp, false); + if (fileCount > ParameterSupport.maxFileCount) { + throw new ServletException("Too many files uploaded. Limit is " + ParameterSupport.maxFileCount); } + + for (Part part : parts) { + parameters.addParameter(new MultipartRequestParameter(part, encoding), false); + } + + } catch (Exception e) { + this.log.error("parseMultiPartPost: Error parsing request", e); } } } diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/RequestPartsIterator.java b/src/main/java/org/apache/sling/engine/impl/parameters/RequestPartsIterator.java index 99d27e54..600c9f39 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/RequestPartsIterator.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/RequestPartsIterator.java @@ -20,17 +20,12 @@ import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; -import java.util.List; +import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.Part; -import org.apache.commons.fileupload2.core.FileItemInput; -import org.apache.commons.fileupload2.core.FileItemInputIterator; -import org.apache.commons.fileupload2.jakarta.servlet5.JakartaServletDiskFileUpload; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,35 +36,35 @@ public class RequestPartsIterator implements Iterator { private static final Logger LOG = LoggerFactory.getLogger(RequestPartsIterator.class); /** The CommonsFile Upload streaming API iterator */ - private final FileItemInputIterator itemIterator; + private final Iterator partsIterator; /** * Create and initialse the iterator using the request. The request must be fresh. Headers can have been read but the stream * must not have been parsed. * @param servletRequest the request * @throws IOException when there is a problem reading the request. - * @throws FileUploadException when there is a problem parsing the request. + * @throws ServletException when there is a problem parsing the request. */ - public RequestPartsIterator(HttpServletRequest servletRequest) throws IOException { - JakartaServletDiskFileUpload upload = new JakartaServletDiskFileUpload(); - upload.setFileCountMax(50); - itemIterator = upload.getItemIterator(servletRequest); + public RequestPartsIterator(HttpServletRequest servletRequest) throws IOException, ServletException { + Collection parts = servletRequest.getParts(); + // TODO: needed? + long fileCount = + parts.stream().filter(p -> p.getSubmittedFileName() != null).count(); + if (fileCount > 50) { + throw new ServletException("Too many files uploaded. Limit is 50."); + } + this.partsIterator = parts.iterator(); } @Override public boolean hasNext() { - try { - return itemIterator.hasNext(); - } catch (IOException e) { - LOG.error("hasNext Item failed cause:" + e.getMessage(), e); - } - return false; + return this.partsIterator.hasNext(); } @Override public Part next() { try { - return new StreamedRequestPart(itemIterator.next()); + return new StreamedRequestPart(partsIterator.next()); } catch (IOException e) { LOG.error("next Item failed cause:" + e.getMessage(), e); } @@ -85,12 +80,12 @@ public void remove() { * Internal implementation of the Part API from Servlet 3 wrapping the Commons File Upload FIleItemStream object. */ private static class StreamedRequestPart implements Part { - private final FileItemInput fileItem; + private final Part part; private final InputStream inputStream; - public StreamedRequestPart(FileItemInput fileItem) throws IOException { - this.fileItem = fileItem; - inputStream = fileItem.getInputStream(); + public StreamedRequestPart(Part part) throws IOException { + this.part = part; + inputStream = part.getInputStream(); } @Override @@ -100,12 +95,12 @@ public InputStream getInputStream() throws IOException { @Override public String getContentType() { - return fileItem.getContentType(); + return part.getContentType(); } @Override public String getName() { - return fileItem.getFieldName(); + return part.getName(); } @Override @@ -126,34 +121,22 @@ public void delete() throws IOException { @Override public String getHeader(String headerName) { - return fileItem.getHeaders().getHeader(headerName); + return this.part.getHeader(headerName); } @Override public Collection getHeaders(String headerName) { - return toCollection(fileItem.getHeaders().getHeaders(headerName)); + return this.part.getHeaders(headerName); } @Override public Collection getHeaderNames() { - return toCollection(fileItem.getHeaders().getHeaderNames()); + return this.part.getHeaderNames(); } @Override public String getSubmittedFileName() { - return fileItem.getName(); - } - - private Collection toCollection(Iterator i) { - if (i == null) { - return Collections.emptyList(); - } else { - List c = new ArrayList(); - while (i.hasNext()) { - c.add(i.next()); - } - return c; - } + return this.part.getSubmittedFileName(); } } } diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/SlingPart.java b/src/main/java/org/apache/sling/engine/impl/parameters/SlingPart.java index 59562434..71bca508 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/SlingPart.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/SlingPart.java @@ -46,7 +46,7 @@ public String getContentType() { @Override public String getName() { - return this.param.getFileItem().getFieldName(); + return this.param.getPart().getName(); } @Override @@ -62,7 +62,7 @@ public void write(String fileName) throws IOException { @Override public void delete() { try { - this.param.getFileItem().delete(); + this.param.dispose(); } catch (final IOException e) { // ignore, nothing we can do } @@ -70,14 +70,14 @@ public void delete() { @Override public String getHeader(String name) { - return this.param.getFileItem().getHeaders().getHeader(name); + return this.param.getPart().getHeader(name); } @Override public Collection getHeaders(String name) { final ArrayList headers = new ArrayList(); final Iterator itemHeaders = - this.param.getFileItem().getHeaders().getHeaders(name); + this.param.getPart().getHeaders(name).iterator(); while (itemHeaders.hasNext()) { headers.add(itemHeaders.next()); } @@ -88,7 +88,7 @@ public Collection getHeaders(String name) { public Collection getHeaderNames() { final ArrayList headers = new ArrayList(); final Iterator itemHeaders = - this.param.getFileItem().getHeaders().getHeaderNames(); + this.param.getPart().getHeaderNames().iterator(); while (itemHeaders.hasNext()) { headers.add(itemHeaders.next()); }