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 fea65cbf6..71fdf2ff8 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 @@ -298,8 +298,8 @@ public class DefaultConfiguration implements Configuration { builder.factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON); builder.factory(PropertyAccessor.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON); - builder.factory(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); - builder.factory(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); + builder.factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); + builder.factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); builder.factory(OgnlUtil.class, Scope.SINGLETON); builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); 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 394aaa69d..49308d263 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 @@ -217,8 +217,8 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider .factory(TextProviderFactory.class, StrutsTextProviderFactory.class, Scope.SINGLETON) .factory(LocaleProviderFactory.class, DefaultLocaleProviderFactory.class, Scope.SINGLETON) - .factory(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) - .factory(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) + .factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) + .factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) .factory(OgnlUtil.class, Scope.SINGLETON) .factory(CollectionConverter.class, Scope.SINGLETON) .factory(ArrayConverter.class, Scope.SINGLETON) 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 560f18968..49be23790 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -82,7 +82,9 @@ public class OgnlUtil { */ @Deprecated public OgnlUtil() { - this(null, null); // Instantiate default Expression and BeanInfo caches (null factories) + // Instantiate default Expression and BeanInfo caches (factories must be non-null). + this(new DefaultOgnlExpressionCacheFactory(), + new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); } /** @@ -98,9 +100,15 @@ public class OgnlUtil { */ @Inject public OgnlUtil( - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, required = false) ExpressionCacheFactory ognlExpressionCacheFactory, - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, required = false) BeanInfoCacheFactory, BeanInfo> ognlBeanInfoCacheFactory + @Inject ExpressionCacheFactory ognlExpressionCacheFactory, + @Inject BeanInfoCacheFactory, BeanInfo> ognlBeanInfoCacheFactory ) { + if (ognlExpressionCacheFactory == null) { + throw new IllegalArgumentException("ExpressionCacheFactory parameter cannot be null"); + } + if (ognlBeanInfoCacheFactory == null) { + throw new IllegalArgumentException("BeanInfoCacheFactory parameter cannot be null"); + } excludedClasses = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); @@ -109,11 +117,8 @@ public class OgnlUtil { devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); - OgnlCacheFactory ognlExpressionCacheFactory1 = (ognlExpressionCacheFactory != null ? ognlExpressionCacheFactory : new DefaultOgnlExpressionCacheFactory<>()); - OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory1 = (ognlBeanInfoCacheFactory != null ? ognlBeanInfoCacheFactory : new DefaultOgnlBeanInfoCacheFactory<>()); - - this.expressionCache = ognlExpressionCacheFactory1.buildOgnlCache(); - this.beanInfoCache = ognlBeanInfoCacheFactory1.buildOgnlCache(); + this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(); + this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(); } @Inject diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index edc327810..753a80b8d 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -231,8 +231,8 @@ 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=com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory -struts.ognl.beanInfoCacheFactory=com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory +# struts.ognl.expressionCacheFactory=customOgnlExpressionCacheFactory +# struts.ognl.beanInfoCacheFactory=customOgnlBeanInfoCacheFactory ### Specify a limit to the number of entries in the OGNL expressionCache. ### For the standard expressionCache mode, when the limit is exceeded the entire cache's diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index b6ade6a74..f06a338d7 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -231,8 +231,8 @@ - - + + 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 8549fe8b2..b5795a326 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1313,11 +1313,20 @@ public class OgnlUtilTest extends XWorkTestCase { internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); } - public void testDefaultOgnlUtilExclusionsAlternateConstructor() { - OgnlUtil basicOgnlUtil = new OgnlUtil(null, null); - - internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil); - internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); + public void testDefaultOgnlUtilAlternateConstructorArguments() { + // Code coverage test for the OgnlUtil alternate constructor method, and verify expected behaviour. + try { + OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), null); + fail("null beanInfoCacheFactory should result in exception"); + } catch (IllegalArgumentException iaex) { + // expected result + } + try { + OgnlUtil basicOgnlUtil = new OgnlUtil(null, new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); + fail("null expressionCacheFactory should result in exception"); + } catch (IllegalArgumentException iaex) { + // expected result + } } public void testDefaultOgnlUtilExclusionsAlternateConstructorPopulated() { @@ -1690,20 +1699,6 @@ public class OgnlUtilTest extends XWorkTestCase { } } - public void testGetExcludedPackageNamesAlternateConstructor() { - // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(null, null); - util.setExcludedPackageNames("java.lang,java.awt"); - assertEquals(util.getExcludedPackageNames().size(), 2); - try { - util.getExcludedPackageNames().clear(); - } catch (Exception ex) { - assertTrue(ex instanceof UnsupportedOperationException); - } finally { - assertEquals(util.getExcludedPackageNames().size(), 2); - } - } - public void testGetExcludedPackageNamesAlternateConstructorPopulated() { // Getter should return an immutable collection OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); @@ -1732,20 +1727,6 @@ public class OgnlUtilTest extends XWorkTestCase { } } - public void testGetExcludedClassesAlternateConstructor() { - // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(null, null); - util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL"); - assertEquals(util.getExcludedClasses().size(), 3); - try { - util.getExcludedClasses().clear(); - } catch (Exception ex) { - assertTrue(ex instanceof UnsupportedOperationException); - } finally { - assertEquals(util.getExcludedClasses().size(), 3); - } - } - public void testGetExcludedClassesAlternateConstructorPopulated() { // Getter should return an immutable collection OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); @@ -1774,20 +1755,6 @@ public class OgnlUtilTest extends XWorkTestCase { } } - public void testGetExcludedPackageNamePatternsAlternateConstructor() { - // Getter should return an immutable collection - OgnlUtil util = new OgnlUtil(null, null); - util.setExcludedPackageNamePatterns("java.lang."); - assertEquals(util.getExcludedPackageNamePatterns().size(), 1); - try { - util.getExcludedPackageNamePatterns().clear(); - } catch (Exception ex) { - assertTrue(ex instanceof UnsupportedOperationException); - } finally { - assertEquals(util.getExcludedPackageNamePatterns().size(), 1); - } - } - public void testGetExcludedPackageNamePatternsAlternateConstructorPopulated() { // Getter should return an immutable collection OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>());