From 55c7fb3dbcd70e6b8b43a6703fe851a618efebc8 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 9 Feb 2026 10:57:22 +0200 Subject: [PATCH 1/2] feat(proxy): WW-5514 add StrutsProxyService for proxy detection and resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../showcase/proxy/LoggingInterceptor.java | 42 ++ .../main/resources/struts-actionchaining.xml | 31 +- apps/showcase/src/main/resources/struts.xml | 95 ++-- .../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 | 40 +- .../struts2/util/StrutsProxyService.java | 198 +++++++++ .../org/apache/struts2/default.properties | 13 + 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 | 412 ++++++++++++++++++ .../ExternalSecurityMemberAccessTest.java | 1 + .../struts2/json/DefaultJSONWriter.java | 10 +- .../apache/struts2/json/JSONResultTest.java | 8 +- .../ognl/SecurityMemberAccessProxyTest.java | 9 +- ...02-07-WW-5514-proxy-cache-configuration.md | 372 ++++++++++++++++ .../2026-02-08-WW-5514-validation.md | 175 ++++++++ 29 files changed, 2044 insertions(+), 79 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 0000000000..4a5dd40a7c --- /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 4f39940f06..ae2a7461c8 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 5c1cf37ff8..5dbe07cee5 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 ef700ef48f..788890326b 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 0000000000..8b3a9794c9 --- /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 39b811068e..84b9fd16e1 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 eda65e527f..c584a4f587 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.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.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 void register(ContainerBuilder builder, LocatableProperties props) { 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 48791c9191..9ec9f9c20e 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.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 static ContainerBuilder bootstrapFactories(ContainerBuilder builder) { .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 2837d4c105..9c18d88696 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.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 @@ *

* * Example code: - * + *

* * 

  * <action name="someAction" class="com.examples.SomeAction">
@@ -114,7 +114,6 @@
  *
* * - * * @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 @@ private void copyStack(ActionInvocation invocation, CompoundRoot root) { } 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 @@ private void copyStack(ActionInvocation invocation, CompoundRoot root) { 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 32cffc291f..293f4968a9 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.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 void setThreadAllowlist(ThreadAllowlist threadAllowlist) { 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 @@ protected StrutsParameter getParameterAnnotation(AnnotatedElement element) { } 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 0000000000..1243b6f24a --- /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 035a685bf8..d25bbe3774 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.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 SecurityMemberAccess(@Inject ProviderAllowlist providerAllowlist, @Inject 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 @@ protected boolean checkAllowlist(Object target, Member member) { 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 @@ protected boolean checkDefaultPackageAccess(Object target, Member member) { * @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 0000000000..ac80163af9 --- /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 0000000000..fe6aca7ae1 --- /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 dbc1940e80..a6a6010e01 100644 --- a/core/src/main/java/org/apache/struts2/util/ProxyUtil.java +++ b/core/src/main/java/org/apache/struts2/util/ProxyUtil.java @@ -43,10 +43,12 @@ /** * 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; @@ -59,10 +61,13 @@ public class ProxyUtil { * 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 static 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 + * @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 static 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 + * @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 static 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 + * @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 static 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 + * @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 static boolean isHibernateProxyMember(Member member) { * 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 @@ private static Class springUltimateTargetClass(Object candidate) { /** * 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 @@ private static boolean isSpringAopProxy(Object object) { /** * 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 @@ private static boolean isSpringProxyMember(Member member) { /** * 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 @@ private static boolean hasMember(Class clazz, Member member) { } /** + * 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 static Object getHibernateProxyTarget(Object object) { } /** + * 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 0000000000..8d3e54798b --- /dev/null +++ b/core/src/main/java/org/apache/struts2/util/StrutsProxyService.java @@ -0,0 +1,198 @@ +/* + * 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; + private final OgnlCache> targetClassCache; + + @Inject + @SuppressWarnings("unchecked") + public StrutsProxyService(ProxyCacheFactory proxyCacheFactory) { + this.isProxyCache = (OgnlCache, Boolean>) proxyCacheFactory.buildOgnlCache(); + this.isProxyMemberCache = (OgnlCache) proxyCacheFactory.buildOgnlCache(); + this.targetClassCache = (OgnlCache>) proxyCacheFactory.buildOgnlCache(); + } + + @Override + public Class ultimateTargetClass(Object candidate) { + return targetClassCache.computeIfAbsent(candidate, k -> { + Class result = null; + if (isSpringAopProxy(k)) { + result = springUltimateTargetClass(k); + } else if (isHibernateProxy(k)) { + result = getHibernateProxyTarget(k).getClass(); + } + if (result == null) { + result = k.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 7f1e9e1a80..4ed6bc3e9a 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -283,6 +283,19 @@ 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. +### Using 'basic' by default to avoid mandatory Caffeine dependency. +struts.proxy.cacheType=basic + +### 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 742d5634f2..7c59a88da3 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 54125fdefb..0040e98d3c 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.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 void setUp() throws Exception { 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 371e39aa47..a9b7b8c12d 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.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 void setUp() { target = new FooBar(); mockedProviderAllowlist = mock(ProviderAllowlist.class); mockedThreadAllowlist = mock(ThreadAllowlist.class); + proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic")); assignNewSma(true); } @@ -77,6 +81,7 @@ protected void assignNewSma(boolean allowStaticFieldAccess) { 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 0000000000..52f8fdf5fe --- /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 0000000000..931cfb3ff0 --- /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 0000000000..cbdb8283ec --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java @@ -0,0 +1,412 @@ +/* + * 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 ultimateTargetClassCachesResult() { + TestService proxy = createSpringProxy(new TestServiceImpl()); + + // First call should populate cache + Class result1 = proxyService.ultimateTargetClass(proxy); + // Second call with same object should use cached result + Class result2 = proxyService.ultimateTargetClass(proxy); + + assertThat(result1).isEqualTo(result2); + assertThat(result1).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 4309790576..5a1edc25b2 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 8e768bd915..df4ba2dec4 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.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 @@ protected void bean(Object object) throws JSONException { 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 78ee5535ba..77831ed945 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.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 void testExcludeNullPropeties() throws Exception { 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 @@ protected void setUp() throws Exception { 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 c02dc5cd33..526d528ab7 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.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 void setUp() throws Exception { 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/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 0000000000..c65cdc0632 --- /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 0000000000..261d6ace6c --- /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` From d4bfc13559e0ff1becd89c4df67309e4e2edae4a Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 20 Feb 2026 07:18:41 +0100 Subject: [PATCH 2/2] 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 --- .../struts2/spring/SpringProxyUtilTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) 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 f2e045ce8c..d878c8b9a8 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.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 @@ */ public class SpringProxyUtilTest extends XWorkTestCase { private ApplicationContext appContext; + private ProxyService proxyService; @Override public void setUp() throws Exception { @@ -43,6 +45,7 @@ public void setUp() throws Exception { 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 void testIsProxyMember() throws Exception { 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)); + } }