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 a66478a97c..418c55e440 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -516,6 +516,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 a574af1c22..1260a7dee6 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; @@ -61,10 +63,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) { return targetClassCache.computeIfAbsent(candidate, k -> { Class result = null; @@ -82,8 +87,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(), @@ -92,9 +101,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; @@ -107,7 +120,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()); @@ -120,7 +136,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); @@ -133,6 +152,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}) @@ -147,6 +167,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) { @@ -159,6 +180,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) { @@ -176,7 +198,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) { @@ -193,8 +216,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); @@ -204,9 +232,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 a980196a68..eb841d7833 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`