This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5343-sec-extend
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/WW-5343-sec-extend by this
push:
new 85d2c742c WW-5343 Clean up bootstrap constants
85d2c742c is described below
commit 85d2c742cda110182a5e864751926ff0302124a9
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Sun Nov 26 16:52:07 2023 +1100
WW-5343 Clean up bootstrap constants
---
.../xwork2/config/impl/DefaultConfiguration.java | 5 +---
.../xwork2/config/impl/MockConfiguration.java | 2 --
.../StrutsDefaultConfigurationProvider.java | 4 ---
.../com/opensymphony/xwork2/ognl/OgnlUtil.java | 4 +--
.../xwork2/ognl/SecurityMemberAccess.java | 31 +++++++++++++---------
.../org/apache/struts2/default.properties | 7 -----
.../xwork2/ognl/OgnlValueStackTest.java | 23 ++++++++--------
.../xwork2/ognl/SecurityMemberAccessTest.java | 6 ++++-
8 files changed, 37 insertions(+), 45 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
index d0cbcef1c..3715c3bae 100644
---
a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
+++
b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
@@ -133,16 +133,13 @@ public class DefaultConfiguration implements
Configuration {
static {
Map<String, Object> constants = new HashMap<>();
constants.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE);
- constants.put(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES,
Boolean.FALSE);
- constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE,
Boolean.TRUE);
constants.put(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD,
Boolean.FALSE);
- constants.put(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE);
constants.put(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS,
Boolean.TRUE);
constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE,
OgnlCacheFactory.CacheType.BASIC);
constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE,
10000);
constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE,
OgnlCacheFactory.CacheType.BASIC);
constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE,
10000);
- constants.put(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS,
Boolean.TRUE);
+ constants.put(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
Boolean.FALSE);
BOOTSTRAP_CONSTANTS = Collections.unmodifiableMap(constants);
}
diff --git
a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java
b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java
index d245ccf4b..30eee9566 100644
---
a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java
+++
b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java
@@ -30,7 +30,6 @@ import com.opensymphony.xwork2.inject.Container;
import com.opensymphony.xwork2.inject.ContainerBuilder;
import com.opensymphony.xwork2.inject.Scope;
import com.opensymphony.xwork2.util.location.LocatableProperties;
-import org.apache.struts2.StrutsConstants;
import java.util.HashMap;
import java.util.HashSet;
@@ -62,7 +61,6 @@ public class MockConfiguration implements Configuration {
for (Map.Entry<String, Object> entry :
DefaultConfiguration.BOOTSTRAP_CONSTANTS.entrySet()) {
builder.constant(entry.getKey(), String.valueOf(entry.getValue()));
}
-
builder.constant(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
"false");
container = builder.create(true);
}
diff --git
a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
index 09eeb7c85..af2eff4d8 100644
---
a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
+++
b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
@@ -114,7 +114,6 @@ import com.opensymphony.xwork2.validator.ValidatorFactory;
import com.opensymphony.xwork2.validator.ValidatorFileParser;
import ognl.MethodAccessor;
import ognl.PropertyAccessor;
-import org.apache.struts2.StrutsConstants;
import org.apache.struts2.conversion.StrutsConversionPropertiesProcessor;
import org.apache.struts2.conversion.StrutsTypeConverterCreator;
import org.apache.struts2.conversion.StrutsTypeConverterHolder;
@@ -257,8 +256,5 @@ public class StrutsDefaultConfigurationProvider implements
ConfigurationProvider
for (Map.Entry<String, Object> entry :
DefaultConfiguration.BOOTSTRAP_CONSTANTS.entrySet()) {
props.setProperty(entry.getKey(),
String.valueOf(entry.getValue()));
}
-
- props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS,
Boolean.TRUE.toString());
-
props.setProperty(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
Boolean.FALSE.toString());
}
}
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 62e635fbc..de81c2685 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -77,7 +77,7 @@ public class OgnlUtil {
private final OgnlGuard ognlGuard;
private boolean devMode;
- private boolean enableExpressionCache;
+ private boolean enableExpressionCache = true;
private boolean enableEvalExpression;
private String devModeExcludedClasses = "";
@@ -126,7 +126,7 @@ public class OgnlUtil {
this.devMode = BooleanUtils.toBoolean(mode);
}
- @Inject(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE)
+ @Inject(value = StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE,
required = false)
protected void setEnableExpressionCache(String cache) {
enableExpressionCache = BooleanUtils.toBoolean(cache);
}
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
index c68423014..a5d2aa0b4 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -46,6 +46,7 @@ import static
com.opensymphony.xwork2.util.ConfigParseUtil.toNewPatternsSet;
import static com.opensymphony.xwork2.util.ConfigParseUtil.toPackageNamesSet;
import static java.text.MessageFormat.format;
import static java.util.Collections.emptySet;
+import static java.util.Collections.singletonList;
import static java.util.Collections.unmodifiableSet;
/**
@@ -56,10 +57,10 @@ public class SecurityMemberAccess implements MemberAccess {
private static final Logger LOG =
LogManager.getLogger(SecurityMemberAccess.class);
- private final boolean allowStaticFieldAccess;
+ private boolean allowStaticFieldAccess = true;
private Set<Pattern> excludeProperties = emptySet();
private Set<Pattern> acceptProperties = emptySet();
- private Set<String> excludedClasses = emptySet();
+ private Set<String> excludedClasses = unmodifiableSet(new
HashSet<>(singletonList(Object.class.getName())));
private Set<Pattern> excludedPackageNamePatterns = emptySet();
private Set<String> excludedPackageNames = emptySet();
private Set<String> excludedPackageExemptClasses = emptySet();
@@ -69,9 +70,7 @@ public class SecurityMemberAccess implements MemberAccess {
private boolean disallowProxyMemberAccess = false;
private boolean disallowDefaultPackageAccess = false;
- @Inject
- public SecurityMemberAccess(@Inject(value =
StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS) String
allowStaticFieldAccess) {
- this(BooleanUtils.toBoolean(allowStaticFieldAccess));
+ public SecurityMemberAccess() {
}
/**
@@ -80,10 +79,11 @@ public class SecurityMemberAccess implements MemberAccess {
* - block or allow access to properties (configurable-after-construction)
*
* @param allowStaticFieldAccess if set to true static fields (constants)
will be accessible
+ * @deprecated since 6.4.0, use {@link #SecurityMemberAccess()} instead.
*/
+ @Deprecated
public SecurityMemberAccess(boolean allowStaticFieldAccess) {
- this.allowStaticFieldAccess = allowStaticFieldAccess;
- useExcludedClasses(""); // Initialise default exclusions
+ useAllowStaticFieldAccess(String.valueOf(allowStaticFieldAccess));
}
@Override
@@ -376,20 +376,24 @@ public class SecurityMemberAccess implements MemberAccess
{
this.acceptProperties = acceptedProperties;
}
+ @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, required
= false)
+ public void useAllowStaticFieldAccess(String allowStaticFieldAccess) {
+ this.allowStaticFieldAccess =
BooleanUtils.toBoolean(allowStaticFieldAccess);
+ if (!this.allowStaticFieldAccess) {
+ useExcludedClasses(Class.class.getName());
+ }
+ }
+
@Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false)
public void useExcludedClasses(String commaDelimitedClasses) {
- Set<String> newExcludedClasses = new
HashSet<>(toNewClassesSet(excludedClasses, commaDelimitedClasses));
- newExcludedClasses.add(Object.class.getName());
- if (!allowStaticFieldAccess) {
- newExcludedClasses.add(Class.class.getName());
- }
- this.excludedClasses = unmodifiableSet(newExcludedClasses);
+ this.excludedClasses = toNewClassesSet(excludedClasses,
commaDelimitedClasses);
}
@Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS,
required = false)
public void useExcludedPackageNamePatterns(String
commaDelimitedPackagePatterns) {
this.excludedPackageNamePatterns =
toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns);
}
+
@Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required =
false)
public void useExcludedPackageNames(String commaDelimitedPackageNames) {
this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames,
commaDelimitedPackageNames);
@@ -399,6 +403,7 @@ public class SecurityMemberAccess implements MemberAccess {
public void useExcludedPackageExemptClasses(String commaDelimitedClasses) {
this.excludedPackageExemptClasses =
toClassesSet(commaDelimitedClasses);
}
+
@Inject(value = StrutsConstants.STRUTS_ALLOWLIST_ENABLE, required = false)
public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) {
this.enforceAllowlistEnabled =
BooleanUtils.toBoolean(enforceAllowlistEnabled);
diff --git a/core/src/main/resources/org/apache/struts2/default.properties
b/core/src/main/resources/org/apache/struts2/default.properties
index 6b3cb4dfe..96c5459fe 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -218,9 +218,6 @@ struts.mapper.alwaysSelectFullNamespace=false
### Whether to allow static field access in OGNL expressions or not
struts.ognl.allowStaticFieldAccess=true
-### Whether to allow static method access in OGNL expressions or not
-struts.ognl.allowStaticMethodAccess=false
-
### Whether to throw a RuntimeException when a property is not found
### in an expression, or when the expression evaluation fails
struts.el.throwExceptionOnFailure=false
@@ -228,10 +225,6 @@ struts.el.throwExceptionOnFailure=false
### Logs as Warnings properties that are not found (very verbose)
struts.ognl.logMissingProperties=false
-### Caches parsed OGNL expressions, but can lead to memory leaks
-### if the application generates a lot of different expressions
-struts.ognl.enableExpressionCache=true
-
### Specify the OGNL expression cache factory and BeanInfo cache factory to
use.
### Currently, the default implementations are used, but can be replaced with
custom ones if desired.
# struts.ognl.expressionCacheFactory=customOgnlExpressionCacheFactory
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
index 42cc4b110..8068dbb86 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -61,6 +61,8 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import static
com.opensymphony.xwork2.ognl.SecurityMemberAccessTest.reflectField;
+
public class OgnlValueStackTest extends XWorkTestCase {
// Fields for static field access test
@@ -1148,14 +1150,13 @@ public class OgnlValueStackTest extends XWorkTestCase {
* Test a default OgnlValueStackFactory and OgnlValueStack generated by it
* when a default configuration is used.
*/
- public void testOgnlValueStackFromOgnlValueStackFactoryDefaultConfig() {
+ public void testOgnlValueStackFromOgnlValueStackFactoryDefaultConfig()
throws IllegalAccessException {
OgnlValueStackFactory ognlValueStackFactory = getValueStackFactory();
OgnlValueStack ognlValueStack = (OgnlValueStack)
ognlValueStackFactory.createValueStack();
Object accessedValue;
- // An OgnlValueStackFactory using a container config with default
(from XWorkConfigurationProvider)
- // static access flag values present should prevent staticMethodAccess
but allow staticFieldAccess.
- assertTrue("OgnlValueStackFactory staticFieldAccess (default flags)
not true?", ognlValueStackFactory.containerAllowsStaticFieldAccess());
+ assertTrue("OgnlValueStackFactory staticFieldAccess (default flags)
not true?",
+ reflectField(ognlValueStack.securityMemberAccess,
"allowStaticFieldAccess"));
// An OgnlValueStack created from the above OgnlValueStackFactory
should allow public field access,
// but prevent non-public field access. It should also deny static
method access.
accessedValue =
ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()");
@@ -1182,14 +1183,13 @@ public class OgnlValueStackTest extends XWorkTestCase {
* Test a raw OgnlValueStackFactory and OgnlValueStack generated by it
* when static access flag is set to false.
*/
- public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() {
+ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess()
throws IllegalAccessException {
OgnlValueStackFactory ognlValueStackFactory =
reloadValueStackFactory(false);
OgnlValueStack ognlValueStack = (OgnlValueStack)
ognlValueStackFactory.createValueStack();
Object accessedValue;
- // An OgnlValueStackFactory using a container config with both static
access flags set false should
- // prevent staticMethodAccess AND prevent staticFieldAccess.
- assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not
false?", ognlValueStackFactory.containerAllowsStaticFieldAccess());
+ assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not
false?",
+ reflectField(ognlValueStack.securityMemberAccess,
"allowStaticFieldAccess"));
// An OgnlValueStack created from the above OgnlValueStackFactory
should prevent public field access,
// and prevent non-public field access. It should also deny static
method access.
accessedValue =
ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()");
@@ -1216,14 +1216,13 @@ public class OgnlValueStackTest extends XWorkTestCase {
* Test a raw OgnlValueStackFactory and OgnlValueStack generated by it
* when static access flag is set to true.
*/
- public void testOgnlValueStackFromOgnlValueStackFactoryAllStaticAccess() {
+ public void testOgnlValueStackFromOgnlValueStackFactoryAllStaticAccess()
throws IllegalAccessException {
OgnlValueStackFactory ognlValueStackFactory =
reloadValueStackFactory(true);
OgnlValueStack ognlValueStack = (OgnlValueStack)
ognlValueStackFactory.createValueStack();
Object accessedValue;
- // An OgnlValueStackFactory using a container config with both static
access flags set true should
- // allow both staticMethodAccess AND staticFieldAccess.
- assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not
true?", ognlValueStackFactory.containerAllowsStaticFieldAccess());
+ assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not
true?",
+ reflectField(ognlValueStack.securityMemberAccess,
"allowStaticFieldAccess"));
// An OgnlValueStack created from the above OgnlValueStackFactory
should allow public field access,
// but prevent non-public field access. It should also allow static
method access.
accessedValue =
ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()");
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
index ebbc797ee..7d3f04fc7 100644
---
a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -63,7 +63,11 @@ public class SecurityMemberAccessTest {
}
private <T> T reflectField(String fieldName) throws IllegalAccessException
{
- return (T) FieldUtils.readField(sma, fieldName, true);
+ return reflectField(sma, fieldName);
+ }
+
+ public static <T> T reflectField(Object instance, String fieldName) throws
IllegalAccessException {
+ return (T) FieldUtils.readField(instance, fieldName, true);
}
@Test
