add some improvements

This commit is contained in:
Yasser Zamani
2023-06-05 13:37:59 +04:30
parent 36e4e7410e
commit 2d6f1bc0a6
9 changed files with 108 additions and 15 deletions
@@ -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);
}
@@ -145,6 +145,9 @@ public final class StrutsConstants {
/** The maximized number of files allowed to upload */
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 directory to use for storing uploaded files */
public static final String STRUTS_MULTIPART_SAVEDIR = "struts.multipart.saveDir";
@@ -65,6 +65,7 @@ public class ConstantConfig {
private String uiThemeExpansionToken;
private Long multipartMaxSize;
private Long multipartMaxFiles;
private Long multipartMaxStringLength;
private String multipartSaveDir;
private Integer multipartBufferSize;
private BeanConfig multipartParser;
@@ -197,6 +198,7 @@ public class ConstantConfig {
map.put(StrutsConstants.STRUTS_UI_THEME_EXPANSION_TOKEN, uiThemeExpansionToken);
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_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));
@@ -590,6 +592,14 @@ public class ConstantConfig {
this.multipartMaxFiles = multipartMaxFiles;
}
public Long getMultipartMaxStringLength() {
return multipartMaxStringLength;
}
public void setMultipartMaxStringLength(Long multipartMaxStringLength) {
this.multipartMaxStringLength = multipartMaxStringLength;
}
public String getMultipartSaveDir() {
return multipartSaveDir;
}
@@ -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 buffer size to use during streaming.
*/
@@ -96,6 +101,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
this.maxFiles = Long.parseLong(maxFiles);
}
@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();
@@ -137,8 +137,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 {
@@ -69,6 +69,7 @@ struts.multipart.parser=jakarta
struts.multipart.saveDir=
struts.multipart.maxSize=2097152
struts.multipart.maxFiles=256
struts.multipart.maxStringLength=4096
### Load custom property files (does not override struts.properties!)
# struts.custom.properties=application,org/apache/struts2/extension/custom
@@ -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}
@@ -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()"));
}
@@ -235,7 +235,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
mai.setInvocationContext(ActionContext.getContext());
ActionContext.getContext().setParameters(HttpParameters.create().build());
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000, -1));
interceptor.intercept(mai);
@@ -257,7 +257,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
mai.setInvocationContext(ActionContext.getContext());
ActionContext.getContext().setParameters(HttpParameters.create().build());
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000, -1));
interceptor.intercept(mai);
@@ -288,7 +288,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
mai.setInvocationContext(ActionContext.getContext());
Map<String, Object> param = new HashMap<>();
ActionContext.getContext().setParameters(HttpParameters.create(param).build());
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000, -1));
interceptor.intercept(mai);
@@ -349,7 +349,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
mai.setInvocationContext(ActionContext.getContext());
Map<String, Object> param = new HashMap<String, Object>();
ActionContext.getContext().setParameters(HttpParameters.create(param).build());
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000, -1));
interceptor.setAllowedTypes("text/html");
interceptor.intercept(mai);
@@ -402,7 +402,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
mai.setInvocationContext(ActionContext.getContext());
Map<String, Object> param = new HashMap<>();
ActionContext.getContext().setParameters(HttpParameters.create(param).build());
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000, -1));
interceptor.setAllowedTypes("text/html");
interceptor.intercept(mai);
@@ -413,6 +413,55 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
assertEquals("Request exceeded allowed number of files! Max allowed files number is: 3!", action.getActionErrors().iterator().next());
}
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(createMultipartRequest(req, -1, 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());
@@ -439,7 +488,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext()
.withParameters(HttpParameters.create(param).build())
.withLocale(Locale.GERMAN)
.withServletRequest(createMultipartRequest(req, 10));
.withServletRequest(createMultipartRequest(req, 10, -1));
interceptor.intercept(mai);
@@ -472,10 +521,11 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
return sb.toString();
}
private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize) throws IOException {
private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize, int maxStringLength) throws IOException {
JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
jak.setMaxSize(String.valueOf(maxsize));
jak.setMaxFiles("3");
jak.setMaxStringLength(String.valueOf(maxStringLength));
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}