From e2b644a4fbe31f0898008c2938069ef04f0f9124 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sat, 2 Nov 2019 13:31:09 -0400 Subject: [PATCH 1/2] Minor follow-up changes to PR #371 - added some additional exclusions in struts-default.xml. - added log warning that specifies the value of maxLength involved if applyExpressionMaxLength(maxLength) fails. - added null guards to two handleOgnlException() methods that could result in an NPE with #371 changes (a null OgnlException parameter was permissible previously, correct or not). --- .../com/opensymphony/xwork2/ognl/OgnlUtil.java | 15 ++++++++++----- .../opensymphony/xwork2/ognl/OgnlValueStack.java | 4 ++-- core/src/main/resources/struts-default.xml | 4 ++++ 3 files changed, 16 insertions(+), 7 deletions(-) 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 8bd8b5631..bc53c758e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -189,11 +189,16 @@ public class OgnlUtil { */ @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)); + try { + if (maxLength == null || maxLength.isEmpty()) { + // user is going to disable this functionality + Ognl.applyExpressionMaxLength(null); + } else { + Ognl.applyExpressionMaxLength(Integer.parseInt(maxLength)); + } + } catch (Exception ex) { + LOG.warn("Unable to set OGNL Expression Max Length {}.", maxLength); // Help configuration debugging. + throw ex; } } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index bb7b4cb14..93f82425e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java @@ -204,7 +204,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS } protected void handleOgnlException(String expr, Object value, boolean throwExceptionOnFailure, OgnlException e) { - if (e.getReason() instanceof SecurityException) { + if (e != null && e.getReason() instanceof SecurityException) { LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e); } boolean shouldLog = shouldLogMissingPropertyWarning(e); @@ -330,7 +330,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS protected Object handleOgnlException(String expr, boolean throwExceptionOnFailure, OgnlException e) { Object ret = null; - if (e.getReason() instanceof SecurityException) { + if (e != null && e.getReason() instanceof SecurityException) { LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e); } else { ret = findInContext(expr); diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index f8f8bd0f8..0206d6c01 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -45,6 +45,7 @@ java.lang.ClassLoader, java.lang.Shutdown, java.lang.ProcessBuilder, + sun.misc.Unsafe, com.opensymphony.xwork2.ActionContext" /> @@ -56,11 +57,14 @@ value=" ognl., java.io., + java.net., + java.nio., javax., freemarker.core., freemarker.template., freemarker.ext.jsp., freemarker.ext.rhino., + sun.misc., sun.reflect., javassist., org.apache.velocity., From dd6d206d7801c8e0e5edf73fa4f38086a3f0e782 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sat, 2 Nov 2019 14:11:21 -0400 Subject: [PATCH 2/2] Additional change - added unit test (hoping to make coveralls happy). --- .../xwork2/ognl/OgnlUtilTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) 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 5f1a5a5f1..997955306 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1232,6 +1232,40 @@ public class OgnlUtilTest extends XWorkTestCase { } } + /** + * Test OGNL Expression Max Length feature setting via OgnlUtil. + * + * @since 2.5.21 + */ + public void testApplyExpressionMaxLength() { + try { + ognlUtil.applyExpressionMaxLength(null); + } catch (Exception ex) { + fail ("applyExpressionMaxLength did not accept null maxlength string ?"); + } + try { + ognlUtil.applyExpressionMaxLength(""); + } catch (Exception ex) { + fail ("applyExpressionMaxLength did not accept empty maxlength string ?"); + } + try { + ognlUtil.applyExpressionMaxLength("-1"); + fail ("applyExpressionMaxLength accepted negative maxlength string ?"); + } catch (IllegalArgumentException iae) { + // Expected rejection of -ive length. + } + try { + ognlUtil.applyExpressionMaxLength("0"); + } catch (Exception ex) { + fail ("applyExpressionMaxLength did not accept maxlength string 0 ?"); + } + try { + ognlUtil.applyExpressionMaxLength(Integer.toString(Integer.MAX_VALUE, 10)); + } catch (Exception ex) { + fail ("applyExpressionMaxLength did not accept MAX_VALUE maxlength string ?"); + } + } + private void internalTestInitialEmptyOgnlUtilExclusions(OgnlUtil ognlUtilParam) throws Exception { Set> excludedClasses = ognlUtilParam.getExcludedClasses(); assertNotNull("parameter (default) exluded classes null?", excludedClasses);