You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@struts.apache.org by "lukaszlenart (via GitHub)" <gi...@apache.org> on 2026/07/14 10:52:27 UTC
[PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
lukaszlenart opened a new pull request, #1784:
URL: https://github.com/apache/struts/pull/1784
Fixes [WW-4858](https://issues.apache.org/jira/browse/WW-4858)
## What & why
Follow-up to #1773. That change added `ParametersInterceptor`-style name/value
filtering to `JSONInterceptor`'s reflection population path, but applied **every**
name check at **every** node of the JSON tree before recursing. Two of those
checks — **accepted name patterns** (`AcceptedPatternsChecker`) and the
**`ParameterNameAware`** callback — target the *full dotted binding path*, so
evaluating them on an intermediate node was wrong:
- A leaf-targeting accepted pattern such as `bean\.stringField` fails to match the
intermediate node `bean`, so the whole `bean` subtree was dropped before the
leaf was ever visited. The flat `ParametersInterceptor` path only evaluates the
complete name `bean.stringField`, so it accepts it. Divergence.
- Same shape for `ParameterNameAware`: `acceptableParameterName("bean")` was
invoked on the intermediate node, which the flat path never sees.
- For arrays, the accepted allowlist judged the **container** name (`list`)
instead of the element path (`list[0]`), so a container-matching pattern let
elements through that the flat path would reject at `list[0]`.
These only ever *tightened* acceptance (no security bypass), but they made
restrictive accepted/`ParameterNameAware` config silently drop nested JSON.
## Change
Split the per-key gate in the recursive walk:
- **Per-node (every node):** key length, excluded name patterns, `@StrutsParameter`
authorization, and (opt-in) property filters. Exclusion is prefix-safe and
authorization is intentionally hierarchical, so these are correct at intermediate
nodes — unchanged.
- **Leaf keys only:** accepted name patterns and `ParameterNameAware`, now also
applied to scalar array elements at their indexed path (`items[0]`).
This reproduces the flat-path semantics exactly. `excludeProperties`/`includeProperties`
behavior is unchanged (include patterns already hierarchy-expand across levels).
## Tests (`JSONInterceptorTest`)
- `testAcceptedNamePatternRejectsNestedKey` — leaf-targeting accepted pattern now
populates the nested leaf and drops the non-matching sibling.
- `testAcceptedNamePatternAppliesToListElementPath` — accepted patterns evaluated
at the array element path, not the container.
- `testParameterNameAwareDoesNotRejectIntermediateNode` — an action rejecting the
intermediate node name no longer drops the subtree.
- `testIncludePropertiesAppliedToNestedInputWhenEnabled` — regression guard;
nested include-property filtering still works.
All three fix-driving tests were confirmed to fail before the change.
## Testing
`mvn test -pl plugins/json -am -DskipAssembly` — 146 tests, all pass.
## Note
Supersedes #1783 (which documented the accepted-name limitation as a test); that
PR can be closed. The still-valid include-properties test from it is carried here.
🤖 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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1784:
URL: https://github.com/apache/struts/pull/1784#issuecomment-4968753677
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1784:
URL: https://github.com/apache/struts/pull/1784#issuecomment-4968402572
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1784:
URL: https://github.com/apache/struts/pull/1784#issuecomment-4969128638
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1784:
URL: https://github.com/apache/struts/pull/1784#issuecomment-5006370675
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1784&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1784&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [97.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1784&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1784)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart merged PR #1784:
URL: https://github.com/apache/struts/pull/1784
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-4858 Evaluate JSON name allowlist at leaf keys only [struts]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1784:
URL: https://github.com/apache/struts/pull/1784#discussion_r3578625942
##########
plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java:
##########
@@ -263,15 +270,23 @@ private void filterUnacceptableList(java.util.List list, String prefix, Object t
filterUnacceptableKeysRecursive((Map) item, elementPrefix, target, action);
} else if (item instanceof java.util.List) {
filterUnacceptableList((java.util.List) item, elementPrefix, target, action);
- // Scalar list elements are value-checked only; their parent key already passed name/authorization checks.
- } else if (!isAcceptableValue(elementPrefix, item, action)) {
+ // Scalar list elements are leaf binding targets: apply the leaf name-allowlist checks at
+ // the element path (e.g. "items[0]") plus value checks. The container key already passed
+ // the per-node checks in the caller.
+ } else if (!isAcceptableLeafName(elementPrefix, action) || !isAcceptableValue(elementPrefix, item, action)) {
Review Comment:
Scalar list elements now run accepted-name/ParameterNameAware checks via isAcceptableLeafName(elementPrefix, …), but they still skip the per-node gate (length limit, excluded patterns, `@StrutsParameter` authorization, and optional include/exclude property filters). This diverges from the method’s own comment (“per-node checks apply to every node”) and from ParametersInterceptor, which applies these checks to the full leaf name (e.g. "items[0]"). Consider also applying isAcceptableNode(elementPrefix, target, action) before value/name checks for scalar elements.
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org