From 4e4cf468a2778903cdac4e494acb59f1f9128bb1 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Thu, 25 Jul 2024 20:33:14 +1000 Subject: [PATCH] WW-5411 Delete deprecated code part 5 --- .../xwork2/ognl/SecurityMemberAccess.java | 14 -------- .../util/StrutsLocalizedTextProvider.java | 12 +++++-- .../reflection/ReflectionProviderFactory.java | 32 ------------------- .../xwork2/ognl/OgnlValueStackTest.java | 9 +++--- .../xwork2/ognl/SecurityMemberAccessTest.java | 10 ++++-- .../xwork2/ognl/SetPropertiesTest.java | 8 +++-- .../util/StrutsLocalizedTextProviderTest.java | 7 ++-- .../test/ExternalSecurityMemberAccess.java | 6 ++-- .../ExternalSecurityMemberAccessTest.java | 4 +-- .../interceptor/CspInterceptorTest.java | 6 ++-- .../parameter/ParametersInterceptorTest.java | 2 +- .../SecurityMemberAccessInServletsTest.java | 4 +-- 12 files changed, 42 insertions(+), 72 deletions(-) delete mode 100644 core/src/main/java/com/opensymphony/xwork2/util/reflection/ReflectionProviderFactory.java 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 f15b50af1..29faaf3da 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -112,20 +112,6 @@ public class SecurityMemberAccess implements MemberAccess { this.threadAllowlist = threadAllowlist; } - /** - * SecurityMemberAccess - * - access decisions based on whether member is static (or not) - * - 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(ProviderAllowlist, ThreadAllowlist)} instead. - */ - @Deprecated - public SecurityMemberAccess(boolean allowStaticFieldAccess) { - this(null, null); - useAllowStaticFieldAccess(String.valueOf(allowStaticFieldAccess)); - } - @Override public Object setup(Map context, Object target, Member member, String propertyName) { Object result = null; diff --git a/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java b/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java index 441e52e47..abe407866 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java @@ -22,7 +22,8 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.ModelDriven; import com.opensymphony.xwork2.conversion.impl.XWorkConverter; -import com.opensymphony.xwork2.util.reflection.ReflectionProviderFactory; +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.util.reflection.ReflectionProvider; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -37,6 +38,7 @@ import java.util.ResourceBundle; public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { private static final Logger LOG = LogManager.getLogger(StrutsLocalizedTextProvider.class); + private transient ReflectionProvider reflectionProvider; public StrutsLocalizedTextProvider() { addDefaultResourceBundle(XWORK_MESSAGES_BUNDLE); @@ -276,9 +278,9 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { if (prop != null) { Object obj = valueStack.findValue(prop); try { - Object actionObj = ReflectionProviderFactory.getInstance().getRealTarget(prop, valueStack.getContext(), valueStack.getRoot()); + Object actionObj = reflectionProvider.getRealTarget(prop, valueStack.getContext(), valueStack.getRoot()); if (actionObj != null) { - PropertyDescriptor propertyDescriptor = ReflectionProviderFactory.getInstance().getPropertyDescriptor(actionObj.getClass(), prop); + PropertyDescriptor propertyDescriptor = reflectionProvider.getPropertyDescriptor(actionObj.getClass(), prop); if (propertyDescriptor != null) { Class clazz = propertyDescriptor.getPropertyType(); @@ -373,4 +375,8 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { return findText(bundle, aTextName, locale, defaultMessage, args, valueStack); } + @Inject + public void setReflectionProvider(ReflectionProvider reflectionProvider) { + this.reflectionProvider = reflectionProvider; + } } diff --git a/core/src/main/java/com/opensymphony/xwork2/util/reflection/ReflectionProviderFactory.java b/core/src/main/java/com/opensymphony/xwork2/util/reflection/ReflectionProviderFactory.java deleted file mode 100644 index 55541add6..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/util/reflection/ReflectionProviderFactory.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package com.opensymphony.xwork2.util.reflection; - -import com.opensymphony.xwork2.ActionContext; - -/** - * @deprecated inject RefectionProvider directly - */ -@Deprecated -public class ReflectionProviderFactory { - - public static ReflectionProvider getInstance() { - return ActionContext.getContext().getContainer().getInstance(ReflectionProvider.class); - } -} 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 a983b5d26..f6e513710 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java @@ -26,7 +26,6 @@ import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.conversion.impl.ConversionData; import com.opensymphony.xwork2.conversion.impl.XWorkConverter; import com.opensymphony.xwork2.inject.ContainerBuilder; -import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor; import com.opensymphony.xwork2.ognl.accessor.RootAccessor; import com.opensymphony.xwork2.test.StubConfigurationProvider; import com.opensymphony.xwork2.test.TestBean2; @@ -1059,9 +1058,11 @@ public class OgnlValueStackTest extends XWorkTestCase { public void testConstructorWithAStack() { vs.push("Hello World"); - OgnlValueStack stack2 = new OgnlValueStack(vs, - container.getInstance(XWorkConverter.class), - (CompoundRootAccessor) container.getInstance(RootAccessor.class), new SecurityMemberAccess(true)); + OgnlValueStack stack2 = new OgnlValueStack( + vs, + container.getInstance(XWorkConverter.class), + container.getInstance(RootAccessor.class), + new SecurityMemberAccess(null, null)); container.inject(stack2); assertEquals(vs.getRoot(), stack2.getRoot()); 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 d508ef99d..47eb2fb23 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -58,8 +58,8 @@ public class SecurityMemberAccessTest { private Map context; private FooBar target; protected SecurityMemberAccess sma; - private ProviderAllowlist mockedProviderAllowlist; - private ThreadAllowlist mockedThreadAllowlist; + protected ProviderAllowlist mockedProviderAllowlist; + protected ThreadAllowlist mockedThreadAllowlist; @Before public void setUp() throws Exception { @@ -73,10 +73,14 @@ public class SecurityMemberAccessTest { protected void assignNewSma(boolean allowStaticFieldAccess) { when(mockedProviderAllowlist.getProviderAllowlist()).thenReturn(new HashSet<>()); when(mockedThreadAllowlist.getAllowlist()).thenReturn(new HashSet<>()); - sma = new SecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); + assignNewSmaHelper(); sma.useAllowStaticFieldAccess(String.valueOf(allowStaticFieldAccess)); } + protected void assignNewSmaHelper() { + sma = new SecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); + } + private T reflectField(String fieldName) throws IllegalAccessException { return reflectField(sma, fieldName); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java index cbd1f519e..436dced89 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java @@ -38,7 +38,11 @@ import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.util.reflection.ReflectionContextState; import ognl.Ognl; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; /** @@ -57,7 +61,7 @@ public class SetPropertiesTest extends XWorkTestCase { } public void testOgnlUtilEmptyStringAsLong() { Bar bar = new Bar(); - Map context = Ognl.createDefaultContext(bar, new SecurityMemberAccess(true)); + Map context = Ognl.createDefaultContext(bar, new SecurityMemberAccess(null, null)); context.put(XWorkConverter.REPORT_CONVERSION_ERRORS, Boolean.TRUE); bar.setId(null); diff --git a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java index a7ba7f61e..7c19a5f0e 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java @@ -581,7 +581,7 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { * * @since 6.0.0 */ - class TestStrutsLocalizedTextProvider extends StrutsLocalizedTextProvider { + static class TestStrutsLocalizedTextProvider extends StrutsLocalizedTextProvider { /** * Some test correctness depends on this {@link #RELOADED} value matching that of the private ancestor @@ -624,9 +624,8 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { * @return true if resource bundles reloaded indicator is true, false otherwise (including if value was never set). */ public boolean getBundlesReloadedIndicatorValue() { - final ActionContext actionContext = ActionContext.getContext(); - final Object reloadedObject = actionContext.get(RELOADED); - return ((reloadedObject instanceof Boolean) ? ((Boolean) reloadedObject).booleanValue() : false); + final Object reloadedObject = ActionContext.getContext().get(RELOADED); + return reloadedObject instanceof Boolean && (Boolean) reloadedObject; } } } diff --git a/core/src/test/java/com/test/ExternalSecurityMemberAccess.java b/core/src/test/java/com/test/ExternalSecurityMemberAccess.java index a53f1736b..4043e9ada 100644 --- a/core/src/test/java/com/test/ExternalSecurityMemberAccess.java +++ b/core/src/test/java/com/test/ExternalSecurityMemberAccess.java @@ -19,11 +19,13 @@ package com.test; import com.opensymphony.xwork2.ognl.SecurityMemberAccess; +import org.apache.struts2.ognl.ProviderAllowlist; +import org.apache.struts2.ognl.ThreadAllowlist; class ExternalSecurityMemberAccess extends SecurityMemberAccess { - ExternalSecurityMemberAccess(boolean allowStaticFieldAccess) { - super(allowStaticFieldAccess); + public ExternalSecurityMemberAccess(ProviderAllowlist providerAllowlist, ThreadAllowlist threadAllowlist) { + super(providerAllowlist, threadAllowlist); } @Override diff --git a/core/src/test/java/com/test/ExternalSecurityMemberAccessTest.java b/core/src/test/java/com/test/ExternalSecurityMemberAccessTest.java index a418e6b4e..5fd642eb0 100644 --- a/core/src/test/java/com/test/ExternalSecurityMemberAccessTest.java +++ b/core/src/test/java/com/test/ExternalSecurityMemberAccessTest.java @@ -27,7 +27,7 @@ import com.opensymphony.xwork2.ognl.SecurityMemberAccessTest; public class ExternalSecurityMemberAccessTest extends SecurityMemberAccessTest { @Override - protected void assignNewSma(boolean allowStaticFieldAccess) { - sma = new ExternalSecurityMemberAccess(allowStaticFieldAccess); + protected void assignNewSmaHelper() { + sma = new ExternalSecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java index f913f74d5..94d86cf46 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java @@ -21,6 +21,8 @@ package org.apache.struts2.interceptor; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.mock.MockActionInvocation; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; import org.apache.logging.log4j.util.Strings; import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.TestAction; @@ -32,9 +34,6 @@ import org.apache.struts2.interceptor.csp.DefaultCspSettings; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; -import jakarta.servlet.http.HttpSession; -import jakarta.servlet.http.HttpServletRequest; - import static org.junit.Assert.assertNotEquals; public class CspInterceptorTest extends StrutsInternalTestCase { @@ -312,6 +311,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase { */ public static class CustomDefaultCspSettings extends DefaultCspSettings { + @Override protected String createPolicyFormat(HttpServletRequest request) { return "foo"; } diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java index 9bcc70ae0..b5caa3d26 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java @@ -965,7 +965,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { ValueStack stack = new OgnlValueStack( container.getInstance(XWorkConverter.class), (CompoundRootAccessor) container.getInstance(RootAccessor.class), - container.getInstance(TextProvider.class, "system"), new SecurityMemberAccess(true)) { + container.getInstance(TextProvider.class, "system"), new SecurityMemberAccess(null, null)) { @Override public void setValue(String expr, Object value) { actual.put(expr, value); diff --git a/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java b/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java index 9d934c36e..68766c477 100644 --- a/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java +++ b/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java @@ -38,7 +38,7 @@ public class SecurityMemberAccessInServletsTest extends StrutsInternalTestCase { public void testJavaxServletPackageAccess() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true); + SecurityMemberAccess sma = new SecurityMemberAccess(null, null); sma.useExcludedPackageNamePatterns("^(?!jakarta\\.servlet\\..+)(jakarta\\..+)"); @@ -54,7 +54,7 @@ public class SecurityMemberAccessInServletsTest extends StrutsInternalTestCase { public void testJavaxServletPackageExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true); + SecurityMemberAccess sma = new SecurityMemberAccess(null, null); sma.useExcludedPackageNamePatterns("^jakarta\\..+");