Compare commits

..

4 Commits

Author SHA1 Message Date
Lukasz Lenart a6edb0d5df [maven-release-plugin] prepare release STRUTS_2_5_22 2019-11-17 20:17:20 +01:00
JCgH4164838Gh792C124B5 3dfc5a4074 Disable expressionMaxLength by default for Struts 2.5.x. (#380)
* Disable struts.ognl.expressionMaxLength by default for Struts 2.5.x.
- Commented out struts.ognl.expressionMaxLength line in default.properties
and provided in-place comments about its usage.
- Changed OgnlValueStack.handleOgnlException() methods to output error
instead of warn for failures to evaluate expressions due to security
constraints.
- Updated existing unit tests to compensate for change in default
behaviour.
- Added a unit test to confirm default behaviour for
struts.ognl.expressionMaxLength is disabled.

* Updated commit for disable struts.ognl.expressionMaxLength by default for
Struts 2.5.x
- Additional unit test requested by Y. Zamani for code coverage.
- Corrected accidental use of wrong (static) toString method in one test.
- Addition of a minimum struts.ognl.expressionMaxLength value permitted
by Struts 2 (128).  Any value smaller than that is likely to be a
configuration error and if a user really wishes to force it they may go to
OGNL directly to do so.

* Updated commit for disable struts.ognl.expressionMaxLength by default for
Struts 2.5.x
- Removed minimum struts.ognl.expressionMaxLength (restored to previous
behaviour) as requested by Y. Zamani and L. Lenart.
- Updated unit tests to compensate for the above change.
- Changed log output from warn to error in applyExpressionMaxLength() on
exception since it will likely be considered a fatal condition.
2019-11-16 20:09:19 +03:30
Lukasz Lenart fdfeb3233a Uses the latest struts-master 2019-11-07 20:41:58 +01:00
Lukasz Lenart ae3ae2be76 [maven-release-plugin] prepare for next development iteration 2019-11-07 20:18:06 +01:00
40 changed files with 174 additions and 82 deletions
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-apps</artifactId>
<packaging>pom</packaging>
+2 -2
View File
@@ -24,12 +24,12 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-apps</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-rest-showcase</artifactId>
<packaging>war</packaging>
<version>2.5.21</version>
<version>2.5.22</version>
<name>Struts 2 Rest Showcase Webapp</name>
<description>Struts 2 Rest Showcase Example</description>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-apps</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-showcase</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-assembly</artifactId>
+3 -3
View File
@@ -30,7 +30,7 @@
</parent>
<artifactId>struts2-bom</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
<packaging>pom</packaging>
<name>Struts 2 Bill of Materials</name>
@@ -45,7 +45,7 @@
</licenses>
<properties>
<struts-version.version>2.5.21</struts-version.version>
<struts-version.version>2.5.22</struts-version.version>
<maven.site.skip>true</maven.site.skip>
<maven.site.deploy.skip>true</maven.site.deploy.skip>
</properties>
@@ -181,6 +181,6 @@
</dependencyManagement>
<scm>
<tag>STRUTS_2_5_21</tag>
<tag>STRUTS_2_5_22</tag>
</scm>
</project>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-osgi-bundles</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-osgi-admin-bundle</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-osgi-bundles</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-osgi-demo-bundle</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-osgi-bundles</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-core</artifactId>
<packaging>jar</packaging>
@@ -197,7 +197,7 @@ public class OgnlUtil {
Ognl.applyExpressionMaxLength(Integer.parseInt(maxLength));
}
} catch (Exception ex) {
LOG.warn("Unable to set OGNL Expression Max Length {}.", maxLength); // Help configuration debugging.
LOG.error("Unable to set OGNL Expression Max Length {}.", maxLength); // Help configuration debugging.
throw ex;
}
}
@@ -205,7 +205,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
protected void handleOgnlException(String expr, Object value, boolean throwExceptionOnFailure, OgnlException e) {
if (e != null && e.getReason() instanceof SecurityException) {
LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
LOG.error("Could not evaluate this expression due to security constraints: [{}]", expr, e);
}
boolean shouldLog = shouldLogMissingPropertyWarning(e);
String msg = null;
@@ -331,7 +331,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
protected Object handleOgnlException(String expr, boolean throwExceptionOnFailure, OgnlException e) {
Object ret = null;
if (e != null && e.getReason() instanceof SecurityException) {
LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
LOG.error("Could not evaluate this expression due to security constraints: [{}]", expr, e);
} else {
ret = findInContext(expr);
}
@@ -220,7 +220,14 @@ struts.ognl.enableExpressionCache=true
### 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
### Applies maximum length allowed on OGNL expressions for security enhancement (optional)
###
### **WARNING**: If developers enable this option (by configuration) they should make sure that they understand the implications of setting
### struts.ognl.expressionMaxLength. They must choose a value large enough to permit ALL valid OGNL expressions used within the application.
### Values larger than the 200-400 range have diminishing security value (at which point it is really only a "style guard" for long OGNL
### expressions in an application. Setting a value of null or "" will also disable the feature.
###
### NOTE: The sample line below is *INTENTIONALLY* commented out, as this feature is disabled by default.
# struts.ognl.expressionMaxLength=256
### END SNIPPET: complete_file
@@ -1232,6 +1232,27 @@ public class OgnlUtilTest extends XWorkTestCase {
}
}
/**
* Test OGNL Expression Max Length feature setting via OgnlUtil is disabled by default (in default.properties).
*
* @since 2.5.21
*/
public void testDefaultExpressionMaxLengthDisabled() {
final String LONG_OGNL_EXPRESSION = "true == ThisIsAReallyLongOGNLExpressionOfRepeatedGarbageText." + new String(new char[65535]).replace('\0', 'A'); // Expression larger than 64KB.
try {
Object compileResult = ognlUtil.compile(LONG_OGNL_EXPRESSION);
assertNotNull("Long OGNL expression compilation produced a null result ?", compileResult);
} catch (OgnlException oex) {
if (oex.getReason() instanceof SecurityException) {
fail ("Unable to compile expression (unexpected). 'struts.ognl.expressionMaxLength' may have accidentally been enabled by default. Exception: " + oex);
} else {
fail ("Unable to compile expression (unexpected). Exception: " + oex);
}
} catch (Exception ex) {
fail ("Unable to compile expression (unexpected). Exception: " + ex);
}
}
/**
* Test OGNL Expression Max Length feature setting via OgnlUtil.
*
@@ -1239,30 +1260,35 @@ public class OgnlUtilTest extends XWorkTestCase {
*/
public void testApplyExpressionMaxLength() {
try {
try {
ognlUtil.applyExpressionMaxLength(null);
} catch (Exception ex) {
fail ("applyExpressionMaxLength did not accept null maxlength string (disable feature) ?");
}
try {
ognlUtil.applyExpressionMaxLength("");
} catch (Exception ex) {
fail ("applyExpressionMaxLength did not accept empty maxlength string (disable feature) ?");
}
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 ?");
}
} finally {
// Reset expressionMaxLength value to default (disabled)
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 ?");
}
}
@@ -40,6 +40,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import ognl.ParseException;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
@@ -350,36 +351,94 @@ public class OgnlValueStackTest extends XWorkTestCase {
}
}
public void testFailOnTooLongExpressionWithDefaultProperties() {
public void testFailOnTooLongExpressionLongerThan192_ViaOverriddenProperty() {
try {
loadConfigurationProviders(new StubConfigurationProvider() {
@Override
public void register(ContainerBuilder builder,
LocatableProperties props) throws ConfigurationException {
props.setProperty(StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH, "192");
}
});
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);
}
} finally {
// Reset expressionMaxLength value to default (disabled)
ognlUtil.applyExpressionMaxLength(null);
}
}
public void testNotFailOnTooLongExpressionWithDefaultProperties() {
loadConfigurationProviders(new DefaultPropertiesProvider());
Integer repeat = Integer.parseInt(
container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
Object defaultMaxLengthFromConfiguration = container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH);
if (defaultMaxLengthFromConfiguration != null) {
assertTrue("non-null defaultMaxLengthFromConfiguration not a String ?", defaultMaxLengthFromConfiguration instanceof String);
assertTrue("non-null defaultMaxLengthFromConfiguration not empty string by default ?", ((String) defaultMaxLengthFromConfiguration).length() == 0);
} else {
assertNull("defaultMaxLengthFromConfiguration not null ?", defaultMaxLengthFromConfiguration);
}
// Original test logic was to confirm failure of exceeding the default value. Now the feature should be disabled by default,
// so this test's expectations are now changed.
Integer repeat = Integer.valueOf(256); // Since maxlength is disabled by default, just choose an arbitrary value for test
OgnlValueStack vs = createValueStack();
try {
vs.findValue(StringUtils.repeat('.', repeat + 1), true);
fail("Failed to throw exception on too long expression");
fail("findValue did not throw any exception (should either fail as invalid expression syntax or security exception) ?");
} catch (Exception ex) {
// If STRUTS_OGNL_EXPRESSION_MAX_LENGTH feature is disabled (default), the parse should fail due to a reason of invalid expression syntax
// with ParseException. Previously when it was enabled the reason for the failure would have been SecurityException.
assertTrue(ex.getCause() instanceof OgnlException);
assertTrue(((OgnlException) ex.getCause()).getReason() instanceof SecurityException);
assertTrue(((OgnlException) ex.getCause()).getReason() instanceof ParseException);
}
}
public void testNotFailOnTooLongValueWithDefaultProperties() {
loadConfigurationProviders(new DefaultPropertiesProvider());
Integer repeat = Integer.parseInt(
container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
try {
loadConfigurationProviders(new DefaultPropertiesProvider());
OgnlValueStack vs = createValueStack();
Object defaultMaxLengthFromConfiguration = container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH);
if (defaultMaxLengthFromConfiguration != null) {
assertTrue("non-null defaultMaxLengthFromConfiguration not a String ?", defaultMaxLengthFromConfiguration instanceof String);
assertTrue("non-null defaultMaxLengthFromConfiguration not empty string by default ?", ((String) defaultMaxLengthFromConfiguration).length() == 0);
} else {
assertNull("defaultMaxLengthFromConfiguration not null ?", defaultMaxLengthFromConfiguration);
}
// Original test logic is unchanged (testing that values can be larger than maximum expression length), but since the feature is disabled by
// default we will now have to enable it with an arbitrary value, test, and reset it to disabled.
Integer repeat = Integer.valueOf(256); // Since maxlength is disabled by default, just choose an arbitrary value for test
Dog dog = new Dog();
vs.push(dog);
// Apply a non-default value for expressionMaxLength (as it should be disabled by default)
try {
ognlUtil.applyExpressionMaxLength(repeat.toString());
} catch (Exception ex) {
fail ("applyExpressionMaxLength did not accept maxlength string " + repeat.toString() + " ?");
}
String value = StringUtils.repeat('.', repeat + 1);
OgnlValueStack vs = createValueStack();
vs.setValue("name", value);
Dog dog = new Dog();
vs.push(dog);
assertEquals(value, dog.getName());
String value = StringUtils.repeat('.', repeat + 1);
vs.setValue("name", value);
assertEquals(value, dog.getName());
} finally {
// Reset expressionMaxLength value to default (disabled)
ognlUtil.applyExpressionMaxLength(null);
}
}
public void testFailsOnMethodThatThrowsException() {
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<modelVersion>4.0.0</modelVersion>
+1 -1
View File
@@ -25,7 +25,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-cdi-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-config-browser-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-convention-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-dwr-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-embeddedjsp-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-gxp-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-jasperreports-plugin</artifactId>
+1 -1
View File
@@ -25,7 +25,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-javatemplates-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-jfreechart-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-json-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-junit-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-osgi-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-oval-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-pell-multipart-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-plexus-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-plugins</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-portlet-tiles-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-portlet-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-rest-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-sitegraph-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-sitemesh-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-spring-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-testng-plugin</artifactId>
+1 -1
View File
@@ -24,7 +24,7 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts2-plugins</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
</parent>
<artifactId>struts2-tiles-plugin</artifactId>
+3 -3
View File
@@ -24,12 +24,12 @@
<parent>
<groupId>org.apache.struts</groupId>
<artifactId>struts-master</artifactId>
<version>12</version>
<version>13</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>struts2-parent</artifactId>
<version>2.5.21</version>
<version>2.5.22</version>
<packaging>pom</packaging>
<name>Struts 2</name>
<url>http://struts.apache.org/</url>
@@ -51,7 +51,7 @@
<connection>scm:git:https://gitbox.apache.org/repos/asf/struts.git</connection>
<developerConnection>scm:git:https://gitbox.apache.org/repos/asf/struts.git</developerConnection>
<url>https://github.com/apache/struts/</url>
<tag>STRUTS_2_5_21</tag>
<tag>STRUTS_2_5_22</tag>
</scm>
<issueManagement>