struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yasserzam...@apache.org
Subject [struts] 01/01: fix double evaluations...
Date Thu, 08 Apr 2021 19:35:27 GMT
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 <yasserzamani@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();
+    }
 }

Mime
View raw message