This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 66af2365e WW-5697 Restrict the indexed-access fast path in
XWorkMethodAccessor to real indexed properties (#1871)
66af2365e is described below
commit 66af2365e1792178b1b0a2dde4adbe47d65a6fd1
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon Aug 31 18:51:39 2026 +0200
WW-5697 Restrict the indexed-access fast path in XWorkMethodAccessor to
real indexed properties (#1871)
* 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]>
* WW-5697 chore(ognl): deprecate DENY_INDEXED_ACCESS_EXECUTION
Nothing in the framework has ever written this key, so the check it guarded
in
XWorkMethodAccessor never fired. Now that indexed property access is
identified
from the target type rather than from a method name prefix, the flag has
nothing
left to guard.
It is public API, so it is deprecated here rather than deleted, and removal
is
tracked for 8.0.0 in WW-5699.
Co-Authored-By: Claude Opus 5 <[email protected]>
* WW-5697 refactor(ognl): address SonarQube findings
Merge the nested indexed-property check into the enclosing condition (S1066)
and give the deprecation its since/forRemoval arguments (S6355).
Also cover the branch that rejects a method with nothing left after the
"get"
prefix, using a map style get(String) accessor. That is worth asserting in
its
own right: such a method is not an indexed property accessor, so it must
not be
executed while method execution is denied.
Co-Authored-By: Claude Opus 5 <[email protected]>
* WW-5697 fix(ognl): identify the indexed accessor by method, not by
property name
Addresses review of #1871.
Keying the check on the property name left two ways through. A class
declaring the indexed pair getItem(int)/setItem(int, String) may also
declare an unrelated getItem(String) overload, and a one-argument call
dispatches to that overload, because the argument types choose the method
and the caller chooses the arguments. And the check was direction-agnostic,
so a read-only getItem(int) legitimised an unrelated two-argument
setItem(String, String). Both executed while method execution was denied.
The descriptor's own indexed accessor must now be the method that will
actually run: same name, same direction, and no same-arity overload for the
dispatcher to prefer instead.
The deny check is hoisted ahead of the indexed-property block, which it now
guards. The two are equivalent - with execution permitted, both paths ended
in the same call - but this way the introspection is skipped entirely on the
common path, and the block reads as the exception it is.
Also reword the deprecation javadoc, which claimed the key had never had any
effect: application code that sets it does still suppress the fast path.
Suppress the removal warning at the framework's own read of it.
Tests: an overload of an indexed accessor, and an unrelated setter named
after a read-only indexed property, are both blocked while execution is
denied. Both fail against the previous predicate. A read-only int-indexed
getter is added because it is the only shape that reaches
INDEXED_PROPERTY_INT - OGNL reclassifies a get/set pair as _OBJECT - so the
existing tests never covered that branch.
Full core suite: 3205 tests, 0 failures.
Co-Authored-By: Claude Opus 5 <[email protected]>
* WW-5697 test(ognl): cover the branches of the indexed accessor fast path
SonarCloud failed the quality gate on PR #1871 at 79.17% new-code coverage.
The uncovered branches were all real behaviour that nothing asserted:
- the deprecated DENY_INDEXED_ACCESS_EXECUTION key, both set and set to
false. Its javadoc claims application code can still suppress the
exemption with it, which is the reason it was deprecated rather than
removed, and nothing tested that claim.
- the object-indexed mutator half of the pair, which parameter binding
itself walks through.
- methods carrying neither prefix, which never reach the property lookup.
- an overload of another arity, which cannot be dispatched to and so must
not cost the bean its indexed property access.
Each new test was checked by mutation: making the predicate always true,
never honouring the legacy key, always honouring it, and dropping the
argument-count filter each fail exactly the tests that assert that branch.
New-code coverage goes from 79.17% to roughly 92%. What stays uncovered is
defensive only: the null target, the OgnlException catch, and the two guards
against a descriptor accessor that disagrees with the invoked method.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01P9Pjt4rvb1ASASjSTHsUhL
* WW-5697 test(ognl): clear the four SonarCloud smells in the accessor
fixtures
The gate passes, but four issues stand, all in the test beans.
S4144 is the one worth having: attack(String) and getAttack(String) had
identical bodies because both recorded into attackArgument, so neither of
the
tests asserting on that field could tell which of the two methods had run.
The
unprefixed pair now records into its own field, which is what the tests
naming
it actually mean to assert.
The three S1172s are unused second parameters on fixtures whose two-argument
shape is the whole point, so the parameter cannot be removed. Each now
records
the full call instead of only its first argument, which is what the
surrounding
fixtures already did and costs nothing.
Mutation still holds: forcing isIndexedPropertyAccessor to accept everything
fails all six "blocks" tests, the two switched to the new field included.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01P9Pjt4rvb1ASASjSTHsUhL
* WW-5697 fix(ognl): count indexed accessor dispatch candidates by signature
isTheOnlyDispatchCandidate counted Method objects, but
OgnlRuntime.getMethods
reports an overridden method and the method overriding it separately. An
ordinary bean shape - a base class declaring the indexed pair, a subclass
refining the accessor - therefore reported two candidates and lost its
indexed
property access while method execution was denied.
Count by signature instead. Two methods related by an override share a
parameter list, so they are not a choice the dispatcher makes: only one
implementation can ever run. Distinct parameter lists of the same arity are
the
real overloads, and still deny, so the guard is unchanged for every shape it
was written to catch.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_016sWTsWWWg3oTg2VpACjxRt
---------
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../struts2/ognl/accessor/XWorkMethodAccessor.java | 98 +++++-
.../util/reflection/ReflectionContextState.java | 8 +
.../ognl/accessor/XWorkMethodAccessorTest.java | 382 +++++++++++++++++++++
3 files changed, 477 insertions(+), 11 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..2373b0160 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,16 +20,22 @@ package org.apache.struts2.ognl.accessor;
import org.apache.struts2.util.reflection.ReflectionContextState;
import ognl.MethodFailedException;
+import ognl.OgnlException;
import ognl.ObjectMethodAccessor;
+import ognl.ObjectIndexedPropertyDescriptor;
import ognl.OgnlContext;
import ognl.OgnlRuntime;
import ognl.PropertyAccessor;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import java.beans.IndexedPropertyDescriptor;
+import java.beans.Introspector;
import java.beans.PropertyDescriptor;
+import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
+import java.util.List;
/**
* Allows methods to be executed under normal cirumstances, except when {@link
ReflectionContextState#DENY_METHOD_EXECUTION}
@@ -77,21 +83,91 @@ public class XWorkMethodAccessor extends
ObjectMethodAccessor {
}
- //HACK - we pass indexed method access i.e. setXXX(A,B) pattern
- 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 (!ReflectionContextState.isDenyMethodExecution(context)) {
+ return callMethodWithDebugInfo(context, object, string, objects);
}
- boolean e = ReflectionContextState.isDenyMethodExecution(context);
- if (!e) {
+ //Method execution is denied. Indexed property access, i.e. the
getXXX(A) / setXXX(A,B) pattern, is
+ //the one exception, because reading a['k'] must keep working during
parameter binding. It is
+ //restricted to calls which really are the indexed accessor of a
property on the target type: a name
+ //prefix and an argument count alone would let any method be called
while execution is denied.
+ if (isIndexedPropertyAccessor(object, string, objects)
+ && !isIndexedAccessDenied(context)) {
return callMethodWithDebugInfo(context, object, string, objects);
- } else {
- return null;
}
+ return null;
+ }
+
+ @SuppressWarnings("removal") // the constant is deprecated for removal in
8.0.0 (WW-5699); until then it is still honoured
+ private static boolean isIndexedAccessDenied(OgnlContext context) {
+ Boolean denied = (Boolean)
context.get(ReflectionContextState.DENY_INDEXED_ACCESS_EXECUTION);
+ return denied != null && denied;
+ }
+
+ /**
+ * Whether this call is the indexed accessor of a property on the target
type, as opposed to an ordinary
+ * method which merely shares the {@code get}/{@code set} prefix and
argument count of one.
+ * <p>
+ * The property name alone is not enough to decide, for two reasons. A
class declaring the indexed pair
+ * {@code getItem(int)} / {@code setItem(int, String)} may <em>also</em>
declare an unrelated
+ * {@code getItem(String)} overload, and it is that overload OGNL
dispatches a one-argument call to, since
+ * the argument types pick the method and the caller chooses those. And an
indexed property may be
+ * read-only, whose name would otherwise legitimise an unrelated
two-argument {@code setItem(String, String)}.
+ * So the descriptor's own accessor must be the method that will actually
be invoked: same name, same
+ * direction, and no same-arity overload for the dispatcher to prefer
instead.
+ */
+ private boolean isIndexedPropertyAccessor(Object object, String
methodName, Object[] args) {
+ boolean reading = args.length == 1 && methodName.startsWith("get");
+ boolean writing = args.length == 2 && methodName.startsWith("set");
+ if (object == null || methodName.length() <= 3 || (!reading &&
!writing)) {
+ return false;
+ }
+ Class<?> targetType = object.getClass();
+ String propertyName =
Introspector.decapitalize(methodName.substring(3));
+ try {
+ Method accessor =
indexedAccessorOf(OgnlRuntime.getPropertyDescriptor(targetType, propertyName),
reading);
+ return accessor != null
+ && accessor.getName().equals(methodName)
+ && isTheOnlyDispatchCandidate(targetType, methodName,
args.length);
+ } catch (OgnlException e) {
+ LOG.debug("Could not determine whether [{}] is an indexed property
of [{}]", propertyName, targetType, e);
+ return false;
+ }
+ }
+
+ /**
+ * The indexed accessor a descriptor declares for the requested direction,
or {@code null} when the
+ * descriptor is not an indexed one or declares no accessor that way
round. Both flavours are covered:
+ * JavaBeans int-indexed properties, and OGNL's arbitrary-object-indexed
ones.
+ */
+ private static Method indexedAccessorOf(PropertyDescriptor descriptor,
boolean reading) {
+ if (descriptor instanceof IndexedPropertyDescriptor indexed) {
+ return reading ? indexed.getIndexedReadMethod() :
indexed.getIndexedWriteMethod();
+ }
+ if (descriptor instanceof ObjectIndexedPropertyDescriptor
objectIndexed) {
+ return reading ? objectIndexed.getIndexedReadMethod() :
objectIndexed.getIndexedWriteMethod();
+ }
+ return null;
+ }
+
+ /**
+ * Whether the named method is the only one of that argument count, and so
is certainly the one OGNL
+ * dispatches to. With an overload present the argument values decide, and
those come from the caller.
+ * <p>
+ * Candidates are counted by signature rather than by {@link Method},
because {@code getMethods} reports
+ * an overridden method and the method overriding it separately. Those two
share a parameter list, so
+ * they are not a choice the dispatcher makes - only one implementation
can ever run - and an accessor
+ * refined in a subclass must not lose the bean its indexed property
access. Distinct parameter lists of
+ * the same arity are the real overloads, and still deny.
+ */
+ private static boolean isTheOnlyDispatchCandidate(Class<?> targetType,
String methodName, int argCount) {
+ List<Method> candidates = OgnlRuntime.getMethods(targetType,
methodName, false);
+ return candidates != null
+ && candidates.stream()
+ .filter(candidate -> candidate.getParameterCount() ==
argCount)
+ .map(candidate ->
Arrays.asList(candidate.getParameterTypes()))
+ .distinct()
+ .count() == 1;
}
private Object callMethodWithDebugInfo(OgnlContext context, Object object,
String methodName, Object[] objects) throws MethodFailedException {
diff --git
a/core/src/main/java/org/apache/struts2/util/reflection/ReflectionContextState.java
b/core/src/main/java/org/apache/struts2/util/reflection/ReflectionContextState.java
index cc2f457c3..d4ad46ea5 100644
---
a/core/src/main/java/org/apache/struts2/util/reflection/ReflectionContextState.java
+++
b/core/src/main/java/org/apache/struts2/util/reflection/ReflectionContextState.java
@@ -38,6 +38,14 @@ public class ReflectionContextState {
public static final String FULL_PROPERTY_PATH =
"current.property.path"; // TODO: Probably a bug
public static final String CREATE_NULL_OBJECTS =
"xwork.NullHandler.createNullObjects";
public static final String DENY_METHOD_EXECUTION =
"xwork.MethodAccessor.denyMethodExecution";
+ /**
+ * @deprecated since 7.4.0, no replacement. Struts core never sets this
key, so it has no effect on
+ * framework-driven binding. Indexed property access is now identified
by inspecting the target type
+ * rather than by trusting a method name prefix, which is the check the
key was standing in for.
+ * Application or plugin code which sets the key itself does still
suppress the fast path, which is
+ * why this is deprecated rather than removed outright. Scheduled for
removal in 8.0.0 by WW-5699.
+ */
+ @Deprecated(since = "7.4.0", forRemoval = true)
public static final String DENY_INDEXED_ACCESS_EXECUTION =
"xwork.IndexedPropertyAccessor.denyMethodExecution";
public static boolean isCreatingNullObjects(Map<String, Object> context) {
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..994c3b4af
--- /dev/null
+++
b/core/src/test/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessorTest.java
@@ -0,0 +1,382 @@
+/*
+ * 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);
+ }
+
+ /**
+ * Note the name: OGNL classifies this pair as {@code
INDEXED_PROPERTY_OBJECT}, not {@code _INT}, because
+ * {@code findObjectIndexedPropertyDescriptors} overwrites the {@code
java.beans} descriptor whenever it
+ * finds a matching get/set pair. {@link
#testDenyMethodExecutionAllowsReadOnlyIntIndexedPropertyAccessor()}
+ * is what covers the {@code _INT} branch.
+ */
+ public void
testDenyMethodExecutionAllowsIndexedPropertyAccessorDeclaredOverAnIntIndex() {
+ 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);
+ }
+
+ /**
+ * A read-only indexed property is the one shape that reaches {@code
INDEXED_PROPERTY_INT}: with no
+ * matching setter, OGNL leaves the {@code java.beans} {@code
IndexedPropertyDescriptor} in place.
+ */
+ public void
testDenyMethodExecutionAllowsReadOnlyIntIndexedPropertyAccessor() {
+ ReadOnlyIndexedBean bean = new ReadOnlyIndexedBean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ Object value = vs.findValue("getItem(1)");
+
+ assertEquals("a read-only indexed property accessor must keep working
while method execution is denied",
+ "item1", value);
+ }
+
+ /**
+ * The property name alone does not identify the method that will run.
This bean really does declare the
+ * indexed pair getItem(int)/setItem(int, String), so the property is
indexed - but the one-argument call
+ * below dispatches to the unrelated getItem(String) overload, because the
argument types choose the
+ * method and the caller chooses the arguments.
+ */
+ public void testDenyMethodExecutionBlocksAnOverloadOfAnIndexedAccessor() {
+ OverloadedIndexedBean bean = new OverloadedIndexedBean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("getItem('PWNED')");
+
+ assertNull("an overload sharing an indexed accessor's name must not be
executed while method"
+ + " execution is denied", bean.overloadArgument);
+ }
+
+ /**
+ * The direction matters too: a read-only indexed property must not
legitimise an unrelated two-argument
+ * setter that merely shares its name.
+ */
+ public void
testDenyMethodExecutionBlocksUnrelatedSetterNamedAfterAReadOnlyIndexedProperty()
{
+ ReadOnlyIndexedBean bean = new ReadOnlyIndexedBean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("setItem('PWNED', 'x')");
+
+ assertNull("a two-argument setter is not the accessor of a read-only
indexed property and must not"
+ + " be executed while method execution is denied",
bean.setterArgument);
+ }
+
+ public void testDenyMethodExecutionBlocksBareGetAccessor() {
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("get('PWNED')");
+
+ assertNull("a map style get(String) is not an indexed property
accessor and must not be"
+ + " executed while method execution is denied",
bean.bareGetArgument);
+ }
+
+ /**
+ * The object indexed pair is what parameter binding itself walks through,
so the mutator half has to keep
+ * working under the deny flag exactly as the accessor half does.
+ */
+ public void testDenyMethodExecutionAllowsObjectIndexedPropertyMutator() {
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("setKeyed('k', 'v')");
+
+ assertEquals("object indexed property mutators must keep working while
method execution is denied",
+ "k=v", bean.keyedArgument);
+ }
+
+ /**
+ * The prefix is half of what makes a call a candidate: a method taking
the right number of arguments but
+ * named nothing like an accessor never reaches the property lookup at all.
+ */
+ public void testDenyMethodExecutionBlocksAnUnprefixedOneArgumentMethod() {
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("attack('PWNED')");
+
+ assertNull("a one argument method without the get prefix must not be
executed while method"
+ + " execution is denied", bean.unprefixedArgument);
+ }
+
+ public void testDenyMethodExecutionBlocksAnUnprefixedTwoArgumentMethod() {
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ vs.findValue("attack('PWNED', 'x')");
+
+ assertNull("a two argument method without the set prefix must not be
executed while method"
+ + " execution is denied", bean.unprefixedArgument);
+ }
+
+ /**
+ * The overload guard is scoped to the argument count, because that is
what OGNL dispatches on. A method
+ * sharing the accessor's name but taking a different number of arguments
is never a candidate for this
+ * call, and so must not cost the bean its indexed property access.
+ */
+ public void
testDenyMethodExecutionAllowsIndexedAccessorWithAnOverloadOfAnotherArity() {
+ DifferentArityOverloadBean bean = new DifferentArityOverloadBean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ Object value = vs.findValue("getItem(1)");
+
+ assertEquals("an overload of another arity cannot be dispatched to and
must not block indexed"
+ + " property access", "item1", value);
+ assertNull("the overload itself must not have been executed",
bean.overloadArgument);
+ }
+
+ /**
+ * An override is not an overload: it is the same signature, so there is
only ever one method a call can
+ * dispatch to, and the bean must keep its indexed property access. The
shape is an ordinary one - a base
+ * class declaring the indexed pair, a subclass refining the accessor.
+ */
+ public void
testDenyMethodExecutionAllowsIndexedAccessorOverriddenInASubclass() {
+ OverriddenIndexedBean bean = new OverriddenIndexedBean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
+ Object value = vs.findValue("getItem(1)");
+
+ assertEquals("an overriding accessor is the same signature as the one
it overrides, not a second"
+ + " dispatch candidate, and must not block indexed property
access", "overridden1", value);
+ }
+
+ /**
+ * {@link ReflectionContextState#DENY_INDEXED_ACCESS_EXECUTION} is
deprecated because Struts itself never
+ * sets it, but application and plugin code still can, and while it does
the indexed accessor exemption
+ * has to stay switched off. That is the whole reason the key is
deprecated rather than removed outright.
+ */
+ @SuppressWarnings("removal")
+ public void
testDenyIndexedAccessExecutionSuppressesTheIndexedAccessorExemption() {
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
vs.getContext().put(ReflectionContextState.DENY_INDEXED_ACCESS_EXECUTION,
Boolean.TRUE);
+
+ Object value = vs.findValue("getItem(1)");
+
+ assertNull("setting the legacy key must suppress the indexed accessor
exemption", value);
+ }
+
+ @SuppressWarnings("removal")
+ public void
testDenyIndexedAccessExecutionSetToFalseLeavesTheIndexedAccessorExemptionInPlace()
{
+ Bean bean = new Bean();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(bean);
+ ReflectionContextState.setDenyMethodExecution(vs.getContext(), true);
+
vs.getContext().put(ReflectionContextState.DENY_INDEXED_ACCESS_EXECUTION,
Boolean.FALSE);
+
+ Object value = vs.findValue("getItem(1)");
+
+ assertEquals("the legacy key set to false must leave indexed property
access working", "item1", 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;
+ private String bareGetArgument;
+ private String keyedArgument;
+ private String unprefixedArgument;
+
+ /**
+ * Named exactly "get", so there is no property name left once the
prefix is removed.
+ */
+ public String get(String key) {
+ this.bareGetArgument = key;
+ return "irrelevant";
+ }
+
+ /**
+ * 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) {
+ this.keyedArgument = key + "=" + value;
+ }
+
+ /**
+ * Neither prefix, so no property name can be derived from it at all -
whatever its argument count.
+ * Records into its own field, so a test can tell this apart from
{@link #getAttack(String)} having run.
+ */
+ public String attack(String argument) {
+ this.unprefixedArgument = argument;
+ return "irrelevant";
+ }
+
+ public String attack(String argument, String other) {
+ this.unprefixedArgument = argument + "," + other;
+ return "irrelevant";
+ }
+ }
+
+ public static class BaseIndexedBean {
+
+ public String getItem(int index) {
+ return "item" + index;
+ }
+
+ public void setItem(int index, String value) {
+ // present so that the pair forms an indexed property
+ }
+ }
+
+ /**
+ * Overrides the inherited indexed accessor rather than overloading it.
The returned value differs from
+ * the base class so a test can tell which of the two ran.
+ */
+ public static class OverriddenIndexedBean extends BaseIndexedBean {
+
+ @Override
+ public String getItem(int index) {
+ return "overridden" + index;
+ }
+ }
+
+ public static class ReadOnlyIndexedBean {
+ private String setterArgument;
+
+ public String getItem(int index) {
+ return "item" + index;
+ }
+
+ /**
+ * Not the indexed setter of {@code item} - that would be {@code
setItem(int, String)}. It only shares
+ * the name and the two-argument shape.
+ */
+ public void setItem(String key, String value) {
+ this.setterArgument = key + "=" + value;
+ }
+ }
+
+ public static class DifferentArityOverloadBean {
+ private String overloadArgument;
+
+ public String getItem(int index) {
+ return "item" + index;
+ }
+
+ public void setItem(int index, String value) {
+ // present so that the pair forms an indexed property
+ }
+
+ /**
+ * Shares the name but not the argument count, so it is not what a one
argument call resolves to.
+ */
+ public String getItem(String key, String other) {
+ this.overloadArgument = key + "," + other;
+ return "irrelevant";
+ }
+ }
+
+ public static class OverloadedIndexedBean {
+ private String overloadArgument;
+
+ 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 getItem(String key) {
+ this.overloadArgument = key;
+ return "irrelevant";
+ }
+ }
+}
