lukaszlenart commented on code in PR #861:
URL: https://github.com/apache/struts/pull/861#discussion_r1470711866
##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java:
##########
@@ -52,309 +51,159 @@
*
* @since 2.3.18
*/
-public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
+public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest<File> {
- static final Logger LOG =
LogManager.getLogger(JakartaStreamMultiPartRequest.class);
+ private static final Logger LOG =
LogManager.getLogger(JakartaStreamMultiPartRequest.class);
/**
- * Map between file fields and file data.
- */
- protected Map<String, List<FileInfo>> fileInfos = new HashMap<>();
-
- /**
- * Map between non-file fields and values.
- */
- protected Map<String, List<String>> parameters = new HashMap<>();
-
- /* (non-Javadoc)
- * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
+ * Processes the upload.
+ *
+ * @param request the servlet request
+ * @param saveDir location of the save dir
*/
- public void cleanUp() {
- LOG.debug("Performing File Upload temporary storage cleanup.");
- for (List<FileInfo> fileInfoList : fileInfos.values()) {
- for (FileInfo fileInfo : fileInfoList) {
- File file = fileInfo.getFile();
- LOG.debug("Deleting file '{}'.", file.getName());
- if (!file.delete()) {
- LOG.warn("There was a problem attempting to delete file
'{}'.", file.getName());
- }
+ @Override
+ protected void processUpload(HttpServletRequest request, String saveDir)
throws IOException {
+ String charset = StringUtils.isBlank(request.getCharacterEncoding())
+ ? defaultEncoding
+ : request.getCharacterEncoding();
+
+ Path location = Path.of(saveDir);
+ JakartaServletDiskFileUpload servletFileUpload =
+ prepareServletFileUpload(Charset.forName(charset), location);
+
+ LOG.debug("Using Jakarta Stream API to process request");
+ servletFileUpload.getItemIterator(request).forEachRemaining(item -> {
+ if (item.isFormField()) {
+ LOG.debug(() -> "Processing a form field: " +
sanitizeNewlines(item.getFieldName()));
+ processFileItemAsFormField(item);
+ } else {
+ LOG.debug(() -> "Processing a file: " +
sanitizeNewlines(item.getFieldName()));
+ processFileItemAsFileField(item, location);
}
- }
+ });
}
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String)
- */
- public String[] getContentType(String fieldName) {
- List<FileInfo> infos = fileInfos.get(fieldName);
- if (infos == null) {
- return null;
- }
+ protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset
charset, Path location) {
+ DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
- List<String> types = new ArrayList<>(infos.size());
- for (FileInfo fileInfo : infos) {
- types.add(fileInfo.getContentType());
- }
+ LOG.debug("Using file save directory: {}", location);
+ builder.setPath(location);
- return types.toArray(new String[0]);
- }
+ LOG.debug("Sets buffer size: {}", bufferSize);
+ builder.setBufferSize(bufferSize);
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String)
- */
- public UploadedFile[] getFile(String fieldName) {
- List<FileInfo> infos = fileInfos.get(fieldName);
- if (infos == null) {
- return null;
- }
+ LOG.debug("Using charset: {}", charset);
+ builder.setCharset(charset);
- return infos.stream().map(fileInfo ->
- StrutsUploadedFile.Builder.create(fileInfo.getFile())
- .withContentType(fileInfo.contentType)
- .withOriginalName(fileInfo.originalName)
- .build()
- ).toArray(UploadedFile[]::new);
+ DiskFileItemFactory factory = builder.get();
+ return new JakartaServletDiskFileUpload(factory);
}
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileNames(java.lang.String)
- */
- public String[] getFileNames(String fieldName) {
- List<FileInfo> infos = fileInfos.get(fieldName);
- if (infos == null) {
- return null;
- }
-
- List<String> names = new ArrayList<>(infos.size());
- for (FileInfo fileInfo : infos) {
- names.add(getCanonicalName(fileInfo.getOriginalName()));
+ private String readStream(InputStream inputStream) throws IOException {
+ ByteArrayOutputStream result = new ByteArrayOutputStream();
+ byte[] buffer = new byte[1024];
+ for (int length; (length = inputStream.read(buffer)) != -1; ) {
+ result.write(buffer, 0, length);
}
-
- return names.toArray(new String[0]);
+ return result.toString(StandardCharsets.UTF_8);
}
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames()
- */
- public Enumeration<String> getFileParameterNames() {
- return Collections.enumeration(fileInfos.keySet());
- }
-
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFilesystemName(java.lang.String)
+ /**
+ * Processes the FileItem as a normal form field.
+ *
+ * @param fileItemInput a form field item input
*/
- public String[] getFilesystemName(String fieldName) {
- List<FileInfo> infos = fileInfos.get(fieldName);
- if (infos == null) {
- return null;
- }
+ protected void processFileItemAsFormField(FileItemInput fileItemInput)
throws IOException {
+ String fieldName = fileItemInput.getFieldName();
+ String fieldValue = readStream(fileItemInput.getInputStream());
- List<String> names = new ArrayList<>(infos.size());
- for (FileInfo fileInfo : infos) {
- names.add(fileInfo.getFile().getName());
+ if (exceedsMaxStringLength(fieldName, fieldValue)) {
+ return;
}
- return names.toArray(new String[0]);
- }
-
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameter(java.lang.String)
- */
- public String getParameter(String name) {
- List<String> values = parameters.get(name);
- if (values != null && !values.isEmpty()) {
- return values.get(0);
+ List<String> values;
+ if (parameters.containsKey(fieldName)) {
+ values = parameters.get(fieldName);
+ } else {
+ values = new ArrayList<>();
+ parameters.put(fieldName, values);
}
- return null;
+ values.add(fieldValue);
}
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterNames()
+ /**
+ * @return actual size of already uploaded files
*/
- public Enumeration<String> getParameterNames() {
- return Collections.enumeration(parameters.keySet());
+ protected Long actualSizeOfUploadedFiles() {
+ return uploadedFiles.values().stream()
+ .map(files ->
files.stream().map(UploadedFile::length).reduce(0L, Long::sum))
+ .reduce(0L, Long::sum);
}
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterValues(java.lang.String)
- */
- public String[] getParameterValues(String name) {
- List<String> values = parameters.get(name);
- if (values != null && !values.isEmpty()) {
- return values.toArray(new String[0]);
- }
- return null;
- }
-
- /* (non-Javadoc)
- * @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#parse(jakarta.servlet.http.HttpServletRequest,
java.lang.String)
- */
- public void parse(HttpServletRequest request, String saveDir) throws
IOException {
- try {
- setLocale(request);
- processUpload(request, saveDir);
- } catch (Exception e) {
- LOG.debug("Error occurred during parsing of multi part request",
e);
- LocalizedMessage errorMessage = buildErrorMessage(e, new
Object[]{});
+ private boolean exceedsMaxFiles(FileItemInput fileItemInput) {
+ if (maxFiles != null && maxFiles == uploadedFiles.size()) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Cannot accept another file: {} as it will exceed
max files: {}",
+ sanitizeNewlines(fileItemInput.getName()), maxFiles);
+ }
+ LocalizedMessage errorMessage = buildErrorMessage(new
FileUploadFileCountLimitException(
+ String.format("File %s exceeds allowed maximum
number of files %s",
+ fileItemInput.getName(), maxFiles),
+ maxFiles, uploadedFiles.size()),
+ new Object[]{maxFiles, uploadedFiles.size()});
if (!errors.contains(errorMessage)) {
errors.add(errorMessage);
}
+ return true;
}
+ return false;
}
- /**
- * Processes the upload.
- *
- * @param request the servlet request
- * @param saveDir location of the save dir
- */
- protected void processUpload(HttpServletRequest request, String saveDir)
throws Exception {
-
- // Sanity check that the request is a multi-part/form-data request.
- if (JakartaServletFileUpload.isMultipartContent(request)) {
-
- // Sanity check on request size.
- boolean requestSizePermitted = isRequestSizePermitted(request);
-
- // Interface with Commons FileUpload API
- // Using the Streaming API
- JakartaServletFileUpload<DiskFileItem, DiskFileItemFactory>
servletFileUpload = new JakartaServletFileUpload<>();
- if (maxSize != null) {
- servletFileUpload.setSizeMax(maxSize);
- }
- if (maxFiles != null) {
- servletFileUpload.setFileCountMax(maxFiles);
- }
- if (maxFileSize != null) {
- servletFileUpload.setFileSizeMax(maxFileSize);
- }
- FileItemInputIterator i =
servletFileUpload.getItemIterator(request);
-
- // Iterate the file items
- while (i.hasNext()) {
- try {
- FileItemInput itemStream = i.next();
-
- // If the file item stream is a form field, delegate to the
- // field item stream handler
- if (itemStream.isFormField()) {
- processFileItemStreamAsFormField(itemStream);
- }
-
- // Delegate the file item stream for a file field to the
- // file item stream handler, but delegation is skipped
- // if the requestSizePermitted check failed based on the
- // complete content-size of the request.
- else {
-
- // prevent processing file field item if request size
not allowed.
- if (!requestSizePermitted) {
- addFileSkippedError(itemStream.getName(), request);
- LOG.debug("Skipped stream '{}', request maximum
size ({}) exceeded.", itemStream.getName(), maxSize);
- continue;
- }
-
- processFileItemStreamAsFileField(itemStream, saveDir);
- }
- } catch (IOException e) {
- LOG.warn("Error occurred during process upload", e);
- }
- }
+ private void exceedsMaxSizeOfFiles(FileItemInput fileItemInput, File file,
Long currentFilesSize) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("File: {} of size: {} exceeds allowed max size: {},
actual size of already uploaded files: {}",
+ sanitizeNewlines(fileItemInput.getName()), file.length(),
maxSizeOfFiles, currentFilesSize
+ );
}
- }
-
- /**
- * Defines whether the request allowed based on content length.
- *
- * @param request the servlet request
- * @return true if request size is permitted
- */
- protected boolean isRequestSizePermitted(HttpServletRequest request) {
- // if maxSize is specified as -1, there is no sanity check and it's
- // safe to return true for any request, delegating the failure
- // checks later in the upload process.
- if (maxSize == null || maxSize == -1 || request == null) {
- return true;
+ LocalizedMessage errorMessage = buildErrorMessage(new
FileUploadSizeException(
+ String.format("Size %s of file %s exceeds allowed max
size %s",
+ file.length(), fileItemInput.getName(),
maxSizeOfFiles),
+ maxSizeOfFiles, currentFilesSize),
+ new Object[]{maxSizeOfFiles, currentFilesSize});
+ if (!errors.contains(errorMessage)) {
+ errors.add(errorMessage);
}
- return request.getContentLength() < maxSize;
- }
-
- /**
- * @param request the servlet request
- * @return the request content length.
- */
- protected long getRequestSize(HttpServletRequest request) {
- return request != null ? request.getContentLength() : 0;
- }
-
- /**
- * Add a file skipped message notification for action messages.
- *
- * @param fileName file name
- * @param request the servlet request
- */
- protected void addFileSkippedError(String fileName, HttpServletRequest
request) {
- String exceptionMessage = "Skipped file " + fileName + "; request size
limit exceeded.";
- long allowedMaxSize = maxSize != null ? maxSize : -1;
- FileUploadSizeException exception = new
FileUploadSizeException(exceptionMessage, getRequestSize(request),
allowedMaxSize);
- LocalizedMessage message = buildErrorMessage(exception, new
Object[]{fileName, getRequestSize(request), allowedMaxSize});
- if (!errors.contains(message)) {
- errors.add(message);
+ if (!file.delete() && LOG.isWarnEnabled()) {
+ LOG.warn("Cannot delete file: {} which exceeds maximum size: {} of
all files!",
+ sanitizeNewlines(fileItemInput.getName()), maxSizeOfFiles);
}
}
/**
- * Processes the FileItemStream as a Form Field.
+ * Processes the FileItem as a file field.
*
- * @param itemStream file item stream
+ * @param fileItemInput file item representing upload file
+ * @param location location
*/
- protected void processFileItemStreamAsFormField(FileItemInput itemStream) {
- String fieldName = itemStream.getFieldName();
- try {
- List<String> values;
-
- String fieldValue = itemStream.getInputStream().toString();
- if (!parameters.containsKey(fieldName)) {
- values = new ArrayList<>();
- parameters.put(fieldName, values);
- } else {
- values = parameters.get(fieldName);
- }
- values.add(fieldValue);
- } catch (IOException e) {
- LOG.warn("Failed to handle form field '{}'.", fieldName, e);
+ protected void processFileItemAsFileField(FileItemInput fileItemInput,
Path location) throws IOException {
+ // Skip file uploads that don't have a file name - meaning that no
file was selected.
+ if (fileItemInput.getName() == null ||
fileItemInput.getName().trim().isEmpty()) {
+ LOG.debug(() -> "No file has been uploaded for the field: " +
sanitizeNewlines(fileItemInput.getFieldName()));
+ return;
}
- }
- /**
- * Processes the FileItemStream as a file field.
- *
- * @param itemStream file item stream
- * @param location location
- */
- protected void processFileItemStreamAsFileField(FileItemInput itemStream,
String location) {
- // Skip file uploads that don't have a file name - meaning that no
file was selected.
- if (itemStream.getName() == null ||
itemStream.getName().trim().isEmpty()) {
- LOG.debug("No file has been uploaded for the field: {}",
itemStream.getFieldName());
+ if (exceedsMaxFiles(fileItemInput)) {
return;
}
- File file = null;
- try {
- // Create the temporary upload file.
- file = createTemporaryFile(itemStream.getName(), location);
+ File file = createTemporaryFile(fileItemInput.getName(), location);
+ streamFileToDisk(fileItemInput, file);
- if (streamFileToDisk(itemStream, file)) {
- createFileInfoFromItemStream(itemStream, file);
- }
- } catch (IOException e) {
- if (file != null) {
- try {
- file.delete();
- } catch (SecurityException se) {
- LOG.warn("Failed to delete '{}' due to security exception
above.", file.getName(), se);
- }
- }
+ Long currentFilesSize = maxSizeOfFiles != null ?
actualSizeOfUploadedFiles() : null;
Review Comment:
Not really as it is used inside the `if` clause in `exceedsMaxSizeOfFiles`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
