This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5641-json-writer-reader-override in repository https://gitbox.apache.org/repos/asf/struts.git
commit 84ec101c7aee671e92df6c8475536e90fa6ac781 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Jul 6 19:20:20 2026 +0200 WW-5641 docs: design spec for JSON writer/reader override regression Co-Authored-By: Claude Opus 4.8 <[email protected]> --- ...son-writer-reader-override-regression-design.md | 153 +++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/docs/superpowers/specs/2026-07-06-WW-5641-json-writer-reader-override-regression-design.md b/docs/superpowers/specs/2026-07-06-WW-5641-json-writer-reader-override-regression-design.md new file mode 100644 index 000000000..33354e2a2 --- /dev/null +++ b/docs/superpowers/specs/2026-07-06-WW-5641-json-writer-reader-override-regression-design.md @@ -0,0 +1,153 @@ +# WW-5641: Restore `struts.json.writer` / `struts.json.reader` override + +**Ticket:** [WW-5641](https://issues.apache.org/jira/browse/WW-5641) +**Type:** Bug (regression) +**Component:** Plugin - JSON +**Date:** 2026-07-06 + +## Problem + +A custom JSON writer/reader configured the documented way is silently ignored on the 7.2.x +line. The framework always uses the default `StrutsJSONWriter` / `StrutsJSONReader` regardless +of the application's override: + +```xml +<bean type="org.apache.struts2.json.JSONWriter" name="flexJSONWriter" + class="org.demo.FlexJSONWriter" scope="prototype"/> +<constant name="struts.json.writer" value="flexJSONWriter"/> +``` + +This worked on 7.1.x and regressed in 7.2.x. The extension point is still documented at +<https://struts.apache.org/plugins/json/>, so this is a regression, not an intended API change. +Confirmed by the `json-customize` module reproduction in apache/struts-examples PR #535 +(`ProduceActionTest`): passes on 7.1.x, fails on 7.2.x. + +This is a **functional regression** restoring a documented extension point — not a security fix +(no OGNL injection, parameter-filtering bypass, auth bypass, RCE, SSRF, path traversal, etc.), +so it follows the normal PR path. + +## Root cause (verified against current `main`) + +Writer/reader selection moved from a **runtime, by-name lookup** (7.1.x) to a +**container-build-time alias** (7.2.x, introduced by the JSON hardening rework that added +`<bean-selection name="jsonBeans" .../>` to the plugin's `struts-plugin.xml`). + +- `JSONUtil` is the **only** consumer of `JSONWriter` / `JSONReader`. In 7.2.x it injects the + default binding directly (`setWriter(JSONWriter)` / `setReader(JSONReader)`, + `JSONUtil.java:74-77` and `:65-68`). `JSONResult` and `JSONInterceptor` inject `JSONUtil`; + nothing else injects the writer/reader. +- The default binding is chosen once by `JSONBeanSelectionProvider.register()`, which calls + `AbstractBeanSelectionProvider.alias(JSONWriter.class, "struts.json.writer", …)` (and the + reader equivalent). +- `XmlDocConfigurationProvider.registerBeanSelection()` (`:215-222`) invokes the bean-selection + provider **inline, the moment the `<bean-selection>` element is parsed** inside the JSON + plugin's `struts-plugin.xml`. At that instant the shared `props` hold only the plugin's own + `struts.json.writer=struts`; the application `struts.xml` is a **later** `ContainerProvider` + in `DefaultConfiguration.reloadContainer()` (`:283-287`) whose override bean/constant have not + been folded in yet. +- So `alias()` takes the `builder.contains(type, "struts")` branch and locks + `JSONWriter/DEFAULT_NAME → StrutsJSONWriter`. Nothing re-runs the selection once the app + config loads, so the override is dropped. + +The 7.1.x code was immune because `JSONUtil` injected the `Container` and resolved the +writer/reader by the constant value at container-use time — *after* the full container +(including the app `struts.xml`) was built. + +## Chosen approach: Option A — deferred runtime by-name resolution in `JSONUtil` + +Restore the 7.1.x resolution model, localized to `JSONUtil`. Rejected alternative: fixing +provider ordering (Option B) would require deferring **all** `<bean-selection>` execution to the +end of `reloadContainer` in core — because bean-selection runs inline during document parse — +affecting every plugin's bean-selection (objectFactory, converters, …). Broad and risky for no +extra benefit. Option A is the minimal, behavior-restoring change. + +### `JSONUtil` changes + +- Remove `@Inject` from `setWriter(JSONWriter)` and `setReader(JSONReader)`, keeping them as + plain setters (still usable directly by tests / callers). +- Add an `@Inject setContainer(Container)` that resolves both from the **effective** constant + values: + + ```java + @Inject + public void setContainer(Container container) { + this.writer = container.getInstance(JSONWriter.class, + container.getInstance(String.class, JSONConstants.JSON_WRITER)); + this.reader = container.getInstance(JSONReader.class, + container.getInstance(String.class, JSONConstants.JSON_READER)); + } + ``` + +This mirrors 7.1.x. Because `container.getInstance(String.class, JSONConstants.JSON_WRITER)` +reads the constant from the fully-built container, it returns the application's override value +(e.g. `flexJSONWriter`), and `getInstance(JSONWriter.class, "flexJSONWriter")` returns the +user's bean. + +### What is intentionally *not* changed + +- `StrutsJSONWriter` / `StrutsJSONReader` remain the shipped defaults. +- The JSON DoS limits (`struts.json.maxDepth`, `maxElements`, `maxLength`, `maxStringLength`, + `maxKeyLength`) are untouched. Their `@Inject` setters still fire because the container + instantiates the writer/reader when `getInstance(...)` resolves them. +- The `<bean-selection>` / `alias()` machinery stays. It still binds + `JSONWriter/DEFAULT_NAME → StrutsJSONWriter`, but `JSONUtil` no longer relies on that default + binding. It is left in place as a harmless default so that any external code doing + `container.getInstance(JSONWriter.class)` keeps working. + +### Thread-safety + +`JSONUtil` remains `prototype` scope, and the writer/reader are resolved as `prototype` per +`JSONUtil` instance — identical lifecycle to current `main`, so no thread-safety regression for +the stateful bean-info caching (`StrutsJSONWriter.clearBeanInfoCaches()`, `JSONCacheDestroyable`). + +## Consequence for the regression test contract + +Option A does **not** change the default container binding: `container.getInstance(JSONWriter.class)` +still returns `StrutsJSONWriter`. The override is honored via by-name lookup inside `JSONUtil`. +Therefore the regression test asserts the **effective** writer/reader used by `JSONUtil`, not the +raw default binding. + +## Test plan + +New files under `plugins/json/src/test/...`: + +1. **`CustomTestJSONWriter`** — implements the current `JSONWriter` interface; `write(...)` + returns a sentinel string (`{"__customWriter__":true}`) so serialization-level assertions are + unambiguous. Implements all interface methods: + `write(Object)`, `write(Object, Collection<Pattern>, Collection<Pattern>, boolean)`, + `setIgnoreHierarchy`, `setEnumAsBean`, `setDateFormatter`, `setCacheBeanInfo`, + `setExcludeProxyProperties`. +2. **`CustomTestJSONReader`** — implements the current `JSONReader` interface as a marker + (`read(...)` returns a sentinel object; the `setMax*` setters are no-ops): + `read(String)`, `setMaxElements`, `setMaxDepth`, `setMaxStringLength`, `setMaxKeyLength`. +3. **Override config** `src/test/resources/struts-json-override.xml` — registers both custom + beans (`type=...JSONWriter`/`...JSONReader`, `scope="prototype"`) and the + `struts.json.writer` / `struts.json.reader` constants pointing at them. +4. **`JSONWriterOverrideTest extends StrutsTestCase`** — the crux is exercising the real + `Dispatcher` bootstrap so the provider ordering that causes the bug is reproduced. Uses the + config chain `struts-default.xml,struts-plugin.xml,struts-json-override.xml` (defaults → + JSON plugin's `<bean-selection>` → app override, in that order). Assertions on the + **effective** components: + - `jsonUtil.serialize(someBean, false)` returns the writer sentinel (proves the custom writer + is used). + - `jsonUtil.getReader().getClass()` is `CustomTestJSONReader` (proves the custom reader is + used). + + > Note: the `XWorkTestCase` bootstrap uses only `StrutsDefaultConfigurationProvider` and does + > **not** load the JSON plugin's `struts-plugin.xml`, so it cannot reproduce the ordering bug. + > `StrutsTestCase` (full `Dispatcher` init, classpath `struts-plugin.xml` discovery) is + > required. + +Both assertions must **FAIL on current `main`** (before the `JSONUtil` change) and **PASS after**. + +## Verification + +- New `JSONWriterOverrideTest` fails before the fix, passes after. +- Full `struts2-json-plugin` module suite green: + `mvn test -DskipAssembly -pl plugins/json` — DoS-limit and caching tests unaffected. + +## Scope guardrails + +- No change to `StrutsJSONWriter` / `StrutsJSONReader`, the DoS constants, or any core class. +- Change confined to `JSONUtil` + new test fixtures. +- Delivered via a PR from a feature branch referencing WW-5641 (never pushed to `main`).
