WW-5041 Upgrade to OGNL 3.1.26 and adapt to its new features

core/src/main/java/com/opensymphony/xwork2/inject/ContainerImpl.java

@@ -164,7 +164,8 @@ class ContainerImpl implements Container {
         public FieldInjector(ContainerImpl container, Field field, String name)
                 throws MissingDependencyException {
             this.field = field;
-            if (!field.isAccessible()) {
+            if ((!Modifier.isPublic(field.getModifiers()) || !Modifier.isPublic(field.getDeclaringClass().getModifiers()))
+                    && !field.isAccessible()) {
                 SecurityManager sm = System.getSecurityManager();
                 try {
                     if (sm != null) {
@@ -256,7 +257,8 @@ class ContainerImpl implements Container {
 
         public MethodInjector(ContainerImpl container, Method method, String name) throws MissingDependencyException {
             this.method = method;
-            if (!method.isAccessible()) {
+            if ((!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
+                    && !method.isAccessible()) {
                 SecurityManager sm = System.getSecurityManager();
                 try {
                     if (sm != null) {
@@ -306,7 +308,8 @@ class ContainerImpl implements Container {
             this.implementation = implementation;
 
             constructor = findConstructorIn(implementation);
-            if (!constructor.isAccessible()) {
+            if ((!Modifier.isPublic(constructor.getModifiers()) || !Modifier.isPublic(constructor.getDeclaringClass().getModifiers()))
+                    && !constructor.isAccessible()) {
                 SecurityManager sm = System.getSecurityManager();
                 try {
                     if (sm != null) {

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java

@@ -184,6 +184,19 @@ public class OgnlUtil {
         this.disallowProxyMemberAccess = Boolean.parseBoolean(disallowProxyMemberAccess);
     }
 
+    /**
+     * @param maxLength Injects the Struts OGNL maximum expression length.
+     */
+    @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH, required = false)
+    protected void applyExpressionMaxLength(String maxLength) {
+        if (maxLength == null || maxLength.isEmpty()) {
+            // user is going to disable this functionality
+            Ognl.applyExpressionMaxLength(null);
+        } else {
+            Ognl.applyExpressionMaxLength(Integer.parseInt(maxLength));
+        }
+    }
+
     public boolean isDisallowProxyMemberAccess() {
         return disallowProxyMemberAccess;
     }
@@ -755,6 +768,9 @@ public class OgnlUtil {
             setValue(name, context, o, value);
         } catch (OgnlException e) {
             Throwable reason = e.getReason();
+            if (reason instanceof SecurityException) {
+                LOG.warn("Could not evaluate this expression due to security constraints: [{}]", name, e);
+            }
             String msg = "Caught OgnlException while setting property '" + name + "' on type '" + o.getClass().getName() + "'.";
             Throwable exception = (reason == null) ? e : reason;
 

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java

@@ -204,6 +204,9 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
     }
 
     protected void handleOgnlException(String expr, Object value, boolean throwExceptionOnFailure, OgnlException e) {
+        if (e.getReason() instanceof SecurityException) {
+            LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+        }
     	boolean shouldLog = shouldLogMissingPropertyWarning(e);
     	String msg = null;
     	if (throwExceptionOnFailure || shouldLog) {
@@ -326,7 +329,12 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
     }
 
     protected Object handleOgnlException(String expr, boolean throwExceptionOnFailure, OgnlException e) {
-        Object ret = findInContext(expr);
+        Object ret = null;
+        if (e.getReason() instanceof SecurityException) {
+            LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+        } else {
+            ret = findInContext(expr);
+        }
         if (ret == null) {
             if (shouldLogMissingPropertyWarning(e)) {
                 LOG.warn("Could not find property [{}]!", expr, e);

core/src/main/java/org/apache/struts2/StrutsConstants.java

@@ -261,6 +261,9 @@ public final class StrutsConstants {
     /** Enables evaluation of OGNL expressions */
     public static final String STRUTS_ENABLE_OGNL_EVAL_EXPRESSION = "struts.ognl.enableOGNLEvalExpression";
 
+    /** The maximum length of an expression (OGNL) */
+    public static final String STRUTS_OGNL_EXPRESSION_MAX_LENGTH = "struts.ognl.expressionMaxLength";
+
     /** Disables {@link org.apache.struts2.dispatcher.StrutsRequestWrapper} request attribute value stack lookup (JSTL accessibility) */
     public static final String STRUTS_DISABLE_REQUEST_ATTRIBUTE_VALUE_STACK_LOOKUP = "struts.disableRequestAttributeValueStackLookup";
 

core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java

@@ -23,12 +23,16 @@ import ognl.OgnlException;
 
 import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.ognl.OgnlUtil;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 /**
  * FIXME: remove?
  */
 public class OgnlTool {
 
+    private static final Logger LOG = LogManager.getLogger(OgnlTool.class);
+
     private OgnlUtil ognlUtil;
     
     public OgnlTool() {
@@ -43,6 +47,9 @@ public class OgnlTool {
         try {
             return Ognl.getValue(ognlUtil.compile(expr), context);
         } catch (OgnlException e) {
+            if (e.getReason() instanceof SecurityException) {
+                LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+            }
             return null;
         }
     }

core/src/main/resources/org/apache/struts2/default.properties

@@ -219,4 +219,8 @@ struts.ognl.enableExpressionCache=true
 ### Indicates if Dispatcher should handle unexpected exceptions by calling sendError()
 ### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security
 struts.handle.exception=true
+
+### applies maximum length allowed on OGNL expressions for security enhancement
+struts.ognl.expressionMaxLength=200
+
 ### END SNIPPET: complete_file

core/src/main/resources/struts-default.xml

@@ -55,13 +55,18 @@
     <constant name="struts.excludedPackageNames"
               value="
                 ognl.,
+                java.io.,
                 javax.,
                 freemarker.core.,
                 freemarker.template.,
+                freemarker.ext.jsp.,
                 freemarker.ext.rhino.,
                 sun.reflect.,
                 javassist.,
+                org.apache.velocity.,
                 org.objectweb.asm.,
+                org.springframework.context.,
+                com.opensymphony.xwork2.inject.,
                 com.opensymphony.xwork2.ognl.,
                 com.opensymphony.xwork2.security.,
                 com.opensymphony.xwork2.util." />

core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java

@@ -30,6 +30,7 @@ import com.opensymphony.xwork2.util.*;
 import com.opensymphony.xwork2.util.Foo;
 import com.opensymphony.xwork2.util.location.LocatableProperties;
 import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
+import ognl.OgnlException;
 import ognl.PropertyAccessor;
 
 import java.io.*;
@@ -40,11 +41,13 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.Logger;
 import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.config.DefaultPropertiesProvider;
 
 
 /**
@@ -347,6 +350,38 @@ public class OgnlValueStackTest extends XWorkTestCase {
         }
     }
 
+    public void testFailOnTooLongExpressionWithDefaultProperties() {
+        loadConfigurationProviders(new DefaultPropertiesProvider());
+        Integer repeat = Integer.parseInt(
+                container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
+
+        OgnlValueStack vs = createValueStack();
+        try {
+            vs.findValue(StringUtils.repeat('.', repeat + 1), true);
+            fail("Failed to throw exception on too long expression");
+        } catch (Exception ex) {
+            assertTrue(ex.getCause() instanceof OgnlException);
+            assertTrue(((OgnlException) ex.getCause()).getReason() instanceof SecurityException);
+        }
+    }
+
+    public void testNotFailOnTooLongValueWithDefaultProperties() {
+        loadConfigurationProviders(new DefaultPropertiesProvider());
+        Integer repeat = Integer.parseInt(
+                container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
+
+        OgnlValueStack vs = createValueStack();
+
+        Dog dog = new Dog();
+        vs.push(dog);
+
+        String value = StringUtils.repeat('.', repeat + 1);
+
+        vs.setValue("name", value);
+
+        assertEquals(value, dog.getName());
+    }
+
     public void testFailsOnMethodThatThrowsException() {
         SimpleAction action = new SimpleAction();
         OgnlValueStack stack = createValueStack();

core/src/test/java/org/apache/struts2/result/StreamResultTest.java

@@ -246,12 +246,19 @@ public class StreamResultTest extends StrutsInternalTestCase {
 
     public class MyImageAction implements Action {
 
-        public InputStream getStreamForImage() throws Exception {
+        FileInputStream streamForImage;
+        long contentLength;
+
+        public MyImageAction() throws Exception {
             // just use src/test/log4j2.xml as test file
             URL url = ClassLoaderUtil.getResource("log4j2.xml", StreamResultTest.class);
             File file = new File(new URI(url.toString()));
-            FileInputStream fis = new FileInputStream(file);
+            streamForImage = new FileInputStream(file);
-            return fis;
+            contentLength = file.length();
+        }
+
+        public InputStream getStreamForImage() throws Exception {
+            return streamForImage;
         }
 
         public String execute() throws Exception {
@@ -259,9 +266,7 @@ public class StreamResultTest extends StrutsInternalTestCase {
         }
 
         public long getContentLength() throws Exception {
-            URL url = ClassLoaderUtil.getResource("log4j2.xml", StreamResultTest.class);
+            return contentLength;
-            File file = new File(new URI(url.toString()));
-            return file.length();
         }
 
         public String getStreamForImageAsString() {

pom.xml

@@ -98,7 +98,7 @@
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 
         <spring.platformVersion>4.3.24.RELEASE</spring.platformVersion>
-        <ognl.version>3.1.23</ognl.version>
+        <ognl.version>3.1.26</ognl.version>
         <asm.version>7.1</asm.version>
         <tiles.version>3.0.8</tiles.version>
         <tiles-request.version>1.0.7</tiles-request.version>