lukaszlenart opened a new pull request, #1871: URL: https://github.com/apache/struts/pull/1871
Fixes [WW-5697](https://issues.apache.org/jira/browse/WW-5697) ## Problem `XWorkMethodAccessor.callMethod(...)` skipped the `denyMethodExecution` check for any method whose name began with `get` and took one argument, or `set` and took two: ```java //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"))) { ``` That is a name prefix plus an argument count, not a property check. An ordinary method such as `getSomething(String)` is not a JavaBeans property, but it matches, so it was executed during parameter binding with the argument taken from the parameter name — a parameter name of `getSomething('value').property` calls `getSomething("value")`. The flag the branch consults, `DENY_INDEXED_ACCESS_EXECUTION`, is never written anywhere in main source, so `exec` is always `null` and the fast path is unconditional. ## Change The fast path now applies only where the target type genuinely declares an indexed property accessor, determined with `OgnlRuntime.getIndexedPropertyType(...)`. Everything else falls through to the existing `denyMethodExecution` check. Note the check is keyed on the *property* name while `callMethod` receives the *method* name, hence the prefix strip and `Introspector.decapitalize`. `DENY_INDEXED_ACCESS_EXECUTION` is public API, so it is deprecated here rather than deleted; removal in 8.0.0 is tracked as [WW-5699](https://issues.apache.org/jira/browse/WW-5699). ## Scope of the behaviour change Confined to parameter binding. The fast path only *matters* when the deny flag is set, and that flag is set only by `ParametersInterceptor`, `AliasInterceptor` and `StaticParametersInterceptor` — with it unset, such a call already fell through to the check below and executed. JSP and tag rendering are unaffected. Both kinds of indexed accessor keep working. Worth knowing for review: a classic `getItem(int)` / `setItem(int, String)` pair reports as `INDEXED_PROPERTY_OBJECT`, not `INDEXED_PROPERTY_INT`, so the predicate has to be `!= INDEXED_PROPERTY_NONE` — an `INT`-only rule would break the very case the original hack exists to serve. ## Tests `XWorkMethodAccessorTest` is new and covers four behaviours: - an argument-taking getter that is not an indexed property is not executed while method execution is denied (this failed before the change) - an int-indexed accessor still executes while denied - an object-indexed accessor still executes while denied - with the deny flag unset, an argument-taking getter still executes, as before The two indexed-property tests were mutation-checked: forcing the new predicate to always reject fails exactly those two, so they are not passing vacuously. Full `core` suite green: 3201 tests, 0 failures, 0 errors. ## Related [WW-5698](https://issues.apache.org/jira/browse/WW-5698) covers a separate issue found alongside this one — the `ModelDriven` exemption in `StrutsParameterAuthorizer` also exempting the action's own members. Different cause, different fix, not addressed here. An end-to-end `ModelDriven` binding test was deliberately left out of this PR because it would couple these tests to the exemption WW-5698 is expected to change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
