This is an automated email from the ASF dual-hosted git repository. kusal pushed a commit to branch 7.0.x/merge-master-2024-11-01 in repository https://gitbox.apache.org/repos/asf/struts.git
commit f55c404d21a89b1178a798aa2112243563584962 Merge: a34bffdf3 8566c1464 Author: Kusal Kithul-Godage <g...@kusal.io> AuthorDate: Fri Nov 1 15:55:35 2024 +1100 Merge branch 'master' into 7.0.x/merge-master-2024-11-01 .github/workflows/codeql.yml | 6 +- .github/workflows/scorecards-analysis.yaml | 2 +- apps/showcase/pom.xml | 2 +- .../struts2/views/freemarker/ScopesHashModel.java | 12 +- .../ActionFileUploadInterceptorTest.java | 152 +++++++++++++++++++++ pom.xml | 8 +- 6 files changed, 167 insertions(+), 15 deletions(-) diff --cc .github/workflows/codeql.yml index 97a4ed500,0846bfe9f..5794f8746 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@@ -45,14 -43,8 +45,14 @@@ jobs steps: - name: Checkout repository uses: actions/checkout@v4 + - name: Setup Java JDK + uses: actions/setup-java@v3 + with: + distribution: temurin + java-version: 17 + cache: 'maven' - name: Initialize CodeQL - uses: github/codeql-action/init@v3.26.12 + uses: github/codeql-action/init@v3.27.0 with: languages: ${{ matrix.language }} - name: Autobuild diff --cc core/src/main/java/org/apache/struts2/views/freemarker/ScopesHashModel.java index b2c27497c,33be31466..ac49c8326 --- a/core/src/main/java/org/apache/struts2/views/freemarker/ScopesHashModel.java +++ b/core/src/main/java/org/apache/struts2/views/freemarker/ScopesHashModel.java @@@ -50,18 -49,21 +50,18 @@@ import java.util.Map */ public class ScopesHashModel extends SimpleHash implements TemplateModel { + private static final Logger LOG = LogManager.getLogger(ScopesHashModel.class); + + @Serial private static final long serialVersionUID = 5551686380141886764L; - private static final Logger LOG = LogManager.getLogger(ScopesHashModel.class); private static final String TAG_ATTRIBUTES = "attributes"; - /** - * @deprecated since 6.7.0, use {@link #TAG_ATTRIBUTES} instead - */ - @Deprecated - private static final String TAG_PARAMETERS = "parameters"; - private HttpServletRequest request; - private ServletContext servletContext; + private final HttpServletRequest request; + private final ServletContext servletContext; private ValueStack stack; private final Map<String, TemplateModel> unlistedModels = new HashMap<>(); - private volatile Object parametersCache; + private volatile Object attributesCache; public ScopesHashModel(ObjectWrapper objectWrapper, ServletContext context, HttpServletRequest request, ValueStack stack) { super(objectWrapper); @@@ -154,13 -156,13 +154,13 @@@ } private Object findValueOnStack(final String key) { - if (TAG_ATTRIBUTES.equals(key) || TAG_PARAMETERS.equals(key)) { + if (TAG_ATTRIBUTES.equals(key)) { - if (parametersCache != null) { - return parametersCache; + if (attributesCache != null) { + return attributesCache; } - Object parametersLocal = stack.findValue(key); - parametersCache = parametersLocal; - return parametersLocal; + Object attributesLocal = stack.findValue(key); + attributesCache = attributesLocal; + return attributesLocal; } return stack.findValue(key); } diff --cc core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java index 178d6bfc0,81aa122ed..876aec960 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@@ -25,9 -25,8 +25,10 @@@ import com.opensymphony.xwork2.Validati import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.util.ClassLoaderUtil; -import org.apache.commons.fileupload.servlet.ServletFileUpload; -import org.apache.struts2.ServletActionContext; ++import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; +import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletFileUpload; import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.action.UploadedFilesAware; import org.apache.struts2.dispatcher.multipart.JakartaMultiPartRequest; @@@ -324,8 -324,150 +325,150 @@@ public class ActionFileUploadIntercepto assertNotNull("deleteme.txt", files.get(0).getOriginalName()); } + public void testSuccessUploadOfATextFileMultipartRequestNoMaxParamsSet() 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 ActionFileUploadInterceptor" + + "\r\n" + + "-----1234--\r\n"); + req.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + MyFileUploadAction action = new MyFileUploadAction(); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().withServletRequest(createMultipartRequestNoMaxParamsSet(req)); + + interceptor.intercept(mai); + + assertFalse(action.hasErrors()); + + List<UploadedFile> files = action.getUploadFiles(); + + assertNotNull(files); + assertEquals(1, files.size()); + assertEquals("text/html", files.get(0).getContentType()); + assertNotNull("deleteme.txt", files.get(0).getOriginalName()); + } + + public void testSuccessUploadOfATextFileMultipartRequestWithNormalFieldsMaxParamsSet() 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 ActionFileUploadInterceptor" + + "\r\n" + + "-----1234\r\n" + + "Content-Disposition: form-data; name=\"normalFormField1\"\r\n" + + "\r\n" + + "normal field 1" + + "\r\n" + + "-----1234\r\n" + + "Content-Disposition: form-data; name=\"normalFormField2\"\r\n" + + "\r\n" + + "normal field 2" + + "\r\n" + + "-----1234--\r\n"); + req.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + MyFileUploadAction action = new MyFileUploadAction(); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); - ActionContext.getContext().withServletRequest(createMultipartRequest(req, 2000, 2000, 5, 100)); ++ ActionContext.getContext().withServletRequest(createMultipartRequest(2000, 2000, 5, 100)); + + interceptor.intercept(mai); + + assertFalse(action.hasErrors()); + + List<UploadedFile> files = action.getUploadFiles(); + + assertNotNull(files); + assertEquals(1, files.size()); + assertEquals("text/html", files.get(0).getContentType()); + assertNotNull("deleteme.txt", files.get(0).getOriginalName()); + + // Confirm normalFormField1, normalFormField2 were processed by the MultiPartRequestWrapper. + HttpServletRequest invocationServletRequest = mai.getInvocationContext().getServletRequest(); + assertTrue("invocation servelt request is not a MultiPartRequestWrapper ?", invocationServletRequest instanceof MultiPartRequestWrapper); + MultiPartRequestWrapper multipartRequestWrapper = (MultiPartRequestWrapper) invocationServletRequest; + assertNotNull("normalFormField1 missing from MultiPartRequestWrapper parameters ?", multipartRequestWrapper.getParameter("normalFormField1")); + assertNotNull("normalFormField2 missing from MultiPartRequestWrapper parameters ?", multipartRequestWrapper.getParameter("normalFormField2")); + } + + public void testSuccessUploadOfATextFileMultipartRequestWithNormalFieldsNoMaxParamsSet() 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 ActionFileUploadInterceptor" + + "\r\n" + + "-----1234\r\n" + + "Content-Disposition: form-data; name=\"normalFormField1\"\r\n" + + "\r\n" + + "normal field 1" + + "\r\n" + + "-----1234\r\n" + + "Content-Disposition: form-data; name=\"normalFormField2\"\r\n" + + "\r\n" + + "normal field 2" + + "\r\n" + + "-----1234--\r\n"); + req.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + MyFileUploadAction action = new MyFileUploadAction(); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().withServletRequest(createMultipartRequestNoMaxParamsSet(req)); + + interceptor.intercept(mai); + + assertFalse(action.hasErrors()); + + List<UploadedFile> files = action.getUploadFiles(); + + assertNotNull(files); + assertEquals(1, files.size()); + assertEquals("text/html", files.get(0).getContentType()); + assertNotNull("deleteme.txt", files.get(0).getOriginalName()); + + // Confirm normalFormField1, normalFormField2 were processed by the MultiPartRequestWrapper. + HttpServletRequest invocationServletRequest = mai.getInvocationContext().getServletRequest(); + assertTrue("invocation servelt request is not a MultiPartRequestWrapper ?", invocationServletRequest instanceof MultiPartRequestWrapper); + MultiPartRequestWrapper multipartRequestWrapper = (MultiPartRequestWrapper) invocationServletRequest; + assertNotNull("normalFormField1 missing from MultiPartRequestWrapper parameters ?", multipartRequestWrapper.getParameter("normalFormField1")); + assertNotNull("normalFormField2 missing from MultiPartRequestWrapper parameters ?", multipartRequestWrapper.getParameter("normalFormField2")); + } + /** - * tests whether with multiple files sent with the same name, the ones with forbiddenTypes (see + * Tests whether with multiple files sent with the same name, the ones with forbiddenTypes (see * ActionFileUploadInterceptor.setAllowedTypes(...) ) are sorted out. */ public void testMultipleAccept() throws Exception { @@@ -610,13 -703,18 +753,19 @@@ jak.setMaxFileSize(String.valueOf(maxfilesize)); jak.setMaxFiles(String.valueOf(maxfiles)); jak.setMaxStringLength(String.valueOf(maxStringLength)); - return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); + jak.setDefaultEncoding(StandardCharsets.UTF_8.name()); + return new MultiPartRequestWrapper(jak, request, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); } + private MultiPartRequestWrapper createMultipartRequestNoMaxParamsSet(HttpServletRequest req) { + + JakartaMultiPartRequest jak = new JakartaMultiPartRequest(); + return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); + } + protected void setUp() throws Exception { super.setUp(); - + request = new MockHttpServletRequest(); interceptor = new ActionFileUploadInterceptor(); container.inject(interceptor); tempDir = File.createTempFile("struts", "fileupload"); diff --cc pom.xml index d2544f4db,be5a692f0..5da04c34d --- a/pom.xml +++ b/pom.xml @@@ -104,26 -104,22 +104,26 @@@ <properties> <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> - <project.build.outputTimestamp>2024-10-05T14:08:38Z</project.build.outputTimestamp> - <maven.compiler.source>1.8</maven.compiler.source> - <maven.compiler.target>1.8</maven.compiler.target> + <project.build.outputTimestamp>2024-07-21T06:59:43Z</project.build.outputTimestamp> + <maven.compiler.release>17</maven.compiler.release> + <maven.compiler.source>17</maven.compiler.source> + <maven.compiler.target>17</maven.compiler.target> <!-- dependency versions in alphanumeric order --> - <asm.version>9.7</asm.version> + <asm.version>9.7.1</asm.version> + <byte-buddy.version>1.14.11</byte-buddy.version> + <freemarker.version>2.3.33</freemarker.version> + <hibernate-validator.version>8.0.1.Final</hibernate-validator.version> <jackson.version>2.18.0</jackson.version> <log4j2.version>2.24.1</log4j2.version> - <maven-surefire-plugin.version>3.5.0</maven-surefire-plugin.version> ++ <maven-surefire-plugin.version>3.5.1</maven-surefire-plugin.version> + <mockito.version>5.8.0</mockito.version> <ognl.version>3.3.5</ognl.version> <slf4j.version>2.0.16</slf4j.version> - <spring.platformVersion>5.3.39</spring.platformVersion> + <spring.platformVersion>6.0.13</spring.platformVersion> <tiles.version>3.0.8</tiles.version> <tiles-request.version>1.0.7</tiles-request.version> - <maven-surefire-plugin.version>3.5.1</maven-surefire-plugin.version> - <hibernate-validator.version>6.2.4.Final</hibernate-validator.version> - <freemarker.version>2.3.33</freemarker.version> + <velocity-tools.version>3.1</velocity-tools.version> <!-- Site generation --> <fluido-skin.version>1.9</fluido-skin.version>