Return-Path: X-Original-To: apmail-struts-commits-archive@minotaur.apache.org Delivered-To: apmail-struts-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D3F5110FFD for ; Sun, 6 Apr 2014 14:04:21 +0000 (UTC) Received: (qmail 92111 invoked by uid 500); 6 Apr 2014 14:03:38 -0000 Delivered-To: apmail-struts-commits-archive@struts.apache.org Received: (qmail 91823 invoked by uid 500); 6 Apr 2014 14:03:31 -0000 Mailing-List: contact commits-help@struts.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@struts.apache.org Delivered-To: mailing list commits@struts.apache.org Received: (qmail 90948 invoked by uid 99); 6 Apr 2014 14:03:17 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 06 Apr 2014 14:03:17 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 1356C94B9D3; Sun, 6 Apr 2014 14:03:15 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: lukaszlenart@apache.org To: commits@struts.apache.org Date: Sun, 06 Apr 2014 14:03:37 -0000 Message-Id: <0ee2361084094fa5801b57a26d5c5486@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [24/50] [abbrv] git commit: WW-4146 Caches only valid Ognl expressions to avoid cache attack WW-4146 Caches only valid Ognl expressions to avoid cache attack Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/86813c1a Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/86813c1a Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/86813c1a Branch: refs/heads/master Commit: 86813c1a7214bc002a5d7ce9981a9ef333e27142 Parents: 5fb27ba Author: Lukasz Lenart Authored: Thu Mar 27 19:07:11 2014 +0100 Committer: Lukasz Lenart Committed: Thu Mar 27 19:07:11 2014 +0100 ---------------------------------------------------------------------- .../com/opensymphony/xwork2/ognl/OgnlUtil.java | 85 ++++++++++++++------ .../ognl/accessor/CompoundRootAccessor.java | 27 ++++--- 2 files changed, 76 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/86813c1a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java ---------------------------------------------------------------------- diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 3e622d5..fa907e3 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -226,12 +226,16 @@ public class OgnlUtil { setValue(name, context, root, value, true); } - protected void setValue(String name, Map context, Object root, Object value, boolean evalName) throws OgnlException { - Object tree = compile(name, context); - if (!evalName && isEvalExpression(tree, context)) { - throw new OgnlException("Eval expression cannot be used as parameter name"); - } - Ognl.setValue(tree, context, root, value); + protected void setValue(String name, final Map context, final Object root, final Object value, final boolean evalName) throws OgnlException { + compileAndExecute(name, context, new OgnlTask() { + public Void execute(Object tree) throws OgnlException { + if (!evalName && isEvalExpression(tree, context)) { + throw new OgnlException("Eval expression cannot be used as parameter name"); + } + Ognl.setValue(tree, context, root, value); + return null; + } + }); } private boolean isEvalExpression(Object tree, Map context) throws OgnlException { @@ -247,12 +251,20 @@ public class OgnlUtil { return false; } - public Object getValue(String name, Map context, Object root) throws OgnlException { - return Ognl.getValue(compile(name, context), context, root); + public Object getValue(final String name, final Map context, final Object root) throws OgnlException { + return compileAndExecute(name, context, new OgnlTask() { + public Object execute(Object tree) throws OgnlException { + return Ognl.getValue(tree, context, root); + } + }); } - public Object getValue(String name, Map context, Object root, Class resultType) throws OgnlException { - return Ognl.getValue(compile(name, context), context, root, resultType); + public Object getValue(final String name, final Map context, final Object root, final Class resultType) throws OgnlException { + return compileAndExecute(name, context, new OgnlTask() { + public Object execute(Object tree) throws OgnlException { + return Ognl.getValue(tree, context, root, resultType); + } + }); } @@ -260,7 +272,7 @@ public class OgnlUtil { return compile(expression, null); } - public Object compile(String expression, Map context) throws OgnlException { + private Object compileAndExecute(String expression, Map context, OgnlTask task) throws OgnlException { Object tree; if (enableExpressionCache) { tree = expressions.get(expression); @@ -274,7 +286,19 @@ public class OgnlUtil { checkEnableEvalExpression(tree, context); } - return tree; + + final T exec = task.execute(tree); + if(enableExpressionCache) + expressions.putIfAbsent(expression, tree); + return exec; + } + + public Object compile(String expression, Map context) throws OgnlException { + return compileAndExecute(expression,context,new OgnlTask() { + public Object execute(Object tree) throws OgnlException { + return tree; + } + }); } private void checkEnableEvalExpression(Object tree, Map context) throws OgnlException { @@ -295,7 +319,7 @@ public class OgnlUtil { * @param inclusions collection of method names to included copying (can be null) * note if exclusions AND inclusions are supplied and not null nothing will get copied. */ - public void copy(Object from, Object to, Map context, Collection exclusions, Collection inclusions) { + public void copy(final Object from, final Object to, final Map context, Collection exclusions, Collection inclusions) { if (from == null || to == null) { if (LOG.isWarnEnabled()) { LOG.warn("Attempting to copy from or to a null source. This is illegal and is bein skipped. This may be due to an error in an OGNL expression, action chaining, or some other event."); @@ -305,9 +329,9 @@ public class OgnlUtil { } TypeConverter conv = getTypeConverterFromContext(context); - Map contextFrom = Ognl.createDefaultContext(from); + final Map contextFrom = Ognl.createDefaultContext(from); Ognl.setTypeConverter(contextFrom, conv); - Map contextTo = Ognl.createDefaultContext(to); + final Map contextTo = Ognl.createDefaultContext(to); Ognl.setTypeConverter(contextTo, conv); PropertyDescriptor[] fromPds; @@ -342,9 +366,14 @@ public class OgnlUtil { PropertyDescriptor toPd = toPdHash.get(fromPd.getName()); if ((toPd != null) && (toPd.getWriteMethod() != null)) { try { - Object expr = compile(fromPd.getName(), context); - Object value = Ognl.getValue(expr, contextFrom, from); - Ognl.setValue(expr, contextTo, to, value); + compileAndExecute(fromPd.getName(), context, new OgnlTask() { + public Void execute(Object expr) throws OgnlException { + Object value = Ognl.getValue(expr, contextFrom, from); + Ognl.setValue(expr, contextTo, to, value); + return null; + } + }); + } catch (OgnlException e) { if (LOG.isDebugEnabled()) { LOG.debug("Got OGNL exception", e); @@ -409,16 +438,19 @@ public class OgnlUtil { * @throws IntrospectionException is thrown if an exception occurs during introspection. * @throws OgnlException is thrown by OGNL if the property value could not be retrieved */ - public Map getBeanMap(Object source) throws IntrospectionException, OgnlException { - Map beanMap = new HashMap(); - Map sourceMap = Ognl.createDefaultContext(source); + public Map getBeanMap(final Object source) throws IntrospectionException, OgnlException { + Map beanMap = new HashMap(); + final Map sourceMap = Ognl.createDefaultContext(source); PropertyDescriptor[] propertyDescriptors = getPropertyDescriptors(source); for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { - String propertyName = propertyDescriptor.getDisplayName(); + final String propertyName = propertyDescriptor.getDisplayName(); Method readMethod = propertyDescriptor.getReadMethod(); if (readMethod != null) { - Object expr = compile(propertyName); - Object value = Ognl.getValue(expr, sourceMap, source); + final Object value = compileAndExecute(propertyName, null, new OgnlTask() { + public Object execute(Object expr) throws OgnlException { + return Ognl.getValue(expr, sourceMap, source); + } + }); beanMap.put(propertyName, value); } else { beanMap.put(propertyName, "There is no read method for " + propertyName); @@ -485,4 +517,9 @@ public class OgnlUtil { */ return defaultConverter; } + + private interface OgnlTask { + T execute(Object tree) throws OgnlException; + } + } http://git-wip-us.apache.org/repos/asf/struts/blob/86813c1a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java ---------------------------------------------------------------------- diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java index df038b0..58942fd 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java @@ -17,8 +17,8 @@ package com.opensymphony.xwork2.ognl.accessor; import com.opensymphony.xwork2.XWorkConstants; import com.opensymphony.xwork2.XWorkException; -import com.opensymphony.xwork2.ognl.OgnlValueStack; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.ognl.OgnlValueStack; import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.logging.Logger; @@ -29,6 +29,8 @@ import java.beans.IntrospectionException; import java.beans.PropertyDescriptor; import java.util.*; +import static java.lang.String.format; +import static org.apache.commons.lang3.BooleanUtils.toBoolean; /** * A stack that is able to call methods on objects in the stack. @@ -55,7 +57,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C private final static Logger LOG = LoggerFactory.getLogger(CompoundRootAccessor.class); private final static Class[] EMPTY_CLASS_ARRAY = new Class[0]; - private static Map invalidMethods = new HashMap(); + private static Map invalidMethods = new HashMap(); static boolean devMode = false; @@ -79,7 +81,8 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C return; } else if (o instanceof Map) { - Map map = (Map) o; + @SuppressWarnings("unchecked") + Map map = (Map) o; try { map.put(name, value); return; @@ -98,14 +101,14 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C } } - Boolean reportError = (Boolean) context.get(ValueStack.REPORT_ERRORS_ON_NO_PROP); - - final String msg = "No object in the CompoundRoot has a publicly accessible property named '" + name + "' (no setter could be found)."; + boolean reportError = toBoolean((Boolean) context.get(ValueStack.REPORT_ERRORS_ON_NO_PROP)); - if ((reportError != null) && (reportError.booleanValue())) { - throw new XWorkException(msg); - } else { - if (devMode) { + if (reportError || devMode) { + final String msg = format("No object in the CompoundRoot has a publicly accessible property named '%s' " + + "(no setter could be found).", name); + if (reportError) { + throw new XWorkException(msg); + } else { LOG.warn(msg); } } @@ -118,7 +121,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C if (name instanceof Integer) { Integer index = (Integer) name; - return root.cutStack(index.intValue()); + return root.cutStack(index); } else if (name instanceof String) { if ("top".equals(name)) { if (root.size() > 0) { @@ -234,7 +237,7 @@ public class CompoundRootAccessor implements PropertyAccessor, MethodAccessor, C if ((argTypes == null) || !invalidMethods.containsKey(mc)) { try { - Object value = OgnlRuntime.callMethod((OgnlContext) context, o, name, name, objects); + Object value = OgnlRuntime.callMethod((OgnlContext) context, o, name, objects); if (value != null) { return value;