This is an automated email from the ASF dual-hosted git repository.
yasserzamani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/master by this push:
new 5a67d58b7 add some improvements
5a67d58b7 is described below
commit 5a67d58b757426376943b084fec89b121392a24c
Author: Yasser Zamani <yasserzam...@apache.org>
AuthorDate: Sun Jun 4 14:22:49 2023 +0430
add some improvements
---
.../ognl/accessor/XWorkListPropertyAccessor.java | 5 ++
.../java/org/apache/struts2/StrutsConstants.java | 3 +
.../struts2/config/entities/ConstantConfig.java | 10 ++++
.../multipart/AbstractMultiPartRequest.java | 10 ++++
.../multipart/JakartaMultiPartRequest.java | 13 ++++-
.../org/apache/struts2/default.properties | 1 +
.../org/apache/struts2/struts-messages.properties | 1 +
.../accessor/XWorkListPropertyAccessorTest.java | 16 +++---
.../interceptor/FileUploadInterceptorTest.java | 64 ++++++++++++++++++++--
9 files changed, 111 insertions(+), 12 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
index ece1a9248..47a640438 100644
---
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
+++
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
@@ -109,6 +109,11 @@ public class XWorkListPropertyAccessor extends
ListPropertyAccessor {
if (listSize <= index) {
Object result;
+ if (index > autoGrowCollectionLimit) {
+ throw new OgnlException("Error auto growing collection
size to " + index + " which limited to "
+ + autoGrowCollectionLimit);
+ }
+
for (int i = listSize; i < index; i++) {
list.add(null);
}
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index e53bb2322..29dfca4b9 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -145,6 +145,9 @@ public final class StrutsConstants {
/** The maximum number of files allowed in a multipart request */
public static final String STRUTS_MULTIPART_MAXFILES =
"struts.multipart.maxFiles";
+ /** The maximum length of a string parameter in a multipart request. */
+ public static final String STRUTS_MULTIPART_MAX_STRING_LENGTH =
"struts.multipart.maxStringLength";
+
/** The maximum size per file in a multipart request */
public static final String STRUTS_MULTIPART_MAXFILESIZE =
"struts.multipart.maxFileSize";
/** The directory to use for storing uploaded files */
diff --git
a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
index 73f22013c..2b854243d 100644
--- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
+++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
@@ -66,6 +66,7 @@ public class ConstantConfig {
private Long multipartMaxSize;
private Long multipartMaxFiles;
private Long multipartMaxFileSize;
+ private Long multipartMaxStringLength;
private String multipartSaveDir;
private Integer multipartBufferSize;
private BeanConfig multipartParser;
@@ -201,6 +202,7 @@ public class ConstantConfig {
map.put(StrutsConstants.STRUTS_MULTIPART_MAXSIZE,
Objects.toString(multipartMaxSize, null));
map.put(StrutsConstants.STRUTS_MULTIPART_MAXFILES,
Objects.toString(multipartMaxFiles, null));
map.put(StrutsConstants.STRUTS_MULTIPART_MAXFILESIZE,
Objects.toString(multipartMaxFileSize, null));
+ map.put(StrutsConstants.STRUTS_MULTIPART_MAX_STRING_LENGTH,
Objects.toString(multipartMaxStringLength, null));
map.put(StrutsConstants.STRUTS_MULTIPART_SAVEDIR, multipartSaveDir);
map.put(StrutsConstants.STRUTS_MULTIPART_BUFFERSIZE,
Objects.toString(multipartBufferSize, null));
map.put(StrutsConstants.STRUTS_MULTIPART_PARSER,
beanConfToString(multipartParser));
@@ -602,6 +604,14 @@ public class ConstantConfig {
this.multipartMaxFileSize = multipartMaxFileSize;
}
+ public Long getMultipartMaxStringLength() {
+ return multipartMaxStringLength;
+ }
+
+ public void setMultipartMaxStringLength(Long multipartMaxStringLength) {
+ this.multipartMaxStringLength = multipartMaxStringLength;
+ }
+
public String getMultipartSaveDir() {
return multipartSaveDir;
}
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index e8806d370..23d879ba4 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -58,6 +58,11 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
*/
protected Long maxFiles;
+ /**
+ * Specifies the maximum length of a string parameter in a multipart
request.
+ */
+ protected Long maxStringLength;
+
/**
* Specifies the maximum size per file in the request.
*/
@@ -106,6 +111,11 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
this.maxFileSize = Long.parseLong(maxFileSize);
}
+ @Inject(StrutsConstants.STRUTS_MULTIPART_MAX_STRING_LENGTH)
+ public void setMaxStringLength(String maxStringLength) {
+ this.maxStringLength = Long.parseLong(maxStringLength);
+ }
+
@Inject
public void setLocaleProviderFactory(LocaleProviderFactory
localeProviderFactory) {
defaultLocale =
localeProviderFactory.createLocaleProvider().getLocale();
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index 551e1d51c..cd0aaff96 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -140,8 +140,19 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
values = new ArrayList<>();
}
- if (item.getSize() == 0) {
+ long size = item.getSize();
+ if (size == 0) {
values.add(StringUtils.EMPTY);
+ } else if (size > maxStringLength) {
+ String errorKey = "struts.messages.upload.error.parameter.too.long";
+ LocalizedMessage localizedMessage = new
LocalizedMessage(this.getClass(), errorKey, null,
+ new Object[] { item.getFieldName(), maxStringLength, size });
+
+ if (!errors.contains(localizedMessage)) {
+ errors.add(localizedMessage);
+ }
+ return;
+
} else if (charset != null) {
values.add(item.getString(charset));
} else {
diff --git a/core/src/main/resources/org/apache/struts2/default.properties
b/core/src/main/resources/org/apache/struts2/default.properties
index c516604bf..56bc714de 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -69,6 +69,7 @@ struts.multipart.parser=jakarta
struts.multipart.saveDir=
struts.multipart.maxSize=2097152
struts.multipart.maxFiles=256
+struts.multipart.maxStringLength=4096
# struts.multipart.maxFileSize=
### Load custom property files (does not override struts.properties!)
diff --git
a/core/src/main/resources/org/apache/struts2/struts-messages.properties
b/core/src/main/resources/org/apache/struts2/struts-messages.properties
index 1d8f2a7a9..57b7e2b54 100644
--- a/core/src/main/resources/org/apache/struts2/struts-messages.properties
+++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties
@@ -26,6 +26,7 @@ struts.messages.invalid.content.type=Could not find a
Content-Type for {0}. Veri
struts.messages.removing.file=Removing file {0} {1}
struts.messages.error.uploading=Error uploading: {0}
struts.messages.error.file.too.large=File {0} is too large to be uploaded.
Maximum allowed size is {4} bytes!
+struts.messages.upload.error.parameter.too.long=The request parameter "{0}"
was too long. Max length allowed is {1}, but found {2}!
struts.messages.error.content.type.not.allowed=Content-Type not allowed: {0}
"{1}" "{2}" {3}
struts.messages.error.file.extension.not.allowed=File extension not allowed:
{0} "{1}" "{2}" {3}
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
index 66c716c2c..4a89d8935 100644
---
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
@@ -22,7 +22,7 @@ import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.XWorkTestCase;
import com.opensymphony.xwork2.util.ListHolder;
import com.opensymphony.xwork2.util.ValueStack;
-import ognl.ListPropertyAccessor;
+import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import ognl.PropertyAccessor;
import java.util.ArrayList;
@@ -42,11 +42,11 @@ public class XWorkListPropertyAccessorTest extends
XWorkTestCase {
assertNotNull(listHolder.getLongs());
assertEquals(3, listHolder.getLongs().size());
- assertEquals(new Long(1), (Long) listHolder.getLongs().get(0));
- assertEquals(new Long(2), (Long) listHolder.getLongs().get(1));
- assertEquals(new Long(3), (Long) listHolder.getLongs().get(2));
+ assertEquals(new Long(1), listHolder.getLongs().get(0));
+ assertEquals(new Long(2), listHolder.getLongs().get(1));
+ assertEquals(new Long(3), listHolder.getLongs().get(2));
- assertTrue(((Boolean)
vs.findValue("longs.contains(1)")).booleanValue());
+ assertTrue((Boolean) vs.findValue("longs.contains(1)"));
}
public void testCanAccessListSizeProperty() {
@@ -60,8 +60,8 @@ public class XWorkListPropertyAccessorTest extends
XWorkTestCase {
vs.push(listHolder);
- assertEquals(new Integer(myList.size()),
vs.findValue("strings.size()"));
- assertEquals(new Integer(myList.size()), vs.findValue("strings.size"));
+ assertEquals(myList.size(), vs.findValue("strings.size()"));
+ assertEquals(myList.size(), vs.findValue("strings.size"));
}
public void testAutoGrowthCollectionLimit() {
@@ -73,12 +73,14 @@ public class XWorkListPropertyAccessorTest extends
XWorkTestCase {
listHolder.setStrings(myList);
ValueStack vs = ActionContext.getContext().getValueStack();
+ ReflectionContextState.setCreatingNullObjects(vs.getContext(), true);
vs.push(listHolder);
vs.setValue("strings[0]", "a");
vs.setValue("strings[1]", "b");
vs.setValue("strings[2]", "c");
vs.setValue("strings[3]", "d");
+ vs.findValue("strings[3]");
assertEquals(3, vs.findValue("strings.size()"));
}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
index 0e6874086..b9bab88d4 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
@@ -452,6 +452,55 @@ public class FileUploadInterceptorTest extends
StrutsInternalTestCase {
msg);
}
+ public void testMultipartRequestMaxStringLength() throws Exception {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setCharacterEncoding(StandardCharsets.UTF_8.name());
+ req.setMethod("post");
+ req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
+
+ // inspired by the unit tests for jakarta commons fileupload
+ String content = ("-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file\";
filename=\"deleteme.txt\"\r\n" +
+ "Content-Type: text/html\r\n" +
+ "\r\n" +
+ "Unit test of FileUploadInterceptor" +
+ "\r\n" +
+ "-----1234\r\n" +
+ "Content-Disposition: form-data;
name=\"normalFormField1\"\r\n" +
+ "\r\n" +
+ "it works" +
+ "\r\n" +
+ "-----1234\r\n" +
+ "Content-Disposition: form-data;
name=\"normalFormField2\"\r\n" +
+ "\r\n" +
+ "long string should not work" +
+ "\r\n" +
+ "-----1234--\r\n");
+ req.setContent(content.getBytes(StandardCharsets.US_ASCII));
+
+ MyFileupAction action = container.inject(MyFileupAction.class);
+
+ MockActionInvocation mai = new MockActionInvocation();
+ mai.setAction(action);
+ mai.setResultCode("success");
+ mai.setInvocationContext(ActionContext.getContext());
+ Map<String, Object> param = new HashMap<>();
+ ActionContext.getContext()
+ .withParameters(HttpParameters.create(param).build())
+ .withServletRequest(createMultipartRequestMaxStringLength(req,
20));
+
+ interceptor.intercept(mai);
+
+ assertTrue(action.hasActionErrors());
+
+ Collection<String> errors = action.getActionErrors();
+ assertEquals(1, errors.size());
+ String msg = errors.iterator().next();
+ assertEquals(
+ "The request parameter \"normalFormField2\" was too long. Max
length allowed is 20, but found 27!",
+ msg);
+ }
+
public void testMultipartRequestLocalizedError() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setCharacterEncoding(StandardCharsets.UTF_8.name());
@@ -512,22 +561,29 @@ public class FileUploadInterceptorTest extends
StrutsInternalTestCase {
}
private MultiPartRequestWrapper
createMultipartRequestMaxFileSize(HttpServletRequest req, int maxfilesize)
throws IOException {
- return createMultipartRequest(req, -1, maxfilesize, -1);
+ return createMultipartRequest(req, -1, maxfilesize, -1, -1);
}
private MultiPartRequestWrapper
createMultipartRequestMaxFiles(HttpServletRequest req, int maxfiles) throws
IOException {
- return createMultipartRequest(req, -1, -1, maxfiles);
+ return createMultipartRequest(req, -1, -1, maxfiles, -1);
}
private MultiPartRequestWrapper
createMultipartRequestMaxSize(HttpServletRequest req, int maxsize) throws
IOException {
- return createMultipartRequest(req, maxsize, -1, -1);
+ return createMultipartRequest(req, maxsize, -1, -1, -1);
}
- private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest
req, int maxsize, int maxfilesize, int maxfiles) throws IOException {
+ private MultiPartRequestWrapper
createMultipartRequestMaxStringLength(HttpServletRequest req, int
maxStringLength) throws IOException {
+ return createMultipartRequest(req, -1, -1, -1, maxStringLength);
+ }
+
+ private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest
req, int maxsize, int maxfilesize,
+ int maxfiles, int
maxStringLength) throws IOException {
+
JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
jak.setMaxSize(String.valueOf(maxsize));
jak.setMaxFileSize(String.valueOf(maxfilesize));
jak.setMaxFiles(String.valueOf(maxfiles));
+ jak.setMaxStringLength(String.valueOf(maxStringLength));
return new MultiPartRequestWrapper(jak, req,
tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
