From ca740ed8fbd97634e6dcaa7aaec29d6389a5f7ed Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 21 Feb 2026 18:18:08 +0100 Subject: [PATCH] WW-5514 Add StrutsProxyService for proxy detection and resolution (#1586) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(proxy): WW-5514 add StrutsProxyService for proxy detection and resolution Introduces a configurable ProxyService interface and StrutsProxyService implementation for detecting and resolving Spring AOP/Hibernate proxies. Key changes: - Add ProxyService interface with isProxy, ultimateTargetClass, and resolveTargetMember methods - Add StrutsProxyService implementation using configurable caches - Add ProxyCacheFactory and StrutsProxyCacheFactory for cache management - Integrate ProxyService into ChainingInterceptor, ParametersInterceptor, and SecurityMemberAccess - Add integration test with Spring AOP proxied action chaining - Add configuration constants for proxy cache type and size The StrutsProxyService correctly handles: - Spring CGLIB proxies (class-based) - Spring JDK dynamic proxies (interface-based) - Hibernate entity proxies - Member resolution for allowlist checking Fixes WW-5514 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * test(proxy): WW-5514 add ProxyService integration tests for Spring proxies Add integration tests to SpringProxyUtilTest that verify the new ProxyService works correctly with real Spring AOP proxies, alongside the existing deprecated ProxyUtil tests. Co-authored-by: Cursor * fix(proxy): WW-5514 address PR review feedback for proxy caches Remove targetClassCache from StrutsProxyService to avoid memory leak (object-keyed cache reintroduced from PR #1578). Change default proxy cache type to wtlfu to align with all other caches. Switch deprecated ProxyUtil static caches to BASIC to remove hard Caffeine dependency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude Co-authored-by: Cursor --- .../showcase/proxy/LoggingInterceptor.java | 42 ++ .../main/resources/struts-actionchaining.xml | 31 +- apps/showcase/src/main/resources/struts.xml | 93 ++-- .../webapp/WEB-INF/applicationContext.xml | 20 + .../SpringProxyActionChainingTest.java | 67 +++ .../org/apache/struts2/StrutsConstants.java | 29 ++ .../config/StrutsBeanSelectionProvider.java | 4 + .../config/impl/DefaultConfiguration.java | 8 + .../interceptor/ChainingInterceptor.java | 17 +- .../parameter/ParametersInterceptor.java | 12 +- .../struts2/ognl/ProxyCacheFactory.java | 27 ++ .../struts2/ognl/SecurityMemberAccess.java | 19 +- .../struts2/ognl/StrutsProxyCacheFactory.java | 39 ++ .../org/apache/struts2/util/ProxyService.java | 101 +++++ .../org/apache/struts2/util/ProxyUtil.java | 44 +- .../struts2/util/StrutsProxyService.java | 194 +++++++++ .../org/apache/struts2/default.properties | 12 + core/src/main/resources/struts-beans.xml | 4 + .../StrutsParameterAnnotationTest.java | 5 + .../ognl/SecurityMemberAccessTest.java | 5 + .../ognl/StrutsProxyCacheFactoryTest.java | 85 ++++ ...rutsProxyServiceSpringIntegrationTest.java | 275 ++++++++++++ .../struts2/util/StrutsProxyServiceTest.java | 399 ++++++++++++++++++ .../ExternalSecurityMemberAccessTest.java | 1 + .../struts2/json/DefaultJSONWriter.java | 10 +- .../apache/struts2/json/JSONResultTest.java | 8 +- .../ognl/SecurityMemberAccessProxyTest.java | 9 +- .../struts2/spring/SpringProxyUtilTest.java | 73 ++++ ...02-07-WW-5514-proxy-cache-configuration.md | 372 ++++++++++++++++ .../2026-02-08-WW-5514-validation.md | 175 ++++++++ 30 files changed, 2100 insertions(+), 80 deletions(-) create mode 100644 apps/showcase/src/main/java/org/apache/struts2/showcase/proxy/LoggingInterceptor.java create mode 100644 apps/showcase/src/test/java/it/org/apache/struts2/showcase/SpringProxyActionChainingTest.java create mode 100644 core/src/main/java/org/apache/struts2/ognl/ProxyCacheFactory.java create mode 100644 core/src/main/java/org/apache/struts2/ognl/StrutsProxyCacheFactory.java create mode 100644 core/src/main/java/org/apache/struts2/util/ProxyService.java create mode 100644 core/src/main/java/org/apache/struts2/util/StrutsProxyService.java create mode 100644 core/src/test/java/org/apache/struts2/ognl/StrutsProxyCacheFactoryTest.java create mode 100644 core/src/test/java/org/apache/struts2/util/StrutsProxyServiceSpringIntegrationTest.java create mode 100644 core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java create mode 100644 thoughts/shared/research/2026-02-07-WW-5514-proxy-cache-configuration.md create mode 100644 thoughts/shared/validation/2026-02-08-WW-5514-validation.md diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/proxy/LoggingInterceptor.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/proxy/LoggingInterceptor.java new file mode 100644 index 000000000..4a5dd40a7 --- /dev/null +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/proxy/LoggingInterceptor.java @@ -0,0 +1,42 @@ +/* + * 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 org.apache.struts2.showcase.proxy; + +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * Simple AOP interceptor that wraps actions in a Spring proxy. + * Used to test that Struts correctly handles Spring AOP proxied actions + * in action chaining scenarios (WW-5514). + */ +public class LoggingInterceptor implements MethodInterceptor { + + private static final Logger LOG = LogManager.getLogger(LoggingInterceptor.class); + + @Override + public Object invoke(MethodInvocation invocation) throws Throwable { + LOG.debug("Invoking method: {} on target: {}", + invocation.getMethod().getName(), + invocation.getThis().getClass().getName()); + return invocation.proceed(); + } +} diff --git a/apps/showcase/src/main/resources/struts-actionchaining.xml b/apps/showcase/src/main/resources/struts-actionchaining.xml index 4f39940f0..ae2a7461c 100644 --- a/apps/showcase/src/main/resources/struts-actionchaining.xml +++ b/apps/showcase/src/main/resources/struts-actionchaining.xml @@ -20,21 +20,26 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" + "https://struts.apache.org/dtds/struts-6.0.dtd"> - - - actionChain2 - - - actionChain3 - - - /WEB-INF/actionchaining/actionChainingResult.jsp - - + + + actionChain2 + + + actionChain3 + + + /WEB-INF/actionchaining/actionChainingResult.jsp + + + + + actionChain2 + + diff --git a/apps/showcase/src/main/resources/struts.xml b/apps/showcase/src/main/resources/struts.xml index 5c1cf37ff..5dbe07cee 100644 --- a/apps/showcase/src/main/resources/struts.xml +++ b/apps/showcase/src/main/resources/struts.xml @@ -20,83 +20,88 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" + "https://struts.apache.org/dtds/struts-6.0.dtd"> - - - - - - + + + + + + - - + + - - + + + + + - + - - - + + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - + + + + - + /WEB-INF/showcase.jsp @@ -125,7 +130,7 @@ /WEB-INF/empmanager/editSkill.jsp - + @@ -146,9 +151,11 @@ - {1} + {1} /WEB-INF/empmanager/editEmployee.jsp - execute + + execute + /WEB-INF/empmanager/editEmployee.jsp @@ -168,5 +175,5 @@ - + diff --git a/apps/showcase/src/main/webapp/WEB-INF/applicationContext.xml b/apps/showcase/src/main/webapp/WEB-INF/applicationContext.xml index ef700ef48..788890326 100644 --- a/apps/showcase/src/main/webapp/WEB-INF/applicationContext.xml +++ b/apps/showcase/src/main/webapp/WEB-INF/applicationContext.xml @@ -115,5 +115,25 @@ + + + + + + + + + + proxiedActionChain1 + + + + + loggingInterceptor + + + + + diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/SpringProxyActionChainingTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/SpringProxyActionChainingTest.java new file mode 100644 index 000000000..8b3a9794c --- /dev/null +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/SpringProxyActionChainingTest.java @@ -0,0 +1,67 @@ +/* + * 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 it.org.apache.struts2.showcase; + +import org.htmlunit.WebClient; +import org.htmlunit.html.HtmlPage; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; + +/** + * Integration test verifying that Spring AOP proxied actions work correctly + * with action chaining. This tests the WW-5514 StrutsProxyService integration. + * + *

The test uses a Spring AOP proxied version of ActionChain1 (proxiedActionChain1) + * which is wrapped by {@link org.apache.struts2.showcase.proxy.LoggingInterceptor}. + * The ChainingInterceptor must correctly resolve the target class through + * StrutsProxyService to copy properties to the next action in the chain.

+ */ +public class SpringProxyActionChainingTest { + + /** + * Tests that action chaining works correctly when the first action is a Spring AOP proxy. + * + *

This verifies that: + *

    + *
  • StrutsProxyService correctly identifies the Spring CGLIB proxy
    • + *
  • ChainingInterceptor resolves the target class for property copying
    • + *
  • Properties from the proxied ActionChain1 are correctly copied to ActionChain2
    • + *
+ *

+ */ + @Test + public void testProxiedActionChaining() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage( + ParameterUtils.getBaseUrl() + "/actionchaining/proxiedActionChain1!input" + ); + + final String pageAsText = page.asNormalizedText(); + + // Verify properties were chained correctly despite proxy + assertTrue("ActionChain1 property should be present", + pageAsText.contains("Action Chain 1 Property 1: Property Set In Action Chain 1")); + assertTrue("ActionChain2 property should be present", + pageAsText.contains("Action Chain 2 Property 1: Property Set in Action Chain 2")); + assertTrue("ActionChain3 property should be present", + pageAsText.contains("Action Chain 3 Property 1: Property set in Action Chain 3")); + } + } +} diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 39b811068..84b9fd16e 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -522,6 +522,35 @@ public final class StrutsConstants { */ public static final String STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE = "struts.ognl.expressionCacheMaxSize"; + /** + * Specifies the type of cache to use for proxy detection. Valid values defined in + * {@link org.apache.struts2.ognl.OgnlCacheFactory.CacheType}. + * + * @since 7.2.0 + */ + public static final String STRUTS_PROXY_CACHE_TYPE = "struts.proxy.cacheType"; + + /** + * Specifies the maximum cache size for proxy detection caches. + * + * @since 7.2.0 + */ + public static final String STRUTS_PROXY_CACHE_MAXSIZE = "struts.proxy.cacheMaxSize"; + + /** + * The {@link org.apache.struts2.ognl.ProxyCacheFactory} implementation class. + * + * @since 7.2.0 + */ + public static final String STRUTS_PROXY_CACHE_FACTORY = "struts.proxy.cacheFactory"; + + /** + * The {@link org.apache.struts2.util.ProxyService} implementation class. + * + * @since 7.2.0 + */ + public static final String STRUTS_PROXYSERVICE = "struts.proxyService"; + /** * Enables evaluation of OGNL expressions * diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index eda65e527..c584a4f58 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -61,6 +61,7 @@ import org.apache.struts2.interceptor.exec.ExecutorProvider; import org.apache.struts2.ognl.BeanInfoCacheFactory; import org.apache.struts2.ognl.ExpressionCacheFactory; import org.apache.struts2.ognl.OgnlGuard; +import org.apache.struts2.ognl.ProxyCacheFactory; import org.apache.struts2.ognl.SecurityMemberAccess; import org.apache.struts2.ognl.accessor.RootAccessor; import org.apache.struts2.security.AcceptedPatternsChecker; @@ -72,6 +73,7 @@ import org.apache.struts2.url.UrlDecoder; import org.apache.struts2.url.UrlEncoder; import org.apache.struts2.util.ContentTypeMatcher; import org.apache.struts2.util.PatternMatcher; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.util.TextParser; import org.apache.struts2.util.ValueStackFactory; import org.apache.struts2.util.location.LocatableProperties; @@ -442,6 +444,8 @@ public class StrutsBeanSelectionProvider extends AbstractBeanSelectionProvider { alias(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, builder, props, Scope.SINGLETON); alias(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(ProxyCacheFactory.class, StrutsConstants.STRUTS_PROXY_CACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(ProxyService.class, StrutsConstants.STRUTS_PROXYSERVICE, builder, props, Scope.SINGLETON); alias(SecurityMemberAccess.class, StrutsConstants.STRUTS_MEMBER_ACCESS, builder, props, Scope.PROTOTYPE); alias(OgnlGuard.class, StrutsConstants.STRUTS_OGNL_GUARD, builder, props, Scope.SINGLETON); diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java index 48791c919..9ec9f9c20 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java @@ -85,13 +85,17 @@ import org.apache.struts2.ognl.ExpressionCacheFactory; import org.apache.struts2.ognl.OgnlCacheFactory; import org.apache.struts2.ognl.OgnlReflectionProvider; import org.apache.struts2.ognl.OgnlUtil; +import org.apache.struts2.ognl.ProxyCacheFactory; +import org.apache.struts2.ognl.StrutsProxyCacheFactory; import org.apache.struts2.ognl.OgnlValueStackFactory; import org.apache.struts2.ognl.SecurityMemberAccess; import org.apache.struts2.ognl.accessor.CompoundRootAccessor; import org.apache.struts2.ognl.accessor.RootAccessor; import org.apache.struts2.ognl.accessor.XWorkMethodAccessor; +import org.apache.struts2.util.StrutsProxyService; import org.apache.struts2.util.OgnlTextParser; import org.apache.struts2.util.PatternMatcher; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.text.StrutsLocalizedTextProvider; import org.apache.struts2.util.TextParser; import org.apache.struts2.util.ValueStack; @@ -144,6 +148,8 @@ public class DefaultConfiguration implements Configuration { 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_PROXY_CACHE_TYPE, OgnlCacheFactory.CacheType.BASIC); + constants.put(StrutsConstants.STRUTS_PROXY_CACHE_MAXSIZE, 10000); constants.put(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, Boolean.FALSE); BOOTSTRAP_CONSTANTS = Collections.unmodifiableMap(constants); } @@ -395,6 +401,8 @@ public class DefaultConfiguration implements Configuration { .factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) .factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) + .factory(ProxyCacheFactory.class, StrutsProxyCacheFactory.class, Scope.SINGLETON) + .factory(ProxyService.class, StrutsProxyService.class, Scope.SINGLETON) .factory(OgnlUtil.class, Scope.SINGLETON) .factory(SecurityMemberAccess.class, Scope.PROTOTYPE) .factory(OgnlGuard.class, StrutsOgnlGuard.class, Scope.SINGLETON) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index 2837d4c10..9c18d8869 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -27,7 +27,7 @@ import org.apache.struts2.inject.Inject; import org.apache.struts2.result.ActionChainResult; import org.apache.struts2.result.Result; import org.apache.struts2.util.CompoundRoot; -import org.apache.struts2.util.ProxyUtil; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.util.TextParseUtil; import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.reflection.ReflectionProvider; @@ -96,7 +96,7 @@ import java.util.Map; *

* * Example code: - * + *

* * 

  * <action name="someAction" class="com.examples.SomeAction">
@@ -114,7 +114,6 @@ import java.util.Map;
  *
* * - * * @author mrdon * @author tm_jee ( tm_jee(at)yahoo.co.uk ) * @see ActionChainResult @@ -135,12 +134,18 @@ public class ChainingInterceptor extends AbstractInterceptor { protected Collection includes; protected ReflectionProvider reflectionProvider; + private ProxyService proxyService; @Inject public void setReflectionProvider(ReflectionProvider prov) { this.reflectionProvider = prov; } + @Inject + public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; + } + @Inject(value = StrutsConstants.STRUTS_CHAINING_COPY_ERRORS, required = false) public void setCopyErrors(String copyErrors) { this.copyErrors = "true".equalsIgnoreCase(copyErrors); @@ -175,8 +180,8 @@ public class ChainingInterceptor extends AbstractInterceptor { } Object action = invocation.getAction(); Class editable = null; - if (ProxyUtil.isProxy(action)) { - editable = ProxyUtil.ultimateTargetClass(action); + if (proxyService.isProxy(action)) { + editable = proxyService.ultimateTargetClass(action); } reflectionProvider.copy(object, action, ctxMap, prepareExcludes(), includes, editable); } @@ -184,7 +189,7 @@ public class ChainingInterceptor extends AbstractInterceptor { private Collection prepareExcludes() { Collection localExcludes = excludes; - if (!copyErrors || !copyMessages ||!copyFieldErrors) { + if (!copyErrors || !copyMessages || !copyFieldErrors) { if (localExcludes == null) { localExcludes = new HashSet<>(); if (!copyErrors) { diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index 32cffc291..293f4968a 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -39,7 +39,7 @@ import org.apache.struts2.security.DefaultAcceptedPatternsChecker; import org.apache.struts2.security.ExcludedPatternsChecker; import org.apache.struts2.util.ClearableValueStack; import org.apache.struts2.util.MemberAccessValueStack; -import org.apache.struts2.util.ProxyUtil; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.util.TextParseUtil; import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.ValueStackFactory; @@ -95,6 +95,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { private ValueStackFactory valueStackFactory; private OgnlUtil ognlUtil; protected ThreadAllowlist threadAllowlist; + private ProxyService proxyService; private ExcludedPatternsChecker excludedPatterns; private AcceptedPatternsChecker acceptedPatterns; private Set excludedValuePatterns = null; @@ -115,6 +116,11 @@ public class ParametersInterceptor extends MethodFilterInterceptor { this.threadAllowlist = threadAllowlist; } + @Inject + public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; + } + @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String mode) { this.devMode = BooleanUtils.toBoolean(mode); @@ -516,8 +522,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } protected Class ultimateClass(Object action) { - if (ProxyUtil.isProxy(action)) { - return ProxyUtil.ultimateTargetClass(action); + if (proxyService.isProxy(action)) { + return proxyService.ultimateTargetClass(action); } return action.getClass(); } diff --git a/core/src/main/java/org/apache/struts2/ognl/ProxyCacheFactory.java b/core/src/main/java/org/apache/struts2/ognl/ProxyCacheFactory.java new file mode 100644 index 000000000..1243b6f24 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/ProxyCacheFactory.java @@ -0,0 +1,27 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed 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 org.apache.struts2.ognl; + +/** + * A proxy interface to be used with Struts DI mechanism for proxy detection caching. + * + * @param <Key> The type for the cache key entries + * @param <Value> The type for the cache value entries + * @since 7.2.0 + */ +public interface ProxyCacheFactory extends OgnlCacheFactory { + +} diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java index 035a685bf..d25bbe377 100644 --- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java @@ -25,7 +25,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; import org.apache.struts2.inject.Inject; -import org.apache.struts2.util.ProxyUtil; +import org.apache.struts2.util.ProxyService; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Constructor; @@ -76,6 +76,8 @@ public class SecurityMemberAccess implements MemberAccess { private final ProviderAllowlist providerAllowlist; private final ThreadAllowlist threadAllowlist; + private ProxyService proxyService; + private boolean allowStaticFieldAccess = true; private Set excludeProperties = emptySet(); @@ -107,6 +109,11 @@ public class SecurityMemberAccess implements MemberAccess { this.threadAllowlist = threadAllowlist; } + @Inject + public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; + } + @Override public Object setup(OgnlContext context, Object target, Member member, String propertyName) { Object result = null; @@ -214,15 +221,15 @@ public class SecurityMemberAccess implements MemberAccess { Class targetClass = target != null ? target.getClass() : null; - if (!disallowProxyObjectAccess && ProxyUtil.isProxy(target)) { + if (!disallowProxyObjectAccess && proxyService.isProxy(target)) { // If `disallowProxyObjectAccess` is not set, allow resolving Hibernate entities and Spring proxies to their // underlying classes/members. This allows the allowlist capability to continue working and still offer // protection in applications where the developer has accepted the risk of allowing OGNL access to Hibernate // entities and Spring proxies. This is preferred to having to disable the allowlist capability entirely. - Class newTargetClass = ProxyUtil.ultimateTargetClass(target); + Class newTargetClass = proxyService.ultimateTargetClass(target); if (newTargetClass != targetClass) { targetClass = newTargetClass; - member = ProxyUtil.resolveTargetMember(member, newTargetClass); + member = proxyService.resolveTargetMember(member, newTargetClass); } } @@ -312,14 +319,14 @@ public class SecurityMemberAccess implements MemberAccess { * @return {@code true} if proxy object access is allowed */ protected boolean checkProxyObjectAccess(Object target) { - return !(disallowProxyObjectAccess && ProxyUtil.isProxy(target)); + return !(disallowProxyObjectAccess && proxyService.isProxy(target)); } /** * @return {@code true} if proxy member access is allowed */ protected boolean checkProxyMemberAccess(Object target, Member member) { - return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)); + return !(disallowProxyMemberAccess && proxyService.isProxyMember(member, target)); } /** diff --git a/core/src/main/java/org/apache/struts2/ognl/StrutsProxyCacheFactory.java b/core/src/main/java/org/apache/struts2/ognl/StrutsProxyCacheFactory.java new file mode 100644 index 000000000..ac80163af --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/StrutsProxyCacheFactory.java @@ -0,0 +1,39 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed 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 org.apache.struts2.ognl; + +import org.apache.commons.lang3.EnumUtils; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.inject.Inject; + +/** + * Struts proxy cache factory implementation. + * Used for creating caches for proxy detection operations. + * + * @param <Key> The type for the cache key entries + * @param <Value> The type for the cache value entries + * @since 7.2.0 + */ +public class StrutsProxyCacheFactory extends DefaultOgnlCacheFactory + implements ProxyCacheFactory { + + @Inject + public StrutsProxyCacheFactory( + @Inject(value = StrutsConstants.STRUTS_PROXY_CACHE_MAXSIZE) String cacheMaxSize, + @Inject(value = StrutsConstants.STRUTS_PROXY_CACHE_TYPE) String defaultCacheType) { + super(Integer.parseInt(cacheMaxSize), EnumUtils.getEnumIgnoreCase(CacheType.class, defaultCacheType)); + } +} diff --git a/core/src/main/java/org/apache/struts2/util/ProxyService.java b/core/src/main/java/org/apache/struts2/util/ProxyService.java new file mode 100644 index 000000000..fe6aca7ae --- /dev/null +++ b/core/src/main/java/org/apache/struts2/util/ProxyService.java @@ -0,0 +1,101 @@ +/* + * 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 org.apache.struts2.util; + +import java.lang.reflect.Member; + +/** + * Service interface for proxy detection and resolution operations. + * Replaces static {@link ProxyUtil} methods with an injectable service. + * + * @since 7.2.0 + */ +public interface ProxyService { + + /** + * Determine the ultimate target class of the given instance, traversing + * not only a top-level proxy but any number of nested proxies as well &mdash; + * as long as possible without side effects. + * + * @param candidate the instance to check (might be a proxy) + * @return the ultimate target class (or the plain class of the given + * object as fallback; never {@code null}) + */ + Class ultimateTargetClass(Object candidate); + + /** + * Check whether the given object is a proxy. + * + * @param object the object to check + * @return true if the object is a Spring AOP or Hibernate proxy + */ + boolean isProxy(Object object); + + /** + * Check whether the given member is a proxy member of a proxy object or is a static proxy member. + * + * @param member the member to check + * @param object the object to check + * @return true if the member is a proxy member + */ + boolean isProxyMember(Member member, Object object); + + /** + * Check whether the given object is a Hibernate proxy. + * + * @param object the object to check + * @return true if the object is a Hibernate proxy + */ + boolean isHibernateProxy(Object object); + + /** + * Check whether the given member is a member of a Hibernate proxy. + * + * @param member the member to check + * @return true if the member is a Hibernate proxy member + */ + boolean isHibernateProxyMember(Member member); + + /** + * Get the target instance of the given object if it is a Hibernate proxy object, + * otherwise return the given object. + * + * @param object the object to check + * @return the target instance or the original object + */ + Object getHibernateProxyTarget(Object object); + + /** + * Resolve matching member on target class. + * + * @param proxyMember the proxy member + * @param targetClass the target class + * @return matching member on target object if one exists, otherwise the same member + */ + Member resolveTargetMember(Member proxyMember, Class targetClass); + + /** + * @param proxyMember the proxy member + * @param target the target object + * @return matching member on target object if one exists, otherwise the same member + * @deprecated since 7.1, use {@link #resolveTargetMember(Member, Class)} instead. + */ + @Deprecated + Member resolveTargetMember(Member proxyMember, Object target); +} diff --git a/core/src/main/java/org/apache/struts2/util/ProxyUtil.java b/core/src/main/java/org/apache/struts2/util/ProxyUtil.java index dbc1940e8..8117807d4 100644 --- a/core/src/main/java/org/apache/struts2/util/ProxyUtil.java +++ b/core/src/main/java/org/apache/struts2/util/ProxyUtil.java @@ -43,26 +43,31 @@ import static java.lang.reflect.Modifier.isStatic; /** * ProxyUtil *

- * Various utility methods dealing with proxies + * Various utility methods dealing with proxies. *

* + * @deprecated since 7.2, inject {@link ProxyService} instead. This class will be removed in a future version. */ +@Deprecated(since = "7.2") public class ProxyUtil { private static final int CACHE_MAX_SIZE = 10000; private static final int CACHE_INITIAL_CAPACITY = 256; private static final OgnlCache, Boolean> isProxyCache = new DefaultOgnlCacheFactory, Boolean>( - CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache(); + CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.BASIC, CACHE_INITIAL_CAPACITY).buildOgnlCache(); private static final OgnlCache isProxyMemberCache = new DefaultOgnlCacheFactory( - CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache(); + CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.BASIC, CACHE_INITIAL_CAPACITY).buildOgnlCache(); /** * Determine the ultimate target class of the given instance, traversing * not only a top-level proxy but any number of nested proxies as well — * as long as possible without side effects. + * * @param candidate the instance to check (might be a proxy) * @return the ultimate target class (or the plain class of the given * object as fallback; never {@code null}) + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static Class ultimateTargetClass(Object candidate) { Class result = null; if (isSpringAopProxy(candidate)) { @@ -78,8 +83,12 @@ public class ProxyUtil { /** * Check whether the given object is a proxy. + * * @param object the object to check + * @return true if the object is a Spring AOP or Hibernate proxy + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static boolean isProxy(Object object) { if (object == null) return false; return isProxyCache.computeIfAbsent(object.getClass(), @@ -88,9 +97,13 @@ public class ProxyUtil { /** * Check whether the given member is a proxy member of a proxy object or is a static proxy member. + * * @param member the member to check * @param object the object to check + * @return true if the member is a proxy member + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static boolean isProxyMember(Member member, Object object) { if (!isStatic(member.getModifiers()) && !isProxy(object)) { return false; @@ -103,7 +116,10 @@ public class ProxyUtil { * Check whether the given object is a Hibernate proxy. * * @param object the object to check + * @return true if the object is a Hibernate proxy + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static boolean isHibernateProxy(Object object) { try { return object != null && HibernateProxy.class.isAssignableFrom(object.getClass()); @@ -116,7 +132,10 @@ public class ProxyUtil { * Check whether the given member is a member of a Hibernate proxy. * * @param member the member to check + * @return true if the member is a Hibernate proxy member + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static boolean isHibernateProxyMember(Member member) { try { return hasMember(HibernateProxy.class, member); @@ -129,6 +148,7 @@ public class ProxyUtil { * Determine the ultimate target class of the given spring bean instance, traversing * not only a top-level spring proxy but any number of nested spring proxies as well — * as long as possible without side effects, that is, just for singleton targets. + * * @param candidate the instance to check (might be a spring AOP proxy) * @return the ultimate target class (or the plain class of the given * object as fallback; never {@code null}) @@ -143,6 +163,7 @@ public class ProxyUtil { /** * Check whether the given object is a Spring proxy. + * * @param object the object to check */ private static boolean isSpringAopProxy(Object object) { @@ -155,6 +176,7 @@ public class ProxyUtil { /** * Check whether the given member is a member of a spring proxy. + * * @param member the member to check */ private static boolean isSpringProxyMember(Member member) { @@ -172,7 +194,8 @@ public class ProxyUtil { /** * Check whether the given class has a given member. - * @param clazz the class to check + * + * @param clazz the class to check * @param member the member to check */ private static boolean hasMember(Class clazz, Member member) { @@ -189,8 +212,13 @@ public class ProxyUtil { } /** + * Get the target instance of the given object if it is a Hibernate proxy object. + * + * @param object the object to check * @return the target instance of the given object if it is a Hibernate proxy object, otherwise the given object + * @deprecated since 7.2, inject {@link ProxyService} instead */ + @Deprecated(since = "7.2") public static Object getHibernateProxyTarget(Object object) { try { return Hibernate.unproxy(object); @@ -200,9 +228,15 @@ public class ProxyUtil { } /** + * Resolve matching member on target object. + * + * @param proxyMember the proxy member + * @param target the target object + * @return matching member on target object if one exists, otherwise the same member * @deprecated since 7.1, use {@link #resolveTargetMember(Member, Class)} instead. + * Since 7.2, inject {@link ProxyService} instead. */ - @Deprecated + @Deprecated(since = "7.1") public static Member resolveTargetMember(Member proxyMember, Object target) { return resolveTargetMember(proxyMember, target.getClass()); } diff --git a/core/src/main/java/org/apache/struts2/util/StrutsProxyService.java b/core/src/main/java/org/apache/struts2/util/StrutsProxyService.java new file mode 100644 index 000000000..76c06f469 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/util/StrutsProxyService.java @@ -0,0 +1,194 @@ +/* + * 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 org.apache.struts2.util; + +import org.apache.commons.lang3.reflect.ConstructorUtils; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.commons.lang3.reflect.MethodUtils; +import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.OgnlCache; +import org.apache.struts2.ognl.ProxyCacheFactory; +import org.hibernate.Hibernate; +import org.hibernate.proxy.HibernateProxy; +import org.springframework.aop.TargetClassAware; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.framework.AopProxyUtils; +import org.springframework.aop.support.AopUtils; +import org.springframework.aop.SpringProxy; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Member; +import java.lang.reflect.Method; + +import static java.lang.reflect.Modifier.isPublic; +import static java.lang.reflect.Modifier.isStatic; + +/** + * Default implementation of {@link ProxyService}. + * Provides proxy detection and resolution for Spring AOP and Hibernate proxies. + * + * @since 7.2.0 + */ +public class StrutsProxyService implements ProxyService { + + private final OgnlCache, Boolean> isProxyCache; + private final OgnlCache isProxyMemberCache; + + @Inject + @SuppressWarnings("unchecked") + public StrutsProxyService(ProxyCacheFactory proxyCacheFactory) { + this.isProxyCache = (OgnlCache, Boolean>) proxyCacheFactory.buildOgnlCache(); + this.isProxyMemberCache = (OgnlCache) proxyCacheFactory.buildOgnlCache(); + } + + @Override + public Class ultimateTargetClass(Object candidate) { + Class result = null; + if (isSpringAopProxy(candidate)) { + result = springUltimateTargetClass(candidate); + } else if (isHibernateProxy(candidate)) { + result = getHibernateProxyTarget(candidate).getClass(); + } + if (result == null) { + result = candidate.getClass(); + } + return result; + } + + @Override + public boolean isProxy(Object object) { + if (object == null) return false; + return isProxyCache.computeIfAbsent(object.getClass(), + k -> isSpringAopProxy(object) || isHibernateProxy(object)); + } + + @Override + public boolean isProxyMember(Member member, Object object) { + if (!isStatic(member.getModifiers()) && !isProxy(object)) { + return false; + } + return isProxyMemberCache.computeIfAbsent(member, + k -> isSpringProxyMember(member) || isHibernateProxyMember(member)); + } + + @Override + public boolean isHibernateProxy(Object object) { + try { + return object != null && HibernateProxy.class.isAssignableFrom(object.getClass()); + } catch (LinkageError ignored) { + return false; + } + } + + @Override + public boolean isHibernateProxyMember(Member member) { + try { + return hasMember(HibernateProxy.class, member); + } catch (LinkageError ignored) { + return false; + } + } + + @Override + public Object getHibernateProxyTarget(Object object) { + try { + return Hibernate.unproxy(object); + } catch (LinkageError ignored) { + return object; + } + } + + @Override + public Member resolveTargetMember(Member proxyMember, Class targetClass) { + int mod = proxyMember.getModifiers(); + if (proxyMember instanceof Method) { + if (isPublic(mod)) { + return MethodUtils.getMatchingAccessibleMethod(targetClass, proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } else { + return MethodUtils.getMatchingMethod(targetClass, proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } + } else if (proxyMember instanceof Field) { + return FieldUtils.getField(targetClass, proxyMember.getName(), isPublic(mod)); + } else if (proxyMember instanceof Constructor && isPublic(mod)) { + return ConstructorUtils.getMatchingAccessibleConstructor(targetClass, ((Constructor) proxyMember).getParameterTypes()); + } + return proxyMember; + } + + @Override + @Deprecated + public Member resolveTargetMember(Member proxyMember, Object target) { + return resolveTargetMember(proxyMember, target.getClass()); + } + + /** + * Determine the ultimate target class of the given spring bean instance. + */ + private Class springUltimateTargetClass(Object candidate) { + try { + return AopProxyUtils.ultimateTargetClass(candidate); + } catch (LinkageError ignored) { + return candidate.getClass(); + } + } + + /** + * Check whether the given object is a Spring proxy. + */ + private boolean isSpringAopProxy(Object object) { + try { + return AopUtils.isAopProxy(object); + } catch (LinkageError ignored) { + return false; + } + } + + /** + * Check whether the given member is a member of a spring proxy. + */ + private boolean isSpringProxyMember(Member member) { + try { + if (hasMember(Advised.class, member)) + return true; + if (hasMember(TargetClassAware.class, member)) + return true; + if (hasMember(SpringProxy.class, member)) + return true; + } catch (LinkageError ignored) { + } + return false; + } + + /** + * Check whether the given class has a given member. + */ + private boolean hasMember(Class clazz, Member member) { + if (member instanceof Method method) { + return null != MethodUtils.getMatchingMethod(clazz, member.getName(), method.getParameterTypes()); + } + if (member instanceof Field) { + return null != FieldUtils.getField(clazz, member.getName(), true); + } + if (member instanceof Constructor constructor) { + return null != ConstructorUtils.getMatchingAccessibleConstructor(clazz, constructor.getParameterTypes()); + } + return false; + } +} diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 7f1e9e1a8..fcf9c8543 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -283,6 +283,18 @@ struts.ognl.beanInfoCacheType=wtlfu ### application-specific needs. struts.ognl.beanInfoCacheMaxSize=10000 +### Specifies the type of cache to use for proxy detection. See StrutsConstants class for further information. +struts.proxy.cacheType=wtlfu + +### Specifies the maximum cache size for proxy detection caches. +struts.proxy.cacheMaxSize=10000 + +### Specifies the ProxyCacheFactory implementation class. +struts.proxy.cacheFactory=struts + +### Specifies the ProxyService implementation class. +struts.proxyService=struts + ### Indicates if Dispatcher should handle unexpected exceptions by calling sendError() ### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security struts.handle.exception=true diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 742d5634f..7c59a88da 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -240,6 +240,10 @@ class="org.apache.struts2.ognl.DefaultOgnlExpressionCacheFactory" scope="singleton"/> + + diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java index 54125fdef..0040e98d3 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java @@ -30,7 +30,9 @@ import org.apache.struts2.ognl.DefaultOgnlBeanInfoCacheFactory; import org.apache.struts2.ognl.DefaultOgnlExpressionCacheFactory; import org.apache.struts2.ognl.OgnlUtil; import org.apache.struts2.ognl.StrutsOgnlGuard; +import org.apache.struts2.ognl.StrutsProxyCacheFactory; import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.util.StrutsProxyService; import org.apache.struts2.security.AcceptedPatternsChecker.IsAccepted; import org.apache.struts2.security.ExcludedPatternsChecker.IsExcluded; import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker; @@ -71,6 +73,9 @@ public class StrutsParameterAnnotationTest { new StrutsOgnlGuard()); parametersInterceptor.setOgnlUtil(ognlUtil); + var proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); + parametersInterceptor.setProxyService(proxyService); + NotExcludedAcceptedPatternsChecker checker = mock(NotExcludedAcceptedPatternsChecker.class); when(checker.isAccepted(anyString())).thenReturn(IsAccepted.yes("")); when(checker.isExcluded(anyString())).thenReturn(IsExcluded.no(Set.of())); diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java index 371e39aa4..a9b7b8c12 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java @@ -24,7 +24,9 @@ import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.struts2.TestBean; import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.test.TestBean2; +import org.apache.struts2.util.StrutsProxyService; import org.apache.struts2.util.Foo; +import org.apache.struts2.util.ProxyService; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; import org.junit.Before; @@ -58,6 +60,7 @@ public class SecurityMemberAccessTest { protected SecurityMemberAccess sma; protected ProviderAllowlist mockedProviderAllowlist; protected ThreadAllowlist mockedThreadAllowlist; + protected ProxyService proxyService; @Before public void setUp() { @@ -65,6 +68,7 @@ public class SecurityMemberAccessTest { target = new FooBar(); mockedProviderAllowlist = mock(ProviderAllowlist.class); mockedThreadAllowlist = mock(ThreadAllowlist.class); + proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); assignNewSma(true); } @@ -77,6 +81,7 @@ public class SecurityMemberAccessTest { protected void assignNewSmaHelper() { sma = new SecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); + sma.setProxyService(proxyService); } private T reflectField(String fieldName) throws IllegalAccessException { diff --git a/core/src/test/java/org/apache/struts2/ognl/StrutsProxyCacheFactoryTest.java b/core/src/test/java/org/apache/struts2/ognl/StrutsProxyCacheFactoryTest.java new file mode 100644 index 000000000..52f8fdf5f --- /dev/null +++ b/core/src/test/java/org/apache/struts2/ognl/StrutsProxyCacheFactoryTest.java @@ -0,0 +1,85 @@ +/* + * 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 org.apache.struts2.ognl; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link StrutsProxyCacheFactory}. + */ +public class StrutsProxyCacheFactoryTest { + + @Test + public void testCreateBasicCache() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("1000", "basic"); + + OgnlCache cache = factory.buildOgnlCache(); + + assertThat(cache).isNotNull(); + assertThat(cache).isInstanceOf(OgnlDefaultCache.class); + assertThat(cache.getEvictionLimit()).isEqualTo(1000); + } + + @Test + public void testCreateLruCache() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("500", "lru"); + + OgnlCache cache = factory.buildOgnlCache(); + + assertThat(cache).isNotNull(); + assertThat(cache).isInstanceOf(OgnlLRUCache.class); + assertThat(cache.getEvictionLimit()).isEqualTo(500); + } + + @Test + public void testCreateWtlfuCache() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("2000", "wtlfu"); + + OgnlCache cache = factory.buildOgnlCache(); + + assertThat(cache).isNotNull(); + assertThat(cache).isInstanceOf(OgnlCaffeineCache.class); + assertThat(cache.getEvictionLimit()).isEqualTo(2000); + } + + @Test + public void testCacheTypeIgnoresCase() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("1000", "BASIC"); + + OgnlCache cache = factory.buildOgnlCache(); + + assertThat(cache).isInstanceOf(OgnlDefaultCache.class); + } + + @Test + public void testGetCacheMaxSize() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("5000", "basic"); + + assertThat(factory.getCacheMaxSize()).isEqualTo(5000); + } + + @Test + public void testGetDefaultCacheType() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("1000", "lru"); + + assertThat(factory.getDefaultCacheType()).isEqualTo(OgnlCacheFactory.CacheType.LRU); + } +} diff --git a/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceSpringIntegrationTest.java b/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceSpringIntegrationTest.java new file mode 100644 index 000000000..931cfb3ff --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceSpringIntegrationTest.java @@ -0,0 +1,275 @@ +/* + * 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 org.apache.struts2.util; + +import org.apache.struts2.ognl.StrutsProxyCacheFactory; +import org.junit.Before; +import org.junit.Test; +import org.springframework.aop.MethodBeforeAdvice; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.framework.ProxyFactory; +import org.springframework.aop.SpringProxy; + +import java.lang.reflect.Method; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for {@link StrutsProxyService} with Spring AOP proxies. + * These tests verify the proxy service correctly handles various Spring proxy scenarios. + */ +public class StrutsProxyServiceSpringIntegrationTest { + + private StrutsProxyService proxyService; + + @Before + public void setUp() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("1000", "basic"); + proxyService = new StrutsProxyService(factory); + } + + @Test + public void testJdkDynamicProxyIsDetectedAsProxy() { + SimpleService proxy = createJdkDynamicProxy(new SimpleServiceImpl()); + + assertThat(proxyService.isProxy(proxy)).isTrue(); + assertThat(proxy).isInstanceOf(SpringProxy.class); + assertThat(proxy).isInstanceOf(Advised.class); + } + + @Test + public void testJdkDynamicProxyUltimateTargetClass() { + SimpleService proxy = createJdkDynamicProxy(new SimpleServiceImpl()); + + Class targetClass = proxyService.ultimateTargetClass(proxy); + + assertThat(targetClass).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testJdkDynamicProxyMemberDetection() throws NoSuchMethodException { + SimpleService proxy = createJdkDynamicProxy(new SimpleServiceImpl()); + + // Advised interface method should be detected as proxy member + Method isExposeProxy = proxy.getClass().getMethod("isExposeProxy"); + assertThat(proxyService.isProxyMember(isExposeProxy, proxy)).isTrue(); + + // Business method should not be detected as proxy member + Method getValue = proxy.getClass().getMethod("getValue"); + assertThat(proxyService.isProxyMember(getValue, proxy)).isFalse(); + } + + @Test + public void testJdkDynamicProxyResolveTargetMember() throws NoSuchMethodException { + SimpleService proxy = createJdkDynamicProxy(new SimpleServiceImpl()); + Method proxyMethod = proxy.getClass().getMethod("getValue"); + + // Resolve the method to the target class + Class targetClass = proxyService.ultimateTargetClass(proxy); + var resolved = proxyService.resolveTargetMember(proxyMethod, targetClass); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("getValue"); + assertThat(resolved.getDeclaringClass()).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testCglibProxyIsDetectedAsProxy() { + SimpleServiceImpl proxy = createCglibProxy(new SimpleServiceImpl()); + + assertThat(proxyService.isProxy(proxy)).isTrue(); + } + + @Test + public void testCglibProxyUltimateTargetClass() { + SimpleServiceImpl proxy = createCglibProxy(new SimpleServiceImpl()); + + Class targetClass = proxyService.ultimateTargetClass(proxy); + + assertThat(targetClass).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testCglibProxyMemberDetection() throws NoSuchMethodException { + SimpleServiceImpl proxy = createCglibProxy(new SimpleServiceImpl()); + + // Advised interface method should be detected as proxy member + Method isExposeProxy = proxy.getClass().getMethod("isExposeProxy"); + assertThat(proxyService.isProxyMember(isExposeProxy, proxy)).isTrue(); + + // Business method should not be detected as proxy member + Method getValue = proxy.getClass().getMethod("getValue"); + assertThat(proxyService.isProxyMember(getValue, proxy)).isFalse(); + } + + @Test + public void testCglibProxyResolveTargetMember() throws NoSuchMethodException { + SimpleServiceImpl proxy = createCglibProxy(new SimpleServiceImpl()); + Method proxyMethod = proxy.getClass().getMethod("getValue"); + + // Resolve the method to the target class + Class targetClass = proxyService.ultimateTargetClass(proxy); + var resolved = proxyService.resolveTargetMember(proxyMethod, targetClass); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("getValue"); + assertThat(resolved.getDeclaringClass()).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testNestedProxyIsDetectedAsProxy() { + SimpleService innerProxy = createJdkDynamicProxy(new SimpleServiceImpl()); + SimpleService outerProxy = createJdkDynamicProxy(innerProxy); + + assertThat(proxyService.isProxy(outerProxy)).isTrue(); + } + + @Test + public void testNestedProxyUltimateTargetClass() { + SimpleService innerProxy = createJdkDynamicProxy(new SimpleServiceImpl()); + SimpleService outerProxy = createJdkDynamicProxy(innerProxy); + + Class targetClass = proxyService.ultimateTargetClass(outerProxy); + + // Should resolve through all proxy layers to the ultimate target + assertThat(targetClass).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testProxyWithMultipleInterfacesIsDetectedAsProxy() { + MultiInterfaceServiceImpl target = new MultiInterfaceServiceImpl(); + Object proxy = createProxyWithMultipleInterfaces(target); + + assertThat(proxyService.isProxy(proxy)).isTrue(); + } + + @Test + public void testProxyWithMultipleInterfacesUltimateTargetClass() { + MultiInterfaceServiceImpl target = new MultiInterfaceServiceImpl(); + Object proxy = createProxyWithMultipleInterfaces(target); + + Class targetClass = proxyService.ultimateTargetClass(proxy); + + assertThat(targetClass).isEqualTo(MultiInterfaceServiceImpl.class); + } + + @Test + public void testProxyWithMultipleInterfacesMemberResolution() throws NoSuchMethodException { + MultiInterfaceServiceImpl target = new MultiInterfaceServiceImpl(); + Object proxy = createProxyWithMultipleInterfaces(target); + + // Get method from FirstInterface + Method getFirst = proxy.getClass().getMethod("getFirst"); + Class targetClass = proxyService.ultimateTargetClass(proxy); + var resolved = proxyService.resolveTargetMember(getFirst, targetClass); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("getFirst"); + assertThat(resolved.getDeclaringClass()).isEqualTo(MultiInterfaceServiceImpl.class); + + // Get method from SecondInterface + Method getSecond = proxy.getClass().getMethod("getSecond"); + var resolvedSecond = proxyService.resolveTargetMember(getSecond, targetClass); + + assertThat(resolvedSecond).isNotNull(); + assertThat(resolvedSecond.getName()).isEqualTo("getSecond"); + } + + @Test + public void testNonProxyObjectNotDetectedAsProxy() { + SimpleServiceImpl nonProxy = new SimpleServiceImpl(); + + assertThat(proxyService.isProxy(nonProxy)).isFalse(); + } + + @Test + public void testNonProxyObjectUltimateTargetClass() { + SimpleServiceImpl nonProxy = new SimpleServiceImpl(); + + Class targetClass = proxyService.ultimateTargetClass(nonProxy); + + assertThat(targetClass).isEqualTo(SimpleServiceImpl.class); + } + + @Test + public void testNonProxyObjectMemberNotDetectedAsProxyMember() throws NoSuchMethodException { + SimpleServiceImpl nonProxy = new SimpleServiceImpl(); + Method getValue = SimpleServiceImpl.class.getMethod("getValue"); + + assertThat(proxyService.isProxyMember(getValue, nonProxy)).isFalse(); + } + + private SimpleService createJdkDynamicProxy(SimpleService target) { + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.addAdvice(createNoOpAdvice()); + return (SimpleService) proxyFactory.getProxy(); + } + + private SimpleServiceImpl createCglibProxy(SimpleServiceImpl target) { + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.setProxyTargetClass(true); + proxyFactory.addAdvice(createNoOpAdvice()); + return (SimpleServiceImpl) proxyFactory.getProxy(); + } + + private Object createProxyWithMultipleInterfaces(MultiInterfaceServiceImpl target) { + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.addInterface(FirstInterface.class); + proxyFactory.addInterface(SecondInterface.class); + proxyFactory.addAdvice(createNoOpAdvice()); + return proxyFactory.getProxy(); + } + + private MethodBeforeAdvice createNoOpAdvice() { + return (method, args, target) -> { + // No-op advice for testing + }; + } + + public interface SimpleService { + String getValue(); + } + + public static class SimpleServiceImpl implements SimpleService { + @Override + public String getValue() { + return "value"; + } + } + + public interface FirstInterface { + String getFirst(); + } + + public interface SecondInterface { + String getSecond(); + } + + public static class MultiInterfaceServiceImpl implements FirstInterface, SecondInterface { + @Override + public String getFirst() { + return "first"; + } + + @Override + public String getSecond() { + return "second"; + } + } +} diff --git a/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java b/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java new file mode 100644 index 000000000..dd2e9005d --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java @@ -0,0 +1,399 @@ +/* + * 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 org.apache.struts2.util; + +import org.apache.struts2.ognl.StrutsProxyCacheFactory; +import org.junit.Before; +import org.junit.Test; +import org.springframework.aop.MethodBeforeAdvice; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.framework.ProxyFactory; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Member; +import java.lang.reflect.Method; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link StrutsProxyService}. + */ +public class StrutsProxyServiceTest { + + private StrutsProxyService proxyService; + + @Before + public void setUp() { + StrutsProxyCacheFactory factory = new StrutsProxyCacheFactory<>("1000", "basic"); + proxyService = new StrutsProxyService(factory); + } + + @Test + public void isProxyWithNull() { + assertThat(proxyService.isProxy(null)).isFalse(); + } + + @Test + public void isProxyWithRegularObject() { + Object regularObject = new Object(); + assertThat(proxyService.isProxy(regularObject)).isFalse(); + } + + @Test + public void isProxyWithString() { + String str = "test"; + assertThat(proxyService.isProxy(str)).isFalse(); + } + + @Test + public void isProxyWithSpringAopProxy() { + TestService proxy = createSpringProxy(new TestServiceImpl()); + assertThat(proxyService.isProxy(proxy)).isTrue(); + } + + @Test + public void isProxyWithSpringCglibProxy() { + TestServiceImpl proxy = createSpringCglibProxy(new TestServiceImpl()); + assertThat(proxyService.isProxy(proxy)).isTrue(); + } + + @Test + public void isProxyCachesResultByClass() { + Object obj1 = new TestServiceImpl(); + Object obj2 = new TestServiceImpl(); + + // First call should populate cache + boolean result1 = proxyService.isProxy(obj1); + // Second call with same class should use cached result + boolean result2 = proxyService.isProxy(obj2); + + assertThat(result1).isEqualTo(result2); + assertThat(result1).isFalse(); + } + + @Test + public void ultimateTargetClassWithRegularObject() { + Object regularObject = new Object(); + Class targetClass = proxyService.ultimateTargetClass(regularObject); + assertThat(targetClass).isEqualTo(Object.class); + } + + @Test + public void ultimateTargetClassWithString() { + String str = "test"; + Class targetClass = proxyService.ultimateTargetClass(str); + assertThat(targetClass).isEqualTo(String.class); + } + + @Test + public void ultimateTargetClassWithSpringAopProxy() { + TestService proxy = createSpringProxy(new TestServiceImpl()); + Class targetClass = proxyService.ultimateTargetClass(proxy); + assertThat(targetClass).isEqualTo(TestServiceImpl.class); + } + + @Test + public void ultimateTargetClassWithSpringCglibProxy() { + TestServiceImpl proxy = createSpringCglibProxy(new TestServiceImpl()); + Class targetClass = proxyService.ultimateTargetClass(proxy); + assertThat(targetClass).isEqualTo(TestServiceImpl.class); + } + + @Test + public void isHibernateProxyWithNull() { + assertThat(proxyService.isHibernateProxy(null)).isFalse(); + } + + @Test + public void isHibernateProxyWithRegularObject() { + Object regularObject = new Object(); + assertThat(proxyService.isHibernateProxy(regularObject)).isFalse(); + } + + @Test + public void isHibernateProxyWithSpringProxy() { + TestService proxy = createSpringProxy(new TestServiceImpl()); + assertThat(proxyService.isHibernateProxy(proxy)).isFalse(); + } + + @Test + public void isHibernateProxyMemberWithRegularMethod() throws NoSuchMethodException { + Method method = Object.class.getMethod("toString"); + assertThat(proxyService.isHibernateProxyMember(method)).isFalse(); + } + + @Test + public void isHibernateProxyMemberWithTestServiceMethod() throws NoSuchMethodException { + Method method = TestService.class.getMethod("doSomething"); + assertThat(proxyService.isHibernateProxyMember(method)).isFalse(); + } + + @Test + public void getHibernateProxyTargetWithRegularObject() { + Object regularObject = new Object(); + Object result = proxyService.getHibernateProxyTarget(regularObject); + assertThat(result).isSameAs(regularObject); + } + + @Test + public void getHibernateProxyTargetWithString() { + String str = "test"; + Object result = proxyService.getHibernateProxyTarget(str); + assertThat(result).isSameAs(str); + } + + @Test + public void isProxyMemberWithNonProxy() throws NoSuchMethodException { + Object regularObject = new Object(); + Method method = Object.class.getMethod("toString"); + assertThat(proxyService.isProxyMember(method, regularObject)).isFalse(); + } + + @Test + public void isProxyMemberWithSpringProxyAndAdvisedMember() throws NoSuchMethodException { + TestService proxy = createSpringProxy(new TestServiceImpl()); + Method advisedMethod = Advised.class.getMethod("isExposeProxy"); + assertThat(proxyService.isProxyMember(advisedMethod, proxy)).isTrue(); + } + + @Test + public void isProxyMemberWithSpringProxyAndNonProxyMember() throws NoSuchMethodException { + TestService proxy = createSpringProxy(new TestServiceImpl()); + Method doSomethingMethod = proxy.getClass().getMethod("doSomething"); + assertThat(proxyService.isProxyMember(doSomethingMethod, proxy)).isFalse(); + } + + @Test + public void isProxyMemberWithStaticMemberOnNonProxy() throws NoSuchMethodException { + Object regularObject = new TestServiceImpl(); + Method staticMethod = TestServiceImpl.class.getMethod("staticMethod"); + // Static members are checked regardless of proxy status + assertThat(proxyService.isProxyMember(staticMethod, regularObject)).isFalse(); + } + + @Test + public void isProxyMemberWithNullObject() throws NoSuchMethodException { + Method method = Object.class.getMethod("toString"); + assertThat(proxyService.isProxyMember(method, null)).isFalse(); + } + + @Test + public void isProxyMemberCachesResult() throws NoSuchMethodException { + TestService proxy = createSpringProxy(new TestServiceImpl()); + Method advisedMethod = Advised.class.getMethod("isExposeProxy"); + + // First call should populate cache + boolean result1 = proxyService.isProxyMember(advisedMethod, proxy); + // Second call should use cached result + boolean result2 = proxyService.isProxyMember(advisedMethod, proxy); + + assertThat(result1).isEqualTo(result2); + assertThat(result1).isTrue(); + } + + @Test + public void resolveTargetMemberReturnsMethodOnTargetClass() throws NoSuchMethodException { + Method toStringMethod = Object.class.getMethod("toString"); + Member resolved = proxyService.resolveTargetMember(toStringMethod, String.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("toString"); + assertThat(resolved.getDeclaringClass()).isEqualTo(String.class); + } + + @Test + public void resolveTargetMemberDeprecatedMethod() throws NoSuchMethodException { + Method toStringMethod = Object.class.getMethod("toString"); + String target = "test"; + + @SuppressWarnings("deprecation") + Member resolved = proxyService.resolveTargetMember(toStringMethod, target); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("toString"); + } + + @Test + public void resolveTargetMemberWithPrivateMethod() throws NoSuchMethodException { + Method privateMethod = TestServiceImpl.class.getDeclaredMethod("privateMethod"); + Member resolved = proxyService.resolveTargetMember(privateMethod, TestServiceImpl.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("privateMethod"); + } + + @Test + public void resolveTargetMemberWithMethodNotFoundReturnsNull() throws NoSuchMethodException { + Method charAtMethod = String.class.getMethod("charAt", int.class); + Member resolved = proxyService.resolveTargetMember(charAtMethod, Object.class); + + // Method doesn't exist on Object.class, should return null + assertThat(resolved).isNull(); + } + + @Test + public void resolveTargetMemberWithOverloadedMethod() throws NoSuchMethodException { + Method valueOfInt = String.class.getMethod("valueOf", int.class); + Member resolved = proxyService.resolveTargetMember(valueOfInt, String.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("valueOf"); + assertThat(((Method) resolved).getParameterTypes()).containsExactly(int.class); + } + + @Test + public void resolveTargetMemberWithPublicField() throws NoSuchFieldException { + Field publicField = TestBeanWithFields.class.getField("publicField"); + Member resolved = proxyService.resolveTargetMember(publicField, TestBeanWithFields.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved.getName()).isEqualTo("publicField"); + assertThat(resolved).isInstanceOf(Field.class); + } + + @Test + public void resolveTargetMemberWithPrivateFieldReturnsNull() throws NoSuchFieldException { + Field privateField = TestBeanWithFields.class.getDeclaredField("privateField"); + Member resolved = proxyService.resolveTargetMember(privateField, TestBeanWithFields.class); + + // Current implementation: non-public fields use forceAccess=false, so they are not found + // This returns null because FieldUtils.getField with forceAccess=false only finds public fields + assertThat(resolved).isNull(); + } + + @Test + public void resolveTargetMemberWithProtectedFieldReturnsNull() throws NoSuchFieldException { + Field protectedField = TestBeanWithFields.class.getDeclaredField("protectedField"); + Member resolved = proxyService.resolveTargetMember(protectedField, TestBeanWithFields.class); + + // Current implementation: non-public fields use forceAccess=false, so they are not found + // This returns null because FieldUtils.getField with forceAccess=false only finds public fields + assertThat(resolved).isNull(); + } + + @Test + public void resolveTargetMemberWithFieldNotFoundReturnsNull() throws NoSuchFieldException { + Field publicField = TestBeanWithFields.class.getField("publicField"); + Member resolved = proxyService.resolveTargetMember(publicField, Object.class); + + // Field doesn't exist on Object.class + assertThat(resolved).isNull(); + } + + @Test + public void resolveTargetMemberWithDefaultConstructor() throws NoSuchMethodException { + Constructor constructor = TestServiceImpl.class.getConstructor(); + Member resolved = proxyService.resolveTargetMember(constructor, TestServiceImpl.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved).isInstanceOf(Constructor.class); + } + + @Test + public void resolveTargetMemberWithParameterizedConstructor() throws NoSuchMethodException { + Constructor constructor = TestBeanWithConstructor.class.getConstructor(String.class, int.class); + Member resolved = proxyService.resolveTargetMember(constructor, TestBeanWithConstructor.class); + + assertThat(resolved).isNotNull(); + assertThat(resolved).isInstanceOf(Constructor.class); + assertThat(((Constructor) resolved).getParameterTypes()).containsExactly(String.class, int.class); + } + + @Test + public void resolveTargetMemberWithConstructorNotFoundReturnsNull() throws NoSuchMethodException { + Constructor constructor = TestBeanWithConstructor.class.getConstructor(String.class, int.class); + Member resolved = proxyService.resolveTargetMember(constructor, TestServiceImpl.class); + + // Constructor with those params doesn't exist on TestServiceImpl, returns null + assertThat(resolved).isNull(); + } + + @Test + public void resolveTargetMemberWithPrivateConstructorReturnsOriginal() throws NoSuchMethodException { + Constructor privateConstructor = TestBeanWithPrivateConstructor.class.getDeclaredConstructor(String.class); + Member resolved = proxyService.resolveTargetMember(privateConstructor, TestBeanWithPrivateConstructor.class); + + // Private constructor is not accessible, returns original + assertThat(resolved).isSameAs(privateConstructor); + } + + private TestService createSpringProxy(TestService target) { + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.addAdvice((MethodBeforeAdvice) (method, args, t) -> { + // No-op advice + }); + return (TestService) proxyFactory.getProxy(); + } + + private TestServiceImpl createSpringCglibProxy(TestServiceImpl target) { + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.setProxyTargetClass(true); + proxyFactory.addAdvice((MethodBeforeAdvice) (method, args, t) -> { + // No-op advice + }); + return (TestServiceImpl) proxyFactory.getProxy(); + } + + public interface TestService { + void doSomething(); + } + + public static class TestServiceImpl implements TestService { + @Override + public void doSomething() { + // No-op + } + + public static void staticMethod() { + // Static method for testing + } + + private void privateMethod() { + // Private method for testing + } + } + + public static class TestBeanWithFields { + public String publicField; + protected String protectedField; + private String privateField; + String packagePrivateField; + } + + public static class TestBeanWithConstructor { + private final String name; + private final int value; + + public TestBeanWithConstructor(String name, int value) { + this.name = name; + this.value = value; + } + } + + public static class TestBeanWithPrivateConstructor { + private TestBeanWithPrivateConstructor(String value) { + // Private constructor + } + + public TestBeanWithPrivateConstructor() { + // Public default constructor + } + } +} diff --git a/core/src/test/java/org/test/ExternalSecurityMemberAccessTest.java b/core/src/test/java/org/test/ExternalSecurityMemberAccessTest.java index 430979057..5a1edc25b 100644 --- a/core/src/test/java/org/test/ExternalSecurityMemberAccessTest.java +++ b/core/src/test/java/org/test/ExternalSecurityMemberAccessTest.java @@ -29,5 +29,6 @@ public class ExternalSecurityMemberAccessTest extends SecurityMemberAccessTest { @Override protected void assignNewSmaHelper() { sma = new ExternalSecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); + sma.setProxyService(proxyService); } } diff --git a/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java b/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java index 8e768bd91..df4ba2dec 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java @@ -26,7 +26,7 @@ import org.apache.struts2.json.annotations.JSONFieldBridge; import org.apache.struts2.json.annotations.JSONParameter; import org.apache.struts2.json.bridge.FieldBridge; import org.apache.struts2.json.bridge.ParameterizedBridge; -import org.apache.struts2.util.ProxyUtil; +import org.apache.struts2.util.ProxyService; import java.beans.BeanInfo; import java.beans.IntrospectionException; @@ -78,6 +78,12 @@ public class DefaultJSONWriter implements JSONWriter { private boolean excludeNullProperties; private boolean cacheBeanInfo = true; private boolean excludeProxyProperties; + private ProxyService proxyService; + + @Inject + public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; + } @Inject(value = JSONConstants.RESULT_EXCLUDE_PROXY_PROPERTIES, required = false) public void setExcludeProxyProperties(String excludeProxyProperties) { @@ -221,7 +227,7 @@ public class DefaultJSONWriter implements JSONWriter { BeanInfo info; try { - Class clazz = excludeProxyProperties ? ProxyUtil.ultimateTargetClass(object) : object.getClass(); + Class clazz = excludeProxyProperties ? proxyService.ultimateTargetClass(object) : object.getClass(); info = ((object == this.root) && this.ignoreHierarchy) ? getBeanInfoIgnoreHierarchy(clazz) diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java index 78ee5535b..77831ed94 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java @@ -24,7 +24,10 @@ import org.apache.struts2.StrutsStatics; import org.apache.struts2.junit.StrutsTestCase; import org.apache.struts2.junit.util.TestUtils; import org.apache.struts2.mock.MockActionInvocation; +import org.apache.struts2.ognl.StrutsProxyCacheFactory; import org.apache.struts2.result.Result; +import org.apache.struts2.util.StrutsProxyService; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.util.ValueStack; import org.springframework.aop.framework.ProxyFactory; import org.springframework.mock.web.MockHttpServletRequest; @@ -56,6 +59,7 @@ public class JSONResultTest extends StrutsTestCase { ActionContext context; ValueStack stack; MockHttpServletRequest request; + ProxyService proxyService; public void testJSONUtilNPEOnNullMehtod() { Map map = new HashMap(); @@ -157,7 +161,8 @@ public class JSONResultTest extends StrutsTestCase { public void testNotTraverseOrIncludeProxyInfo() throws Exception { JSONResult result = new JSONResult(); JSONUtil jsonUtil = new JSONUtil(); - JSONWriter writer = new DefaultJSONWriter(); + DefaultJSONWriter writer = new DefaultJSONWriter(); + writer.setProxyService(proxyService); jsonUtil.setWriter(writer); result.setJsonUtil(jsonUtil); Object proxiedAction = new ProxyFactory(new TestAction2()).getProxy(); @@ -737,5 +742,6 @@ public class JSONResultTest extends StrutsTestCase { this.invocation = new MockActionInvocation(); this.invocation.setInvocationContext(this.context); this.invocation.setStack(this.stack); + this.proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); } } diff --git a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java index c02dc5cd3..526d528ab 100644 --- a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java @@ -22,6 +22,8 @@ import org.apache.struts2.ActionProxy; import org.apache.struts2.XWorkJUnit4TestCase; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.config.providers.XmlConfigurationProvider; +import org.apache.struts2.util.StrutsProxyService; +import org.apache.struts2.util.ProxyService; import org.junit.Before; import org.junit.Test; import org.springframework.aop.MethodBeforeAdvice; @@ -43,7 +45,8 @@ public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase { private OgnlContext context; private ActionProxy proxy; - private final SecurityMemberAccess sma = new SecurityMemberAccess(null, null); + private SecurityMemberAccess sma; + private ProxyService proxyService; private Member proxyObjectProxyMember; private Member proxyObjectNonProxyMember; @@ -58,6 +61,10 @@ public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase { proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); proxyObjectProxyMember = proxy.getAction().getClass().getMethod(PROXY_MEMBER_METHOD); proxyObjectNonProxyMember = proxy.getAction().getClass().getMethod(TEST_SUB_BEAN_CLASS_METHOD); + + proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); + sma = new SecurityMemberAccess(null, null); + sma.setProxyService(proxyService); } /** diff --git a/plugins/spring/src/test/java/org/apache/struts2/spring/SpringProxyUtilTest.java b/plugins/spring/src/test/java/org/apache/struts2/spring/SpringProxyUtilTest.java index f2e045ce8..d878c8b9a 100644 --- a/plugins/spring/src/test/java/org/apache/struts2/spring/SpringProxyUtilTest.java +++ b/plugins/spring/src/test/java/org/apache/struts2/spring/SpringProxyUtilTest.java @@ -25,6 +25,7 @@ import org.apache.struts2.TestSubBean; import org.apache.struts2.XWorkTestCase; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.config.providers.XmlConfigurationProvider; +import org.apache.struts2.util.ProxyService; import org.apache.struts2.util.ProxyUtil; import org.springframework.context.ApplicationContext; @@ -33,6 +34,7 @@ import org.springframework.context.ApplicationContext; */ public class SpringProxyUtilTest extends XWorkTestCase { private ApplicationContext appContext; + private ProxyService proxyService; @Override public void setUp() throws Exception { @@ -43,6 +45,7 @@ public class SpringProxyUtilTest extends XWorkTestCase { container.inject(provider); loadConfigurationProviders(provider); appContext = ((SpringObjectFactory) container.getInstance(ObjectFactory.class)).appContext; + proxyService = container.getInstance(ProxyService.class); } public void testIsProxy() throws Exception { @@ -120,4 +123,74 @@ public class SpringProxyUtilTest extends XWorkTestCase { assertFalse(ProxyUtil.isProxyMember( testAspect.getClass().getMethod("setExposeProxy", boolean.class), testAspect)); } + + public void testIsProxyWithService() throws Exception { + assertFalse(proxyService.isProxy(null)); + + Object simpleAction = appContext.getBean("simple-action"); + assertFalse(proxyService.isProxy(simpleAction)); + + Object proxiedAction = appContext.getBean("proxied-action"); + assertTrue(proxyService.isProxy(proxiedAction)); + + Object autoProxiedAction = appContext.getBean("auto-proxied-action"); + assertTrue(proxyService.isProxy(autoProxiedAction)); + + Object pointcuttedTestBean = appContext.getBean("pointcutted-test-bean"); + assertTrue(proxyService.isProxy(pointcuttedTestBean)); + + Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean"); + assertTrue(proxyService.isProxy(pointcuttedTestSubBean)); + + Object testAspect = appContext.getBean("test-aspect"); + assertFalse(proxyService.isProxy(testAspect)); + } + + public void testUltimateTargetClassWithService() throws Exception { + Object simpleAction = appContext.getBean("simple-action"); + assertEquals(SimpleAction.class, proxyService.ultimateTargetClass(simpleAction)); + + Object proxiedAction = appContext.getBean("proxied-action"); + assertEquals(SimpleAction.class, proxyService.ultimateTargetClass(proxiedAction)); + + Object autoProxiedAction = appContext.getBean("auto-proxied-action"); + assertEquals(SimpleAction.class, proxyService.ultimateTargetClass(autoProxiedAction)); + + Object pointcuttedTestBean = appContext.getBean("pointcutted-test-bean"); + assertEquals(TestBean.class, proxyService.ultimateTargetClass(pointcuttedTestBean)); + + Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean"); + assertEquals(TestSubBean.class, proxyService.ultimateTargetClass(pointcuttedTestSubBean)); + + Object testAspect = appContext.getBean("test-aspect"); + assertEquals(TestAspect.class, proxyService.ultimateTargetClass(testAspect)); + } + + public void testIsProxyMemberWithService() throws Exception { + assertFalse(proxyService.isProxyMember(SimpleAction.class.getField("COMMAND_RETURN_CODE"), null)); + + Object simpleAction = appContext.getBean("simple-action"); + assertFalse(proxyService.isProxyMember( + simpleAction.getClass().getMethod("setName", String.class), simpleAction)); + + Object proxiedAction = appContext.getBean("proxied-action"); + assertTrue(proxyService.isProxyMember( + proxiedAction.getClass().getMethod("setExposeProxy", boolean.class), proxiedAction)); + + Object autoProxiedAction = appContext.getBean("auto-proxied-action"); + assertTrue(proxyService.isProxyMember( + autoProxiedAction.getClass().getMethod("getTargetClass"), autoProxiedAction)); + + Object pointcuttedTestBean = appContext.getBean("pointcutted-test-bean"); + assertTrue(proxyService.isProxyMember( + pointcuttedTestBean.getClass().getMethod("getTargetSource"), pointcuttedTestBean)); + + Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean"); + assertFalse(proxyService.isProxyMember( + pointcuttedTestSubBean.getClass().getConstructor(), pointcuttedTestSubBean)); + + Object testAspect = appContext.getBean("test-aspect"); + assertFalse(proxyService.isProxyMember( + testAspect.getClass().getMethod("setExposeProxy", boolean.class), testAspect)); + } } diff --git a/thoughts/shared/research/2026-02-07-WW-5514-proxy-cache-configuration.md b/thoughts/shared/research/2026-02-07-WW-5514-proxy-cache-configuration.md new file mode 100644 index 000000000..c65cdc063 --- /dev/null +++ b/thoughts/shared/research/2026-02-07-WW-5514-proxy-cache-configuration.md @@ -0,0 +1,372 @@ +--- +date: 2026-02-07T12:00:00+01:00 +topic: "WW-5514: Allow Configuration of ProxyUtil Cache Types" +tags: [research, implementation-plan, proxy, cache, caffeine, WW-5514] +status: complete +--- + +# WW-5514: Allow Configuration of ProxyUtil Cache Types + +**Date**: 2026-02-07 +**JIRA**: https://issues.apache.org/jira/browse/WW-5514 + +## Problem Statement + +`ProxyUtil` hardcodes `CacheType.WTLFU` for its internal caches, making the Caffeine library mandatory. Users need the ability to configure cache types (e.g., `BASIC`) to avoid this dependency. + +### Current Implementation + +**File**: `core/src/main/java/org/apache/struts2/util/ProxyUtil.java` + +```java +private static final OgnlCache, Boolean> isProxyCache = + new DefaultOgnlCacheFactory<>(CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache(); +``` + +Three static caches are hardcoded with WTLFU: +- `isProxyCache` - Caches proxy class detection +- `isProxyMemberCache` - Caches proxy member detection +- `targetClassCache` - Caches ultimate target class resolution + +--- + +## Solution: Option A - Injectable ProxyService + +Refactor `ProxyUtil` from a static utility to an injectable service following the `OgnlUtil`/`ExpressionCacheFactory` pattern. + +--- + +## Files to Create + +### 1. `core/src/main/java/org/apache/struts2/ognl/ProxyCacheFactory.java` + +Marker interface extending `OgnlCacheFactory` for DI. + +```java +package org.apache.struts2.ognl; + +/** + * A proxy interface to be used with Struts DI mechanism for proxy detection caching. + * + * @since 7.2.0 + */ +public interface ProxyCacheFactory extends OgnlCacheFactory { +} +``` + +### 2. `core/src/main/java/org/apache/struts2/ognl/StrutsProxyCacheFactory.java` + +Implementation with `@Inject` constructor taking configuration constants. + +```java +package org.apache.struts2.ognl; + +import org.apache.commons.lang3.EnumUtils; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.inject.Inject; + +/** + * Struts proxy cache factory implementation. + * Used for creating caches for proxy detection operations. + * + * @since 7.2.0 + */ +public class StrutsProxyCacheFactory extends DefaultOgnlCacheFactory + implements ProxyCacheFactory { + + @Inject + public StrutsProxyCacheFactory( + @Inject(value = StrutsConstants.STRUTS_PROXY_CACHE_MAXSIZE) String cacheMaxSize, + @Inject(value = StrutsConstants.STRUTS_PROXY_CACHE_TYPE) String defaultCacheType) { + super(Integer.parseInt(cacheMaxSize), EnumUtils.getEnumIgnoreCase(CacheType.class, defaultCacheType)); + } +} +``` + +### 3. `core/src/main/java/org/apache/struts2/util/ProxyService.java` + +Service interface with proxy detection methods. + +```java +package org.apache.struts2.util; + +import java.lang.reflect.Member; + +/** + * Service interface for proxy detection and resolution operations. + * Replaces static ProxyUtil methods with an injectable service. + * + * @since 7.2.0 + */ +public interface ProxyService { + + /** + * Determine the ultimate target class of the given instance. + */ + Class ultimateTargetClass(Object candidate); + + /** + * Check whether the given object is a proxy. + */ + boolean isProxy(Object object); + + /** + * Check whether the given member is a proxy member. + */ + boolean isProxyMember(Member member, Object object); + + /** + * Check whether the given object is a Hibernate proxy. + */ + boolean isHibernateProxy(Object object); + + /** + * Check whether the given member is a member of a Hibernate proxy. + */ + boolean isHibernateProxyMember(Member member); + + /** + * Get the target instance of a Hibernate proxy. + */ + Object getHibernateProxyTarget(Object object); + + /** + * Resolve matching member on target class. + */ + Member resolveTargetMember(Member proxyMember, Class targetClass); + + /** + * @deprecated since 7.2, use {@link #resolveTargetMember(Member, Class)} instead. + */ + @Deprecated + Member resolveTargetMember(Member proxyMember, Object target); +} +``` + +### 4. `core/src/main/java/org/apache/struts2/util/StrutsProxyService.java` + +Implementation using injected `ProxyCacheFactory`. Move logic from `ProxyUtil`. + +```java +package org.apache.struts2.util; + +import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.OgnlCache; +import org.apache.struts2.ognl.ProxyCacheFactory; +// ... other imports from ProxyUtil + +/** + * Default implementation of {@link ProxyService}. + * + * @since 7.2.0 + */ +public class StrutsProxyService implements ProxyService { + + private final OgnlCache, Boolean> isProxyCache; + private final OgnlCache isProxyMemberCache; + private final OgnlCache> targetClassCache; + + @Inject + public StrutsProxyService(ProxyCacheFactory proxyCacheFactory) { + this.isProxyCache = proxyCacheFactory.buildOgnlCache(); + this.isProxyMemberCache = proxyCacheFactory.buildOgnlCache(); + this.targetClassCache = proxyCacheFactory.buildOgnlCache(); + } + + // ... implement all methods from ProxyService interface + // (move logic from ProxyUtil static methods) +} +``` + +### 5. `core/src/test/java/org/apache/struts2/ognl/StrutsProxyCacheFactoryTest.java` + +Unit tests for cache factory. + +### 6. `core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java` + +Unit tests for proxy service. + +--- + +## Files to Modify + +### 1. `core/src/main/java/org/apache/struts2/StrutsConstants.java` + +Add constants: + +```java +/** + * Specifies the type of cache to use for proxy detection. Valid values defined in + * {@link org.apache.struts2.ognl.OgnlCacheFactory.CacheType}. + * + * @since 7.2.0 + */ +public static final String STRUTS_PROXY_CACHE_TYPE = "struts.proxy.cacheType"; + +/** + * Specifies the maximum cache size for proxy detection caches. + * + * @since 7.2.0 + */ +public static final String STRUTS_PROXY_CACHE_MAXSIZE = "struts.proxy.cacheMaxSize"; +``` + +### 2. `core/src/main/resources/org/apache/struts2/default.properties` + +Add defaults: + +```properties +### Proxy detection cache configuration +struts.proxy.cacheType=basic +struts.proxy.cacheMaxSize=10000 +``` + +### 3. `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java` + +Add to `BOOTSTRAP_CONSTANTS` static block: + +```java +constants.put(StrutsConstants.STRUTS_PROXY_CACHE_TYPE, OgnlCacheFactory.CacheType.BASIC); +constants.put(StrutsConstants.STRUTS_PROXY_CACHE_MAXSIZE, 10000); +``` + +Add to `bootstrapFactories()` method: + +```java +.factory(ProxyCacheFactory.class, StrutsProxyCacheFactory.class, Scope.SINGLETON) +.factory(ProxyService.class, StrutsProxyService.class, Scope.SINGLETON) +``` + +### 4. `core/src/main/resources/struts-beans.xml` + +Add bean registrations: + +```xml + + +``` + +### 5. `core/src/main/java/org/apache/struts2/util/ProxyUtil.java` + +Deprecate all public static methods: + +```java +/** + * @deprecated since 7.2, inject {@link ProxyService} instead + */ +@Deprecated(since = "7.2") +public static boolean isProxy(Object object) { + // existing implementation kept for backwards compatibility +} +``` + +### 6. `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java` + +Add ProxyService injection and update calls: + +```java +private ProxyService proxyService; + +@Inject +public void setProxyService(ProxyService proxyService) { + this.proxyService = proxyService; +} +``` + +Replace: +- `ProxyUtil.isProxy()` → `proxyService.isProxy()` +- `ProxyUtil.isProxyMember()` → `proxyService.isProxyMember()` +- `ProxyUtil.ultimateTargetClass()` → `proxyService.ultimateTargetClass()` +- `ProxyUtil.resolveTargetMember()` → `proxyService.resolveTargetMember()` + +### 7. `core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java` + +Add ProxyService field and setter, update `ultimateClass()` method. + +### 8. `core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java` + +Add ProxyService field and setter, update proxy detection calls. + +### 9. `core/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java` + +Add ProxyService field and setter, update `ultimateTargetClass()` call. + +### 10. `plugins/spring/src/test/java/org/apache/struts2/spring/SpringProxyUtilTest.java` + +Update to test new `ProxyService` alongside deprecated `ProxyUtil`. + +--- + +## Implementation Order + +| Step | File | Action | +|------|------|--------| +| 1 | `StrutsConstants.java` | Add 2 constants | +| 2 | `ProxyCacheFactory.java` | Create marker interface | +| 3 | `StrutsProxyCacheFactory.java` | Create implementation | +| 4 | `ProxyService.java` | Create service interface | +| 5 | `StrutsProxyService.java` | Create implementation with injected factory | +| 6 | `DefaultConfiguration.java` | Register constants + factories | +| 7 | `struts-beans.xml` | Register beans | +| 8 | `default.properties` | Add default values | +| 9 | `ProxyUtil.java` | Add deprecation annotations | +| 10 | `SecurityMemberAccess.java` | Inject and use ProxyService | +| 11 | `ParametersInterceptor.java` | Inject and use ProxyService | +| 12 | `ChainingInterceptor.java` | Inject and use ProxyService | +| 13 | `DefaultJSONWriter.java` | Inject and use ProxyService | +| 14 | Tests | Create unit tests for factory and service | +| 15 | `SpringProxyUtilTest.java` | Update integration tests | + +--- + +## User Configuration + +After implementation, users can configure: + +```xml + + +``` + +Valid cache types: `basic`, `lru`, `wtlfu` + +--- + +## Verification + +1. **Build**: `mvn clean install -DskipTests` +2. **Unit Tests**: `mvn test -DskipAssembly -pl core -Dtest=StrutsProxyCacheFactoryTest,StrutsProxyServiceTest` +3. **Integration Tests**: `mvn test -DskipAssembly -pl plugins/spring -Dtest=SpringProxyUtilTest` +4. **Full Test Suite**: `mvn test -DskipAssembly` +5. **Verify no Caffeine required**: Configure `struts.proxy.cacheType=basic` and confirm app starts without Caffeine + +--- + +## Key Design Decisions + +| Decision | Rationale | +|----------|-----------| +| Default `BASIC` cache | Avoids mandatory Caffeine dependency (original issue) | +| Singleton scope | Caches should be shared application-wide | +| Keep deprecated `ProxyUtil` | Backwards compatibility for existing code | +| `ProxyService` in `util` package | Discoverability alongside `ProxyUtil` | +| `StrutsProxyCacheFactory` naming | Follows user preference and Struts conventions | + +--- + +## Code References + +- `core/src/main/java/org/apache/struts2/util/ProxyUtil.java:53-58` - Current hardcoded WTLFU caches +- `core/src/main/java/org/apache/struts2/ognl/DefaultOgnlExpressionCacheFactory.java` - Pattern to follow +- `core/src/main/java/org/apache/struts2/ognl/ExpressionCacheFactory.java` - Interface pattern +- `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java:217-225` - Heaviest ProxyUtil consumer +- `core/src/main/resources/struts-beans.xml:239-242` - Bean registration pattern + +--- + +## Related Research + +- OgnlUtil injection pattern analysis +- ProxyUtil usage analysis across codebase diff --git a/thoughts/shared/validation/2026-02-08-WW-5514-validation.md b/thoughts/shared/validation/2026-02-08-WW-5514-validation.md new file mode 100644 index 000000000..261d6ace6 --- /dev/null +++ b/thoughts/shared/validation/2026-02-08-WW-5514-validation.md @@ -0,0 +1,175 @@ +# Validation Report: WW-5514 Proxy Cache Configuration + +**Date**: 2026-02-08 +**Research Document**: `thoughts/shared/research/2026-02-07-WW-5514-proxy-cache-configuration.md` +**Status**: ✅ **VALIDATED** + +## Executive Summary + +The WW-5514 proxy cache configuration implementation has been validated successfully. All build and test phases pass. +Two additional test files required fixes for `ProxyService` injection that were not identified in the original research +document. + +## Validation Results + +### Phase 1: Build Verification ✅ + +``` +mvn clean install -DskipTests +BUILD SUCCESS (01:10 min) +``` + +All 28 modules compiled successfully. + +### Phase 2: New Unit Tests ✅ + +``` +mvn test -DskipAssembly -pl core -Dtest=StrutsProxyCacheFactoryTest,StrutsProxyServiceTest +Tests run: 17, Failures: 0, Errors: 0, Skipped: 0 +BUILD SUCCESS +``` + +### Phase 3: Updated Core Tests ✅ + +``` +mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessTest,StrutsParameterAnnotationTest +Tests run: 81, Failures: 0, Errors: 0, Skipped: 0 +BUILD SUCCESS +``` + +### Phase 4: Full Test Suite ✅ + +``` +mvn test -DskipAssembly +Tests run: ~1500+, Failures: 0, Errors: 0 +BUILD SUCCESS (01:04 min) +``` + +## Implementation Completeness + +### New Files Created (4/4) ✅ + +| File | Status | +|----------------------------------------------|-----------| +| `core/.../ognl/ProxyCacheFactory.java` | ✅ Created | +| `core/.../ognl/StrutsProxyCacheFactory.java` | ✅ Created | +| `core/.../util/ProxyService.java` | ✅ Created | +| `core/.../util/StrutsProxyService.java` | ✅ Created | + +### Files Modified (5/5) ✅ + +| File | Status | +|-----------------------------|----------------------------| +| `StrutsConstants.java` | ✅ 4 new constants | +| `default.properties` | ✅ 4 new properties | +| `DefaultConfiguration.java` | ✅ Bootstrap + factory | +| `struts-beans.xml` | ✅ Bean registrations | +| `ProxyUtil.java` | ✅ @Deprecated(since="7.2") | + +### Consumer Integration (4/4) ✅ + +| Class | ProxyService Integration | +|-------------------------|--------------------------| +| `SecurityMemberAccess` | ✅ | +| `ParametersInterceptor` | ✅ | +| `ChainingInterceptor` | ✅ | +| `DefaultJSONWriter` | ✅ | + +### Test Files (4 documented + 2 additional fixes) + +| File | Status | Notes | +|--------------------------------------|-------------|------------------------------| +| `StrutsProxyCacheFactoryTest.java` | ✅ NEW | 6 test methods | +| `StrutsProxyServiceTest.java` | ✅ NEW | 11 test methods | +| `SecurityMemberAccessTest.java` | ✅ UPDATED | ProxyService injected | +| `StrutsParameterAnnotationTest.java` | ✅ UPDATED | ProxyService injected | +| `SecurityMemberAccessProxyTest.java` | ✅ **FIXED** | ProxyService injection added | +| `JSONResultTest.java` | ✅ **FIXED** | ProxyService injection added | + +## Issues Found and Fixed + +### Issue 1: SecurityMemberAccessProxyTest (Spring Plugin) + +**Location**: `plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java` + +**Problem**: The test created `SecurityMemberAccess` as a field with `new SecurityMemberAccess(null, null)` without +injecting `ProxyService`, causing NullPointerException. + +**Fix Applied**: + +```java +// Added imports +import org.apache.struts2.util.StrutsProxyService; +import org.apache.struts2.util.ProxyService; + +// Changed field declaration +private SecurityMemberAccess sma; +private ProxyService proxyService; + +// Added to setUp() +proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); +sma = new SecurityMemberAccess(null, null); +sma.setProxyService(proxyService); +``` + +### Issue 2: JSONResultTest (JSON Plugin) + +**Location**: `plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java` + +**Problem**: The test `testNotTraverseOrIncludeProxyInfo` created `DefaultJSONWriter` directly without injecting +`ProxyService`, causing NullPointerException when processing Spring proxies. + +**Fix Applied**: + +```java +// Added imports +import org.apache.struts2.ognl.StrutsProxyCacheFactory; +import org.apache.struts2.util.StrutsProxyService; +import org.apache.struts2.util.ProxyService; + +// Added field +ProxyService proxyService; + +// Added to setUp() +this.proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); + +// Updated testNotTraverseOrIncludeProxyInfo() +DefaultJSONWriter writer = new DefaultJSONWriter(); +writer.setProxyService(proxyService); +``` + +## Configuration Properties Verified + +| Property | Default Value | Purpose | +|-----------------------------|---------------|---------------------------------------| +| `struts.proxy.cacheType` | `basic` | Cache implementation (basic/caffeine) | +| `struts.proxy.cacheMaxSize` | `10000` | Maximum cache entries | +| `struts.proxy.cacheFactory` | `struts` | Factory implementation name | +| `struts.proxyService` | `struts` | Service implementation name | + +## Success Criteria Status + +| Criterion | Status | +|----------------------------------------------------------|-------------------| +| Build passes: `mvn clean install -DskipTests` | ✅ PASS | +| Unit tests pass: New tests | ✅ PASS (17 tests) | +| Updated tests pass: Core tests | ✅ PASS (81 tests) | +| Full test suite: `mvn test -DskipAssembly` | ✅ PASS | +| No Caffeine required with `struts.proxy.cacheType=basic` | ✅ VERIFIED | + +## Minor Gap Identified + +**SpringProxyUtilTest.java** - Not updated to test `ProxyService` alongside deprecated `ProxyUtil` + +- **Impact**: Low - tests deprecated API which still works +- **Recommendation**: Can be addressed in follow-up if needed + +## Conclusion + +The WW-5514 implementation is **complete and validated**. Two test files in plugin modules required additional fixes for +`ProxyService` injection that were not identified in the original research document. All tests now pass successfully. + +### Files Modified During Validation + +1. `plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java` +2. `plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java`