You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ya...@apache.org on 2021/04/08 19:35:27 UTC
[struts] 01/01: fix double evaluations...
This is an automated email from the ASF dual-hosted git repository.
yasserzamani pushed a commit to branch fix/double_evaluations
in repository https://gitbox.apache.org/repos/asf/struts.git
commit d2640965ac8c472e55f780acdd06b2f2f0a88499
Author: Yasser Zamani <ya...@apache.org>
AuthorDate: Fri Apr 9 00:04:52 2021 +0430
fix double evaluations...
* fix UIBean and its subclasses double evaluations
* fix Param tag used with Bean tag double evaluations
(to be continued)
Reference:
https://securitylab.github.com/research/apache-struts-double-evaluation/
---
.../org/apache/struts2/components/Component.java | 72 +++++++++++++++++---
.../java/org/apache/struts2/components/Param.java | 2 +-
.../java/org/apache/struts2/components/UIBean.java | 8 +--
.../test/java/org/apache/struts2/TestAction.java | 11 +++
.../org/apache/struts2/components/UIBeanTest.java | 55 ++++++++++++++-
.../org/apache/struts2/views/jsp/BeanTagTest.java | 78 ++++++++++++++++++++++
6 files changed, 207 insertions(+), 19 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/components/Component.java b/core/src/main/java/org/apache/struts2/components/Component.java
index e20b4f5..14555b3 100644
--- a/core/src/main/java/org/apache/struts2/components/Component.java
+++ b/core/src/main/java/org/apache/struts2/components/Component.java
@@ -232,6 +232,27 @@ public class Component {
}
/**
+ * Evaluates the OGNL stack to find and process a String value.
+ * @param expr OGNL expression.
+ * @param evaluator the processor
+ * @return the String value found and processed by evaluator.
+ * @since 2.5.27
+ */
+ protected String findString(String expr, final TextParseUtil.ParsedValueEvaluator evaluator) {
+ Object value = findValue(expr, String.class, evaluator);
+ if (null == value) {
+ return null;
+ }
+
+ String s = value.toString();
+ if (s.trim().isEmpty()) {
+ return null;
+ }
+
+ return s;
+ }
+
+ /**
* Evaluates the OGNL stack to find a String value.
* <br>
* If the given expression is <tt>null</tt> a error is logged and a <code>RuntimeException</code> is thrown
@@ -363,9 +384,29 @@ public class Component {
* @return the Object found, or <tt>null</tt> if not found.
*/
protected Object findValue(String expression, Class<?> toType) {
+ return findValue(expression, toType, null);
+ }
+
+ /**
+ * Evaluates the OGNL stack to find an Object of the given type. Will evaluate and process
+ * <code>expression</code> the portion wrapped with %{...} against stack if
+ * evaluating to String.class, else the whole <code>expression</code> is evaluated
+ * against the stack.
+ *
+ * @param expression OGNL expression.
+ * @param toType the type expected to find.
+ * @param evaluator the processor
+ * @return the Object found, or <tt>null</tt> if not found.
+ * @since 2.5.27
+ */
+ protected Object findValue(String expression, Class<?> toType, final TextParseUtil.ParsedValueEvaluator evaluator) {
if (toType == String.class) {
if (ComponentUtils.containsExpression(expression)) {
- return TextParseUtil.translateVariables('%', expression, stack);
+ if (null != evaluator) {
+ return TextParseUtil.translateVariables('%', expression, stack, String.class, evaluator);
+ } else {
+ return TextParseUtil.translateVariables('%', expression, stack);
+ }
} else {
return expression;
}
@@ -377,16 +418,6 @@ public class Component {
}
/**
- * Detects if expression already contains %{...}
- *
- * @param expression a string to examined
- * @return true if expression contains %{...}
- */
- protected boolean recursion(String expression) {
- return ComponentUtils.containsExpression(expression);
- }
-
- /**
* Renders an action URL by consulting the {@link org.apache.struts2.dispatcher.mapper.ActionMapper}.
*
* @param action the action
@@ -571,4 +602,23 @@ public class Component {
return standardAttributes;
}
+
+ /**
+ * {@link com.opensymphony.xwork2.util.TextParseUtil.ParsedValueEvaluator} to filter not nested java identifiers
+ * values out. To be used when only a nested java identifiers is expected.
+ */
+ static final class NestedJavaIdentifierFilter implements TextParseUtil.ParsedValueEvaluator {
+ public Object evaluate(String parsedValue) {
+ for (int i = 0; i < parsedValue.length(); i++) {
+ char ch = parsedValue.charAt(i);
+ if ('.' != ch && '[' != ch && ']' != ch && '\'' != ch && '"' != ch && !Character.isJavaIdentifierPart(ch)) {
+ LOG.warn("since 2.5.27 following expression must be a nested java identifiers (e.g. foo[index].bar)" +
+ " so it won't be evaluated. Please consider a new design. Expression: {}", parsedValue);
+ return null;
+ }
+ }
+
+ return parsedValue;
+ }
+ }
}
diff --git a/core/src/main/java/org/apache/struts2/components/Param.java b/core/src/main/java/org/apache/struts2/components/Param.java
index 023761a..3c13c93 100644
--- a/core/src/main/java/org/apache/struts2/components/Param.java
+++ b/core/src/main/java/org/apache/struts2/components/Param.java
@@ -125,7 +125,7 @@ public class Param extends Component {
if (component instanceof UnnamedParametric) {
((UnnamedParametric) component).addParameter(findValue(value));
} else {
- String name = findString(this.name);
+ String name = findString(this.name, new NestedJavaIdentifierFilter());
if (name == null) {
throw new StrutsException("No name found for following expression: " + this.name);
diff --git a/core/src/main/java/org/apache/struts2/components/UIBean.java b/core/src/main/java/org/apache/struts2/components/UIBean.java
index 2bafabc..dbaed74 100644
--- a/core/src/main/java/org/apache/struts2/components/UIBean.java
+++ b/core/src/main/java/org/apache/struts2/components/UIBean.java
@@ -671,7 +671,7 @@ public abstract class UIBean extends Component {
}
if (this.name != null) {
- name = findString(this.name);
+ name = findString(this.name, new NestedJavaIdentifierFilter());
addParameter("name", name);
}
@@ -805,11 +805,7 @@ public abstract class UIBean extends Component {
addParameter("nameValue", findValue(value, valueClazz));
} else if (name != null) {
String expr = completeExpression(name);
- if (recursion(name)) {
- addParameter("nameValue", expr);
- } else {
- addParameter("nameValue", findValue(expr, valueClazz));
- }
+ addParameter("nameValue", findValue(expr, valueClazz));
}
} else {
if (value != null) {
diff --git a/core/src/test/java/org/apache/struts2/TestAction.java b/core/src/test/java/org/apache/struts2/TestAction.java
index 35b48b3..b4bebd6 100644
--- a/core/src/test/java/org/apache/struts2/TestAction.java
+++ b/core/src/test/java/org/apache/struts2/TestAction.java
@@ -235,4 +235,15 @@ public class TestAction extends ActionSupport {
this.id = id;
}
+ public Map<String, String> getMyTexts() {
+ return texts;
+ }
+
+ public User getMyUser() {
+ if (null == user) {
+ user = new User();
+ }
+
+ return user;
+ }
}
diff --git a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
index e4b0c63..5ef0d54 100644
--- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
+++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
@@ -301,7 +301,60 @@ public class UIBeanTest extends StrutsInternalTestCase {
txtFld.setName("%{myValue}");
txtFld.evaluateParams();
- assertEquals("%{myBad}", txtFld.getParameters().get("nameValue"));
+ assertNull(txtFld.getParameters().get("name"));
+ assertNull(txtFld.getParameters().get("nameValue"));
+ }
+
+ public void testValueParameterIsJavaIdentifier() {
+ ValueStack stack = ActionContext.getContext().getValueStack();
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ MockHttpServletResponse res = new MockHttpServletResponse();
+
+ stack.push(new Object() {
+ public Object getMy良い() {
+ return new Object() {
+ public String getMyValue名前() {
+ return "myValue";
+ }
+ };
+ }
+ public String getMyValue () {
+ return "%{myGood}";
+ }
+ });
+
+ TextField txtFld = new TextField(stack, req, res);
+ txtFld.setName("%{my良い.myValue名前}");
+ txtFld.evaluateParams();
+
+ assertEquals("myValue", txtFld.getParameters().get("name"));
+ assertEquals("%{myGood}", txtFld.getParameters().get("nameValue"));
+ }
+
+ public void testValueParameterNotJavaIdentifier() {
+ ValueStack stack = ActionContext.getContext().getValueStack();
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ MockHttpServletResponse res = new MockHttpServletResponse();
+
+ stack.push(new Object() {
+ public Object getMyGood() {
+ return new Object() {
+ public String getMyValueName() {
+ return "getMyValue()";
+ }
+ };
+ }
+ public String getMyValue() {
+ return "%{myValue}";
+ }
+ });
+
+ TextField txtFld = new TextField(stack, req, res);
+ txtFld.setName("%{myGood.myValueName}");
+ txtFld.evaluateParams();
+
+ assertNull(txtFld.getParameters().get("name"));
+ assertNull(txtFld.getParameters().get("nameValue"));
}
public void testSetClass() {
diff --git a/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java
index 168ffe5..3c43c01 100644
--- a/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java
+++ b/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java
@@ -18,7 +18,12 @@
*/
package org.apache.struts2.views.jsp;
+import org.apache.struts2.StrutsException;
+import org.apache.struts2.dispatcher.HttpParameters;
+
import javax.servlet.jsp.JspException;
+import java.util.HashMap;
+import java.util.Map;
/**
@@ -47,4 +52,77 @@ public class BeanTagTest extends AbstractUITagTest {
request.verify();
pageContext.verify();
}
+
+ public void testIsJavaIdentifier() throws Exception {
+ BeanTag tag = new BeanTag();
+ tag.setPageContext(pageContext);
+ tag.setName("org.apache.struts2.TestAction");
+
+ Map<String, String> tmp = new HashMap<>();
+ tmp.put("property", "array[0]");
+ tmp.put("property2", "myTexts['key']");
+ tmp.put("property3", "myTexts[\"key2\"]");
+ tmp.put("property4", "myUser.name");
+ context.put("parameters", HttpParameters.create(tmp).build());
+ ParamTag param1 = new ParamTag();
+ param1.setPageContext(pageContext);
+ param1.setName("%{#parameters['property']}");
+ param1.setValue("20");
+ ParamTag param2 = new ParamTag();
+ param2.setPageContext(pageContext);
+ param2.setName("%{#parameters['property2']}");
+ param2.setValue("{'err20', 'err21'}");
+ ParamTag param3 = new ParamTag();
+ param3.setPageContext(pageContext);
+ param3.setName("%{#parameters['property3']}");
+ param3.setValue("{'err22', 'err23'}");
+ ParamTag param4 = new ParamTag();
+ param4.setPageContext(pageContext);
+ param4.setName("%{#parameters['property4']}");
+ param4.setValue("%{getStatus()}");
+
+ tag.doStartTag();
+ tag.component.addParameter("array", "instantiate array[0]");
+ param1.doStartTag();
+ param1.doEndTag();
+ param2.doStartTag();
+ param2.doEndTag();
+ param3.doStartTag();
+ param3.doEndTag();
+ param4.doStartTag();
+ param4.doEndTag();
+
+ assertEquals("20", stack.findValue("array[0]"));
+ assertEquals("[err20, err21]", stack.findValue("myTexts.key").toString());
+ assertEquals("[err22, err23]", stack.findValue("myTexts.key2").toString());
+ assertEquals("COMPLETED", stack.findValue("myUser.name"));
+
+ tag.doEndTag();
+ }
+
+ public void testIsNotJavaIdentifier() throws Exception {
+ BeanTag tag = new BeanTag();
+ tag.setPageContext(pageContext);
+ tag.setName("org.apache.struts2.TestAction");
+
+ Map<String, String> tmp = new HashMap<>();
+ tmp.put("property", "getResult()");
+ context.put("parameters", HttpParameters.create(tmp).build());
+ ParamTag param1 = new ParamTag();
+ param1.setPageContext(pageContext);
+ param1.setName("%{#parameters['property']}");
+ param1.setValue("20");
+
+ tag.doStartTag();
+ param1.doStartTag();
+
+ try {
+ param1.doEndTag();
+ fail("a not nested java identifiers is evaluated?!");
+ } catch (StrutsException e) {
+ assertEquals("No name found for following expression: %{#parameters['property']}", e.getMessage());
+ }
+
+ tag.doEndTag();
+ }
}