This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feature/WW-5697-indexed-access-fast-path in repository https://gitbox.apache.org/repos/asf/struts.git
commit cf53094b7e668cac2f815330272e16f9ea5faa6e Author: Lukasz Lenart <[email protected]> AuthorDate: Thu Aug 27 07:14:55 2026 +0200 WW-5697 fix(ognl): restrict the indexed-access fast path to real indexed properties XWorkMethodAccessor.callMethod skipped the denyMethodExecution check for any method whose name began with "get" and took one argument, or "set" and took two. That test is a name prefix plus an argument count, not a property check, so an ordinary method such as getSomething(String) qualified and was executed during parameter binding with the argument supplied in the parameter name. The fast path now applies only where the target type genuinely declares an indexed property accessor, determined with OgnlRuntime.getIndexedPropertyType. Anything else falls through to the existing denyMethodExecution check. Both int-indexed and object-indexed accessors continue to work. The new tests cover those two, the argument-taking method that must now be blocked while method execution is denied, and the unset-flag path where methods still execute as before, so the change is confined to parameter binding. DENY_INDEXED_ACCESS_EXECUTION is left in place for now; it is public API and is never set anywhere, so its removal is handled separately. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../struts2/ognl/accessor/XWorkMethodAccessor.java | 33 ++++++- .../ognl/accessor/XWorkMethodAccessorTest.java | 103 +++++++++++++++++++++ 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java b/core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java index 025553997..c7bb1796f 100644 --- a/core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java +++ b/core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java @@ -20,6 +20,7 @@ package org.apache.struts2.ognl.accessor; import org.apache.struts2.util.reflection.ReflectionContextState; import ognl.MethodFailedException; +import ognl.OgnlException; import ognl.ObjectMethodAccessor; import ognl.OgnlContext; import ognl.OgnlRuntime; @@ -27,6 +28,7 @@ import ognl.PropertyAccessor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.util.Arrays; import java.util.Collection; @@ -77,12 +79,16 @@ public class XWorkMethodAccessor extends ObjectMethodAccessor { } - //HACK - we pass indexed method access i.e. setXXX(A,B) pattern + //Indexed property access, i.e. the setXXX(A,B) / getXXX(A) pattern. Restricted to methods which + //really are indexed property accessors on the target type: a name prefix and an argument count + //alone would let any method be called while method execution is denied. if ((objects.length == 2 && string.startsWith("set")) || (objects.length == 1 && string.startsWith("get"))) { - Boolean exec = (Boolean) context.get(ReflectionContextState.DENY_INDEXED_ACCESS_EXECUTION); - boolean e = exec != null && exec; - if (!e) { - return callMethodWithDebugInfo(context, object, string, objects); + if (isIndexedPropertyAccessor(object, string)) { + Boolean exec = (Boolean) context.get(ReflectionContextState.DENY_INDEXED_ACCESS_EXECUTION); + boolean e = exec != null && exec; + if (!e) { + return callMethodWithDebugInfo(context, object, string, objects); + } } } boolean e = ReflectionContextState.isDenyMethodExecution(context); @@ -94,6 +100,23 @@ public class XWorkMethodAccessor extends ObjectMethodAccessor { } } + /** + * Whether {@code methodName} is an indexed property accessor on the target type, as opposed to an ordinary + * method which merely shares the {@code get}/{@code set} prefix and argument count of one. + */ + private boolean isIndexedPropertyAccessor(Object object, String methodName) { + if (object == null || methodName.length() <= 3) { + return false; + } + String propertyName = Introspector.decapitalize(methodName.substring(3)); + try { + return OgnlRuntime.getIndexedPropertyType(object.getClass(), propertyName) != OgnlRuntime.INDEXED_PROPERTY_NONE; + } catch (OgnlException e) { + LOG.debug("Could not determine whether [{}] is an indexed property of [{}]", propertyName, object.getClass(), e); + return false; + } + } + private Object callMethodWithDebugInfo(OgnlContext context, Object object, String methodName, Object[] objects) throws MethodFailedException { try { return super.callMethod(context, object, methodName, objects); diff --git a/core/src/test/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessorTest.java b/core/src/test/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessorTest.java new file mode 100644 index 000000000..dcee2f3ad --- /dev/null +++ b/core/src/test/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessorTest.java @@ -0,0 +1,103 @@ +/* + * 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.accessor; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.util.ValueStack; +import org.apache.struts2.util.reflection.ReflectionContextState; + +public class XWorkMethodAccessorTest extends XWorkTestCase { + + public void testDenyMethodExecutionBlocksArgumentTakingGetterThatIsNotAnIndexedProperty() { + Bean bean = new Bean(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(bean); + ReflectionContextState.setDenyMethodExecution(vs.getContext(), true); + + vs.findValue("getAttack('PWNED')"); + + assertNull("getAttack(String) is not an indexed property accessor and must not be" + + " executed while method execution is denied", bean.attackArgument); + } + + public void testDenyMethodExecutionAllowsIntIndexedPropertyAccessor() { + Bean bean = new Bean(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(bean); + ReflectionContextState.setDenyMethodExecution(vs.getContext(), true); + + Object value = vs.findValue("getItem(1)"); + + assertEquals("indexed property accessors must keep working while method execution is denied", + "item1", value); + } + + public void testDenyMethodExecutionAllowsObjectIndexedPropertyAccessor() { + Bean bean = new Bean(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(bean); + ReflectionContextState.setDenyMethodExecution(vs.getContext(), true); + + Object value = vs.findValue("getKeyed('k')"); + + assertEquals("object indexed property accessors must keep working while method execution is denied", + "keyedk", value); + } + + public void testArgumentTakingGetterIsExecutedWhenMethodExecutionIsNotDenied() { + Bean bean = new Bean(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(bean); + + vs.findValue("getAttack('PWNED')"); + + assertEquals("outside parameter binding the deny flag is unset and methods still execute", + "PWNED", bean.attackArgument); + } + + public static class Bean { + private String attackArgument; + + /** + * Not a JavaBeans property: takes an argument and has no matching setter, so it is not an + * indexed property accessor either. + */ + public String getAttack(String argument) { + this.attackArgument = argument; + return "irrelevant"; + } + + public String getItem(int index) { + return "item" + index; + } + + public void setItem(int index, String value) { + // present so that the pair forms an indexed property + } + + public String getKeyed(String key) { + return "keyed" + key; + } + + public void setKeyed(String key, String value) { + // present so that the pair forms an indexed property + } + } +}
