diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 11e27f287..3d0c778ec 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -52,12 +52,12 @@ jobs: java-version: 17 cache: 'maven' - name: Initialize CodeQL - uses: github/codeql-action/init@v3 + uses: github/codeql-action/init@v3.26.6 with: languages: ${{ matrix.language }} - name: Autobuild - uses: github/codeql-action/autobuild@v3 + uses: github/codeql-action/autobuild@v3.26.6 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 + uses: github/codeql-action/analyze@v3.26.6 with: category: "/language:${{matrix.language}}" diff --git a/.github/workflows/scorecards-analysis.yaml b/.github/workflows/scorecards-analysis.yaml index d2d021ae5..93895ffab 100644 --- a/.github/workflows/scorecards-analysis.yaml +++ b/.github/workflows/scorecards-analysis.yaml @@ -46,7 +46,7 @@ jobs: persist-credentials: false - name: "Run analysis" - uses: ossf/scorecard-action@dc50aa9510b46c811795eb24b2f1ba02a914e534 # 2.3.3 + uses: ossf/scorecard-action@62b2cac7ed8198b15735ed49ab1e5cf35480ba46 # 2.4.0 with: results_file: results.sarif results_format: sarif @@ -58,13 +58,13 @@ jobs: publish_results: true - name: "Upload artifact" - uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # 4.3.4 + uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # 4.4.0 with: name: SARIF file path: results.sarif retention-days: 5 - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@03e7845b7bfcd5e7fb63d1ae8c61b0e791134fab # 2.22.11 + uses: github/codeql-action/upload-sarif@821ab42c90a42d1d5cd3241930dff56a7c7dcfb2 # 2.22.11 with: sarif_file: results.sarif diff --git a/apps/showcase/pom.xml b/apps/showcase/pom.xml index 0fc2d9496..799d9b258 100644 --- a/apps/showcase/pom.xml +++ b/apps/showcase/pom.xml @@ -200,7 +200,7 @@ org.apache.maven.plugins maven-failsafe-plugin - 3.3.0 + 3.3.1 it.org.apache.struts2.showcase.*Test diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 0b9c1641f..043f60204 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -430,11 +430,23 @@ public class OgnlUtil { if (enableExpressionCache) { tree = expressionCache.get(expr); } + if (tree instanceof OgnlException) { + // OgnlException was cached, rethrow it with empty stack trace (refilling the stack trace is expensive) + clearStackTraceAndRethrow(tree); + } if (tree == null) { - tree = ognlGuard.parseExpression(expr); + try { + tree = ognlGuard.parseExpression(expr); + } catch (OgnlException e) { + tree = e; + } if (enableExpressionCache) { expressionCache.put(expr, tree); } + if (tree instanceof OgnlException) { + // Rethrow OgnlException after caching + throw (OgnlException) tree; + } } if (EXPR_BLOCKED.equals(tree)) { throw new OgnlException("Expression blocked by OgnlGuard: " + expr); @@ -442,6 +454,15 @@ public class OgnlUtil { return tree; } + private void clearStackTraceAndRethrow(Object ognlException) throws OgnlException { + OgnlException e = (OgnlException) ognlException; + e.setStackTrace(new StackTraceElement[0]); + if (e.getCause() != null) { + e.getCause().setStackTrace(new StackTraceElement[0]); + } + throw e; + } + public Object compile(String expression, Map context) throws OgnlException { Object tree = toTree(expression); checkEnableEvalExpression(tree, context); diff --git a/core/src/main/java/org/apache/struts2/components/IteratorComponent.java b/core/src/main/java/org/apache/struts2/components/IteratorComponent.java index b17943a0a..1a87c53ce 100644 --- a/core/src/main/java/org/apache/struts2/components/IteratorComponent.java +++ b/core/src/main/java/org/apache/struts2/components/IteratorComponent.java @@ -305,7 +305,10 @@ public class IteratorComponent extends ContextBean { if ((iterator != null) && iterator.hasNext()) { Object currentValue = iterator.next(); stack.push(currentValue); - threadAllowlist.allowClass(currentValue.getClass()); + + if (currentValue != null) { + threadAllowlist.allowClass(currentValue.getClass()); + } String var = getVar(); diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 0d96337da..a88860836 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1648,6 +1648,22 @@ public class OgnlUtilTest extends XWorkTestCase { return generateOgnlUtilInstanceWithDefaultLRUCacheFactories(25, 25); } + public void testCompilationErrorsCached() throws Exception { + OgnlException e = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something")); + StackTraceElement[] stackTrace = e.getStackTrace(); + assertThat(stackTrace).isEmpty(); + StackTraceElement[] causeStackTrace = e.getCause().getStackTrace(); + assertThat(causeStackTrace).isNotEmpty(); + + OgnlException e2 = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something")); + StackTraceElement[] stackTrace2 = e2.getStackTrace(); + assertThat(stackTrace2).isEmpty(); + StackTraceElement[] causeStackTrace2 = e2.getCause().getStackTrace(); + + assertThat(causeStackTrace2).isEmpty(); // Stack trace cleared before rethrow + assertSame(e, e2); // Exception is cached + } + /** * Generate a new OgnlUtil instance (not configured by the {@link ContainerBuilder}) that can be used for * basic tests, with its Expression and BeanInfo factories set to LRU mode. diff --git a/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java b/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java index 7f08ef64e..077510a71 100644 --- a/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java +++ b/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java @@ -184,6 +184,42 @@ public class IteratorComponentTest extends StrutsInternalTestCase { assertEquals("1, 2, , 4, ", out.getBuffer().toString()); } + public void testIteratorWithNullsOnly() { + // given + stack.push(new FooAction() { + private final List items = Arrays.asList(null, null, null); + + public List getItems() { + return items; + } + }); + + StringWriter out = new StringWriter(); + + ic.setValue("items"); + ic.setVar("val"); + Property prop = new Property(stack); + + ic.getComponentStack().push(prop); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(prop); + + String body = ", "; + + // when + assertTrue(ic.start(out)); + + for (int i = 0; i < 3; i++) { + prop.start(out); + prop.end(out, body); + ic.end(out, null); + } + + // then + assertEquals(", , , ", out.getBuffer().toString()); + } + public void testIteratorWithDifferentLocale() { // given ActionContext.getContext().withLocale(new Locale("fa_IR")); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java index 3c355bf1d..40a2e92bd 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java @@ -727,6 +727,41 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new String[]{"a", "b", "c"}); } + public void testNullElements() throws JspException { + Foo foo = new Foo(); + foo.setArray(new String[3]); + + stack.push(foo); + tag.setValue("array"); + tag.setVar("anId"); + + // one + int result = tag.doStartTag(); + assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); + assertNull(stack.peek()); + assertNull(stack.getContext().get("anId")); + + tag.doInitBody(); + + // two + result = tag.doAfterBody(); + assertEquals(TagSupport.EVAL_BODY_AGAIN, result); + assertNull(stack.peek()); + assertNull(stack.getContext().get("anId")); + + // three + result = tag.doAfterBody(); + assertEquals(TagSupport.EVAL_BODY_AGAIN, result); + assertNull(stack.peek()); + assertNull(stack.getContext().get("anId")); + + result = tag.doAfterBody(); + assertEquals(TagSupport.SKIP_BODY, result); + + result = tag.doEndTag(); + assertEquals(TagSupport.EVAL_PAGE, result); + } + public void testCounterWithArray() throws JspException { Foo foo = new Foo(); foo.setArray(new String[]{"a", "b", "c", "d"}); diff --git a/pom.xml b/pom.xml index 2dbda17bc..15eb28004 100644 --- a/pom.xml +++ b/pom.xml @@ -116,11 +116,11 @@ 8.0.1.Final 2.17.2 2.23.1 - 3.3.1 + 3.4.0 5.8.0 3.3.5 2.5.0 - 2.0.13 + 2.0.16 6.0.13 3.0.8 1.0.7 @@ -240,7 +240,7 @@ org.apache.maven.plugins maven-project-info-reports-plugin - 3.5.0 + 3.6.2 org.apache.maven.plugins @@ -332,7 +332,7 @@ org.owasp dependency-check-maven - 9.2.0 + 10.0.3 src/etc/project-suppression.xml @@ -370,7 +370,7 @@ org.apache.maven.plugins maven-wrapper-plugin - 3.2.0 + 3.3.2 @@ -379,7 +379,7 @@ org.apache.maven.plugins maven-release-plugin - 3.0.1 + 3.1.1 maven-jar-plugin @@ -485,7 +485,7 @@ org.codehaus.mojo versions-maven-plugin - 2.16.2 + 2.17.1 @@ -715,7 +715,7 @@ org.easymock easymock - 5.2.0 + 5.4.0 test @@ -797,7 +797,7 @@ commons-logging commons-logging - 1.3.0 + 1.3.3 org.apache.commons @@ -817,7 +817,7 @@ org.apache.commons commons-lang3 - 3.14.0 + 3.15.0 org.apache.commons @@ -974,7 +974,7 @@ org.apache.commons commons-compress - 1.26.2 + 1.27.1