lukaszlenart opened a new pull request, #1673: URL: https://github.com/apache/struts/pull/1673
Fixes [WW-5626](https://issues.apache.org/jira/browse/WW-5626). **Part 1 of 2.** Approach C — handler-level per-property authorization that replaces `ContentTypeInterceptor`'s two-phase deserialize-then-copy with a new `AuthorizationAwareContentTypeHandler` interface — will follow in a separate PR. This PR addresses only the three concerns from the WW-5624 review that survive that refactor: ## Changes - **Centralized ModelDriven target resolution.** `ParametersInterceptor.isParameterAnnotatedAndAllowlist` previously did the value-stack peek itself, then `StrutsParameterAuthorizer.isAuthorized` independently checked `target != action && action instanceof ModelDriven`. New `ParameterAuthorizer.resolveTarget(action)` consolidates the logic. Kept as a `default` method to preserve the interface as a SAM (lambda-based test stubs continue to work). - **Defensive guard against non-String JSON keys.** `JSONInterceptor.filterUnauthorizedKeysRecursive` did `String key = (String) entry.getKey();` on a raw `Map`. Safe with the built-in `StrutsJSONReader`, but a custom reader producing non-String keys would throw `ClassCastException`. Replaced with `instanceof String key` pattern that debug-logs and skips. - **Real REST integration tests.** `ContentTypeInterceptorTest.testRequireAnnotationsEnabled_*` use `com.mockobjects` mocks for `ContentTypeHandler`, so `toObject` is a no-op — the tests prove `intercept()` returns SUCCESS but assert nothing about which properties were actually filtered. New `ContentTypeInterceptorIntegrationTest` uses a real `JacksonJsonHandler` and a real `StrutsParameterAuthorizer` end-to-end with a mixed-annotation `SecureRestAction` fixture. ## Finding for follow-up PR Building the REST integration tests surfaced a real semantic divergence (documented inline in `SecureRestAction.java`): REST's recursive copy authorizes EACH path level independently, so `@StrutsParameter(depth=1)` on `getAddress` is not enough to bind `address.city` — the setter `setAddress` also needs `@StrutsParameter` to authorize the top-level `address` at depth 0. `ParametersInterceptor` only requires the getter annotation. This is captured for the Approach C PR. ## Out of scope `ContentTypeInterceptor` deep-copy/scrub complexity, the `isNestedBeanType` package-name heuristic, and the two-phase architectural smell — all to be replaced by Approach C. ## Test plan - [x] `mvn test -DskipAssembly -pl core` — 2926 tests, zero regressions - [x] `mvn test -DskipAssembly -pl plugins/json` — 125 tests, zero regressions - [x] `mvn test -DskipAssembly -pl plugins/rest` — 81 tests, zero regressions - [x] `mvn test -DskipAssembly -pl '!plugins/bean-validation,!plugins/tiles'` — full multi-module BUILD SUCCESS (the two excluded modules fail identically on `main` for unrelated reasons: `AnnotationFormatError` from Hibernate Validator on a test model, and missing Velocity dependencies in the tiles plugin) -- 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]
