[
https://issues.apache.org/jira/browse/WW-5273?focusedWorklogId=836091&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-836091
]
ASF GitHub Bot logged work on WW-5273:
--------------------------------------
Author: ASF GitHub Bot
Created on: 29/Dec/22 09:45
Start Date: 29/Dec/22 09:45
Worklog Time Spent: 10m
Work Description: github-code-scanning[bot] commented on code in PR #650:
URL: https://github.com/apache/struts/pull/650#discussion_r1058848403
##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/ServletMultiPartRequest.java:
##########
@@ -0,0 +1,246 @@
+/*
+ * 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.struts2.dispatcher.multipart;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.dispatcher.LocalizedMessage;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.Part;
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+/**
+ * Pure Servlet API 3.1 based implementation
+ */
+public class ServletMultiPartRequest extends AbstractMultiPartRequest {
+
+ private static final Logger LOG =
LogManager.getLogger(ServletMultiPartRequest.class);
+
+ private Map<String, List<FileData>> uploadedFiles = new HashMap<>();
+ private Map<String, List<String>> parameters = new HashMap<>();
+
+ @Override
+ public void parse(HttpServletRequest request, String saveDir) throws
IOException {
+ try {
+ if (isSizeLimitExceeded(request)) {
+ applySizeLimitExceededError(request);
+ return;
+ }
+ parseParts(request, saveDir);
+ } catch (ServletException e) {
+ LOG.warn("Error occurred during parsing of multi part request", e);
+ LocalizedMessage errorMessage = buildErrorMessage(e, new
Object[]{e.getMessage()});
+ if (!errors.contains(errorMessage)) {
+ errors.add(errorMessage);
+ }
+ }
+ }
+
+ private void parseParts(HttpServletRequest request, String saveDir) throws
IOException, ServletException {
+ Collection<Part> parts = request.getParts();
+ if (parts.isEmpty()) {
+ LocalizedMessage error = buildErrorMessage(new IOException(), new
Object[]{"No boundary defined!"});
+ if (!errors.contains(error)) {
+ errors.add(error);
+ }
+ return;
+ }
+ for (Part part : parts) {
+ if (part.getSubmittedFileName() == null) { // normal field
+ LOG.debug("Ignoring a normal form field: {}", part.getName());
+ } else { // file upload
+ LOG.debug("Storing file: {} in save dir: {}",
part.getSubmittedFileName(), saveDir);
Review Comment:
## Logging should not be vulnerable to injection attacks
<!--SONAR_ISSUE_KEY:AYVdRXPwlHcyP0Z-M9IE-->Change this code to not log
user-controlled data. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AYVdRXPwlHcyP0Z-M9IE&open=AYVdRXPwlHcyP0Z-M9IE&pullRequest=650";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/struts/security/code-scanning/205)
##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/ServletMultiPartRequest.java:
##########
@@ -0,0 +1,246 @@
+/*
+ * 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.struts2.dispatcher.multipart;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.dispatcher.LocalizedMessage;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.Part;
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+/**
+ * Pure Servlet API 3.1 based implementation
+ */
+public class ServletMultiPartRequest extends AbstractMultiPartRequest {
+
+ private static final Logger LOG =
LogManager.getLogger(ServletMultiPartRequest.class);
+
+ private Map<String, List<FileData>> uploadedFiles = new HashMap<>();
+ private Map<String, List<String>> parameters = new HashMap<>();
+
+ @Override
+ public void parse(HttpServletRequest request, String saveDir) throws
IOException {
+ try {
+ if (isSizeLimitExceeded(request)) {
+ applySizeLimitExceededError(request);
+ return;
+ }
+ parseParts(request, saveDir);
+ } catch (ServletException e) {
+ LOG.warn("Error occurred during parsing of multi part request", e);
+ LocalizedMessage errorMessage = buildErrorMessage(e, new
Object[]{e.getMessage()});
+ if (!errors.contains(errorMessage)) {
+ errors.add(errorMessage);
+ }
+ }
+ }
+
+ private void parseParts(HttpServletRequest request, String saveDir) throws
IOException, ServletException {
+ Collection<Part> parts = request.getParts();
+ if (parts.isEmpty()) {
+ LocalizedMessage error = buildErrorMessage(new IOException(), new
Object[]{"No boundary defined!"});
+ if (!errors.contains(error)) {
+ errors.add(error);
+ }
+ return;
+ }
+ for (Part part : parts) {
+ if (part.getSubmittedFileName() == null) { // normal field
+ LOG.debug("Ignoring a normal form field: {}", part.getName());
+ } else { // file upload
+ LOG.debug("Storing file: {} in save dir: {}",
part.getSubmittedFileName(), saveDir);
+ parseFile(part, saveDir);
+ }
+ }
+ }
+
+ private boolean isSizeLimitExceeded(HttpServletRequest request) {
+ if (request.getContentLength() > -1) {
+ return maxSizeProvided && request.getContentLength() > maxSize;
+ } else {
+ LOG.debug("Request Content Length is: {} which means the size
overflows 2 GB!", request.getContentLength());
+ return true;
+ }
+ }
+
+ private void applySizeLimitExceededError(HttpServletRequest request) {
+ String exceptionMessage = "Request size: " +
request.getContentLength() + " exceeded maximum size limit: " + maxSize;
+ SizeLimitExceededException exception = new
SizeLimitExceededException(exceptionMessage);
+ LocalizedMessage message = buildErrorMessage(exception, new
Object[]{request.getContentLength(), maxSize});
+ if (!errors.contains(message)) {
+ errors.add(message);
+ }
+ }
+
+ private void parseFile(Part part, String saveDir) throws IOException {
+ File file = extractFile(part, saveDir);
+ List<FileData> data = uploadedFiles.get(part.getName());
+ if (data == null) {
+ data = new ArrayList<>();
+ }
+ data.add(new FileData(file, part.getContentType(),
part.getSubmittedFileName()));
+ uploadedFiles.put(part.getName(), data);
+ }
+
+ private File extractFile(Part part, String saveDir) throws IOException {
+ String name = part.getSubmittedFileName()
+ .substring(part.getSubmittedFileName().lastIndexOf('/') + 1)
+ .substring(part.getSubmittedFileName().lastIndexOf('\\') + 1);
+
+ String prefix = name;
+ String suffix = "";
+
+ if (name.contains(".")) {
+ prefix = name.substring(0, name.lastIndexOf('.'));
+ suffix = name.substring(name.lastIndexOf('.'));
+ }
+
+ if (prefix.length() < 3) {
+ prefix = UUID.randomUUID().toString();
+ }
+
+ File tempFile = File.createTempFile(prefix + "_", suffix, new
File(saveDir));
+ LOG.debug("Stored file: {} as temporary file: {}",
part.getSubmittedFileName(), tempFile.getName());
Review Comment:
## Logging should not be vulnerable to injection attacks
<!--SONAR_ISSUE_KEY:AYVdRXPwlHcyP0Z-M9ID-->Change this code to not log
user-controlled data. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AYVdRXPwlHcyP0Z-M9ID&open=AYVdRXPwlHcyP0Z-M9ID&pullRequest=650";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/struts/security/code-scanning/206)
Issue Time Tracking
-------------------
Worklog Id: (was: 836091)
Time Spent: 0.5h (was: 20m)
> Support fileupload using native Servlet API 3.1 logic
> -----------------------------------------------------
>
> Key: WW-5273
> URL: https://issues.apache.org/jira/browse/WW-5273
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 6.2.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Since Servlet API 3.1 there is no need in using Commons Fileupload as the
> servlets support it.
> https://stackoverflow.com/questions/68820707/jetty-11-and-commons-fileupload
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
