Skip to content

Commit

Permalink
add some improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
yasserzamani committed Jun 4, 2023
1 parent 2520513 commit e58edfb
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ public Object getProperty(Map context, Object target, Object name) throws OgnlEx
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);
}
Expand Down
11 changes: 7 additions & 4 deletions core/src/main/java/org/apache/struts2/StrutsConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ public final class StrutsConstants {

/** Update freemarker templates cache in seconds */
public static final String STRUTS_FREEMARKER_TEMPLATES_CACHE_UPDATE_DELAY = "struts.freemarker.templatesCache.updateDelay";

/** Cache model instances at BeanWrapper level */
public static final String STRUTS_FREEMARKER_BEANWRAPPER_CACHE = "struts.freemarker.beanwrapperCache";

/** Maximum strong sizing for MruCacheStorage for freemarker */
public static final String STRUTS_FREEMARKER_MRU_MAX_STRONG_SIZE = "struts.freemarker.mru.max.strong.size";

/** org.apache.struts2.views.velocity.VelocityManager implementation class */
public static final String STRUTS_VELOCITY_MANAGER_CLASSNAME = "struts.velocity.manager.classname";

Expand All @@ -127,6 +127,9 @@ public final class StrutsConstants {
/** The maximize size of a multipart request (file upload) */
public static final String STRUTS_MULTIPART_MAXSIZE = "struts.multipart.maxSize";

/** 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";

Expand Down Expand Up @@ -180,7 +183,7 @@ public final class StrutsConstants {
* You can specify different prefixes that will be handled by different mappers
*/
public static final String PREFIX_BASED_MAPPER_CONFIGURATION = "struts.mapper.prefixMapping";

/** Whether the Struts filter should serve static content or not */
public static final String STRUTS_SERVE_STATIC_CONTENT = "struts.serve.static";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
protected long maxSize;
protected boolean maxSizeProvided;

/**
* Specifies the maximum length of a string parameter in a multipart request.
*/
protected Long maxStringLength;

/**
* Specifies the buffer size to use during streaming.
*/
Expand Down Expand Up @@ -88,6 +93,11 @@ public void setMaxSize(String maxSize) {
this.maxSize = Long.parseLong(maxSize);
}

@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();
Expand Down Expand Up @@ -134,9 +144,9 @@ protected String getCanonicalName(final String originalFileName) {
int forwardSlash = fileName.lastIndexOf('/');
int backwardSlash = fileName.lastIndexOf('\\');
if (forwardSlash != -1 && forwardSlash > backwardSlash) {
fileName = fileName.substring(forwardSlash + 1, fileName.length());
fileName = fileName.substring(forwardSlash + 1);
} else {
fileName = fileName.substring(backwardSlash + 1, fileName.length());
fileName = fileName.substring(backwardSlash + 1);
}
return fileName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.commons.fileupload.disk.DiskFileItem;
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.dispatcher.LocalizedMessage;
Expand Down Expand Up @@ -55,9 +56,8 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
*
* @param saveDir the directory to save off the file
* @param request the request containing the multipart
* @throws java.io.IOException is thrown if encoding fails.
*/
public void parse(HttpServletRequest request, String saveDir) throws IOException {
public void parse(HttpServletRequest request, String saveDir) {
try {
setLocale(request);
processUpload(request, saveDir);
Expand Down Expand Up @@ -126,13 +126,26 @@ protected void processNormalFormField(FileItem item, String charset) throws Unsu
values = new ArrayList<>();
}

// note: see http://jira.opensymphony.com/browse/WW-633
// basically, in some cases the charset may be null, so
// we're just going to try to "other" method (no idea if this
// will work)
if (charset != null) {
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 {
// note: see http://jira.opensymphony.com/browse/WW-633
// basically, in some cases the charset may be null, so
// we're just going to try to "other" method (no idea if this
// will work)
values.add(item.getString());
}
params.put(item.getFieldName(), values);
Expand Down Expand Up @@ -183,7 +196,7 @@ public String[] getContentType(String fieldName) {
contentTypes.add(fileItem.getContentType());
}

return contentTypes.toArray(new String[contentTypes.size()]);
return contentTypes.toArray(new String[0]);
}

/* (non-Javadoc)
Expand All @@ -209,7 +222,7 @@ public UploadedFile[] getFile(String fieldName) {
fileList.add(new StrutsUploadedFile(storeLocation));
}

return fileList.toArray(new UploadedFile[fileList.size()]);
return fileList.toArray(new UploadedFile[0]);
}

/* (non-Javadoc)
Expand All @@ -227,7 +240,7 @@ public String[] getFileNames(String fieldName) {
fileNames.add(getCanonicalName(fileItem.getName()));
}

return fileNames.toArray(new String[fileNames.size()]);
return fileNames.toArray(new String[0]);
}

/* (non-Javadoc)
Expand All @@ -245,15 +258,15 @@ public String[] getFilesystemName(String fieldName) {
fileNames.add(((DiskFileItem) fileItem).getStoreLocation().getName());
}

return fileNames.toArray(new String[fileNames.size()]);
return fileNames.toArray(new String[0]);
}

/* (non-Javadoc)
* @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameter(java.lang.String)
*/
public String getParameter(String name) {
List<String> v = params.get(name);
if (v != null && v.size() > 0) {
if (v != null && !v.isEmpty()) {
return v.get(0);
}

Expand All @@ -272,8 +285,8 @@ public Enumeration<String> getParameterNames() {
*/
public String[] getParameterValues(String name) {
List<String> v = params.get(name);
if (v != null && v.size() > 0) {
return v.toArray(new String[v.size()]);
if (v != null && !v.isEmpty()) {
return v.toArray(new String[0]);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ struts.multipart.parser=jakarta
# uses javax.servlet.context.tempdir by default
struts.multipart.saveDir=
struts.multipart.maxSize=2097152
struts.multipart.maxStringLength=4096

### Load custom property files (does not override struts.properties!)
# struts.custom.properties=application,org/apache/struts2/extension/custom
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
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;
Expand All @@ -42,11 +42,11 @@ public void testContains() {

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() {
Expand All @@ -60,8 +60,8 @@ public void testCanAccessListSizeProperty() {

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() {
Expand All @@ -73,12 +73,14 @@ public void testAutoGrowthCollectionLimit() {
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()"));
}
Expand Down
Loading

0 comments on commit e58edfb

Please sign in to comment.