diff --git a/docs/superpowers/plans/2026-07-06-WW-5641-json-writer-reader-override.md b/docs/superpowers/plans/2026-07-06-WW-5641-json-writer-reader-override.md new file mode 100644 index 0000000000..5b36e0bf61 --- /dev/null +++ b/docs/superpowers/plans/2026-07-06-WW-5641-json-writer-reader-override.md @@ -0,0 +1,447 @@ +# WW-5641: Restore `struts.json.writer` / `struts.json.reader` Override — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restore the documented ability to override the JSON serializer/deserializer via the `struts.json.writer` / `struts.json.reader` constants, which regressed in the 7.2.x JSON hardening rework. + +**Architecture:** The JSON plugin's `` runs at plugin-parse time (inside `struts-plugin.xml`), before the app config is folded in, so it freezes the default `JSONWriter`/`JSONReader` binding to the shipped defaults. The framework already provides the order-safe home for order-sensitive bean-selection: `struts-deferred.xml`, which `Dispatcher.init()` loads **last**. The fix moves the JSON `` from `struts-plugin.xml` into a new `struts-deferred.xml` (mirroring the velocity plugin), so `alias()` runs after the app override and binds the default to the override. `JSONUtil` reverts to its clean 7.2.x state. + +**Tech Stack:** Java, Maven, Struts internal IoC container, JUnit (`junit.framework.TestCase` via `StrutsTestCase`). + +**Spec:** `docs/superpowers/specs/2026-07-06-WW-5641-json-writer-reader-override-regression-design.md` +**Ticket:** [WW-5641](https://issues.apache.org/jira/browse/WW-5641) + +## Global Constraints + +- Commit messages MUST be prefixed with the ticket id: `WW-5641 : `. +- Work on the feature branch `WW-5641-json-writer-reader-override`. Never commit to `main`. +- Do NOT change `StrutsJSONWriter`, `StrutsJSONReader`, the DoS-limit constants (`struts.json.maxDepth`, `maxElements`, `maxLength`, `maxStringLength`, `maxKeyLength`), `JSONBeanSelectionProvider`, `AbstractBeanSelectionProvider`, or any core class. +- `JSONUtil.java` must end up byte-identical to its state on `main` (commit `789dbf3cd`): `@Inject` on `setWriter(JSONWriter)` and `setReader(JSONReader)`, no `setContainer`, no `org.apache.struts2.inject.Container` import. +- The `` writer/reader definitions, the `JSONUtil`/`JSONCacheDestroyable` beans, all constants, and the `json-default` package stay in `struts-plugin.xml` — mirror the velocity plugin, which keeps `` in `struts-plugin.xml` and only `` in `struts-deferred.xml`. +- Test idiom for this module: extend `StrutsTestCase`, name tests `testXxx()`, use inherited `assertEquals` (JUnit 3/4 style). No `@Test` annotations. +- The test `config` init param must be `struts-default.xml,struts-plugin.xml,struts-json-override.xml` — do NOT append `struts-deferred.xml` (the framework loads it unconditionally after the chain; listing it twice emits a spurious warning). + +--- + +### Task 1: Move the JSON `` to `struts-deferred.xml`, revert `JSONUtil`, prove the override wins (RED → GREEN) + +One task: revert the earlier (rejected) `JSONUtil` change, relocate the `` element, add the regression test, and verify RED→GREEN plus the full module suite. Single green commit. + +**Files:** +- Modify: `plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java` (revert to `main` state — remove `setContainer` + `Container` import, restore `@Inject` on the two setters) +- Modify: `plugins/json/src/main/resources/struts-plugin.xml` (delete the `` line) +- Create: `plugins/json/src/main/resources/struts-deferred.xml` (the moved ``) +- Create: `plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java` +- Create: `plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java` +- Create: `plugins/json/src/test/resources/struts-json-override.xml` +- Create: `plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java` + +**Interfaces:** +- Consumes: `JSONWriter` (methods: `String write(Object)`; `String write(Object, Collection, Collection, boolean)`; `setIgnoreHierarchy(boolean)`; `setEnumAsBean(boolean)`; `setDateFormatter(String)`; `setCacheBeanInfo(boolean)`; `setExcludeProxyProperties(boolean)`). `JSONReader` (methods: `Object read(String)`; `setMaxElements(int)`; `setMaxDepth(int)`; `setMaxStringLength(int)`; `setMaxKeyLength(int)`). `JSONUtil.serialize(Object, boolean)` and `JSONUtil.getReader()`. `container.getInstance(Class)`. +- Produces: no new API. The behavioral contract: `container.getInstance(JSONWriter.class)` / `getInstance(JSONReader.class)` resolve to the app override when configured. + +- [ ] **Step 1: Revert the `JSONUtil` import** + +In `plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java`, remove the `Container` import line so only the `Inject` import remains: + +```java +import org.apache.struts2.inject.Inject; +``` + +(Delete the `import org.apache.struts2.inject.Container;` line directly above it.) + +- [ ] **Step 2: Revert the `JSONUtil` setters to the `main` state** + +Replace the current block (the `setContainer` method plus the two plain setters): + +```java + @Inject + public void setContainer(Container container) { + setWriter(container.getInstance(JSONWriter.class, + container.getInstance(String.class, JSONConstants.JSON_WRITER))); + setReader(container.getInstance(JSONReader.class, + container.getInstance(String.class, JSONConstants.JSON_READER))); + } + + public void setReader(JSONReader reader) { + this.reader = reader; + } + + public JSONReader getReader() { + return reader; + } + + public void setWriter(JSONWriter writer) { + this.writer = writer; + } +``` + +with the original `main` version (restore `@Inject` on both setters, no `setContainer`): + +```java + @Inject + public void setReader(JSONReader reader) { + this.reader = reader; + } + + public JSONReader getReader() { + return reader; + } + + @Inject + public void setWriter(JSONWriter writer) { + this.writer = writer; + } +``` + +Verify with `git diff 789dbf3cd -- plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java` — it MUST print nothing (file identical to `main`). + +- [ ] **Step 3: Remove the `` from `struts-plugin.xml`** + +In `plugins/json/src/main/resources/struts-plugin.xml`, delete this line (line 68): + +```xml + +``` + +Leave everything else (the `` definitions, constants, `json-default` package) unchanged. The closing `` remains. + +- [ ] **Step 4: Create `struts-deferred.xml` with the moved element** + +Create `plugins/json/src/main/resources/struts-deferred.xml` (mirrors `plugins/velocity/src/main/resources/struts-deferred.xml`): + +```xml + + + + + + + + + +``` + +- [ ] **Step 5: Create the marker writer fixture** + +Create `plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java`: + +```java +/* + * 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.json; + +import java.util.Collection; +import java.util.regex.Pattern; + +/** + * Marker writer proving that a user-configured {@code struts.json.writer} override + * wins over the default {@link StrutsJSONWriter}. Emits a sentinel so output-level + * assertions are unambiguous. + */ +public class CustomTestJSONWriter implements JSONWriter { + + public static final String SENTINEL = "{\"__customWriter__\":true}"; + + @Override + public String write(Object object) throws JSONException { + return SENTINEL; + } + + @Override + public String write(Object object, Collection excludeProperties, + Collection includeProperties, + boolean excludeNullProperties) throws JSONException { + return SENTINEL; + } + + // Configuration setters are intentionally ignored: this marker always emits SENTINEL. + // (in-body comment required by Sonar java:S1186 — empty methods must be explained) + @Override public void setIgnoreHierarchy(boolean ignoreHierarchy) { /* no-op marker */ } + @Override public void setEnumAsBean(boolean enumAsBean) { /* no-op marker */ } + @Override public void setDateFormatter(String defaultDateFormat) { /* no-op marker */ } + @Override public void setCacheBeanInfo(boolean cacheBeanInfo) { /* no-op marker */ } + @Override public void setExcludeProxyProperties(boolean excludeProxyProperties) { /* no-op marker */ } +} +``` + +- [ ] **Step 6: Create the marker reader fixture** + +Create `plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java`: + +```java +/* + * 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.json; + +/** + * Marker reader proving that a user-configured {@code struts.json.reader} override + * wins over the default {@link StrutsJSONReader}. + */ +public class CustomTestJSONReader implements JSONReader { + + public static final String SENTINEL = "__customReader__"; + + @Override + public Object read(String string) throws JSONException { + return SENTINEL; + } + + // Limit setters are intentionally ignored: this marker always returns SENTINEL. + // (in-body comment required by Sonar java:S1186 — empty methods must be explained) + @Override public void setMaxElements(int maxElements) { /* no-op marker */ } + @Override public void setMaxDepth(int maxDepth) { /* no-op marker */ } + @Override public void setMaxStringLength(int maxStringLength) { /* no-op marker */ } + @Override public void setMaxKeyLength(int maxKeyLength) { /* no-op marker */ } +} +``` + +- [ ] **Step 7: Create the application-style override config** + +Create `plugins/json/src/test/resources/struts-json-override.xml`. The ASF license header is +required — Apache RAT (bound to `prepare-package`) flags any `src/test/resources/**/*.xml` +without it and fails the build: + +```xml + + + + + + + + + +``` + +- [ ] **Step 8: Create the regression test** + +Create `plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java`. It boots the real `Dispatcher` chain; `struts-deferred.xml` is auto-loaded last by `Dispatcher.init()` (do NOT list it in `config`). Asserts both the default container binding and the effective use: + +```java +/* + * 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.json; + +import org.apache.struts2.junit.StrutsTestCase; + +import java.util.HashMap; +import java.util.Map; + +/** + * Regression test for WW-5641: {@code struts.json.writer} / {@code struts.json.reader} + * overrides from an application config were ignored on the 7.2.x line because the JSON + * plugin's {@code } ran at plugin-parse time, before the app config. + * Moving it to {@code struts-deferred.xml} makes it run last, so the override wins. + * + *

Boots the real Dispatcher chain. {@code struts-deferred.xml} is loaded unconditionally + * by {@code Dispatcher.init()} after the {@code config} chain, so it is intentionally omitted + * from the config init param below.

+ */ +public class JSONWriterOverrideTest extends StrutsTestCase { + + @Override + protected void setupBeforeInitDispatcher() throws Exception { + Map params = new HashMap<>(); + params.put("config", "struts-default.xml,struts-plugin.xml,struts-json-override.xml"); + dispatcherInitParams = params; + } + + /** + * The default JSONWriter binding must resolve to the app override, not StrutsJSONWriter. + */ + public void testCustomWriterWinsAsDefaultBinding() { + JSONWriter writer = container.getInstance(JSONWriter.class); + assertEquals("struts.json.writer override was ignored; default StrutsJSONWriter was used", + CustomTestJSONWriter.class, writer.getClass()); + } + + /** + * The default JSONReader binding must resolve to the app override, not StrutsJSONReader. + */ + public void testCustomReaderWinsAsDefaultBinding() { + JSONReader reader = container.getInstance(JSONReader.class); + assertEquals("struts.json.reader override was ignored; default StrutsJSONReader was used", + CustomTestJSONReader.class, reader.getClass()); + } + + /** + * End-to-end: the writer JSONUtil actually serializes with must be the override. + */ + public void testJSONUtilUsesCustomWriter() throws Exception { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + assertEquals(CustomTestJSONWriter.SENTINEL, jsonUtil.serialize(new Object(), false)); + } + + /** + * End-to-end: the reader JSONUtil exposes must be the override. + */ + public void testJSONUtilUsesCustomReader() { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + assertEquals(CustomTestJSONReader.class, jsonUtil.getReader().getClass()); + } +} +``` + +- [ ] **Step 9: Verify the test reproduces the bug BEFORE the resource move** + +To confirm the test is a real reproduction, temporarily stash only the two resource changes and run the test against the old wiring: + +```bash +git stash push -- plugins/json/src/main/resources/struts-plugin.xml plugins/json/src/main/resources/struts-deferred.xml +mvn test -DskipAssembly -pl plugins/json -Dtest=JSONWriterOverrideTest +git stash pop +``` + +Expected with the resources stashed (old wiring, `` still in `struts-plugin.xml`, no deferred file): all four tests FAIL — default bindings resolve to `StrutsJSONWriter`/`StrutsJSONReader` and `serialize` returns `{}`. If any test passes here, the bootstrap is not reproducing the ordering — stop and re-check the `config` param before proceeding. (The `JSONUtil` revert from Steps 1-2 stays in place during this check; it is not what causes the bug.) + +- [ ] **Step 10: Run the test to verify it PASSES with the fix in place** + +Run: `mvn test -DskipAssembly -pl plugins/json -Dtest=JSONWriterOverrideTest` +Expected: all four tests PASS. Output pristine — no "default mapping already assigned" warning for `JSONWriter`/`JSONReader`. + +- [ ] **Step 11: Run the full JSON plugin suite AND the license check** + +Run: `mvn test -DskipAssembly -pl plugins/json` +Expected: BUILD SUCCESS, all tests pass (DoS-limit, bean-info caching, and the `new JSONUtil()` + `setWriter`/`setReader` tests are all unaffected). + +Then run the Apache RAT license check — it binds to `prepare-package`, so `mvn test` does NOT +exercise it, and a new resource file missing the ASF header will pass tests but fail CI/`install`: + +Run: `mvn apache-rat:check -pl plugins/json` +Expected: BUILD SUCCESS (exit 0), no `UNAPPROVED` files. If it flags `struts-json-override.xml`, +the ASF header from Step 7 is missing — add it and re-run. + +- [ ] **Step 12: Commit** + +```bash +git add plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java \ + plugins/json/src/main/resources/struts-plugin.xml \ + plugins/json/src/main/resources/struts-deferred.xml \ + plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java \ + plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java \ + plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java \ + plugins/json/src/test/resources/struts-json-override.xml +git commit -m "WW-5641 fix: run JSON bean-selection from struts-deferred.xml + +The JSON plugin declared in struts-plugin.xml, which runs +at plugin-parse time, before the application struts.xml is folded in. That +froze the JSONWriter/JSONReader default binding to StrutsJSONWriter/Reader, +so struts.json.writer / struts.json.reader overrides were ignored. + +Move the element to struts-deferred.xml, which Dispatcher loads last (after +the app config and core's StrutsBeanSelectionProvider), so the alias honors +the override. Mirrors the velocity plugin. JSONUtil is unchanged from main. + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Self-Review + +**Spec coverage:** +- Move `` to `struts-deferred.xml` → Steps 3-4. ✓ +- Revert `JSONUtil` to `main` state → Steps 1-2 + git-diff verification. ✓ +- Keep `` defs/constants in `struts-plugin.xml` (velocity mirror) → Step 3 leaves them; Global Constraints. ✓ +- No core / `JSONBeanSelectionProvider` / DoS / `StrutsJSONWriter` changes → no task touches them; Global Constraints forbids it. ✓ +- Regression test asserts BOTH default binding AND effective use, for writer AND reader → Step 8 (4 tests). ✓ +- `StrutsTestCase` real Dispatcher; `struts-deferred.xml` not in `config` param → Step 8 + Global Constraints + Step 10 pristine-output check. ✓ +- FAIL-before / PASS-after → Steps 9 and 10. ✓ +- Full module suite green → Step 11. ✓ + +**Placeholder scan:** No TBD/TODO; every code/resource step shows complete content. ✓ + +**Type consistency:** Fixture method signatures match the `JSONWriter`/`JSONReader` interfaces. `serialize(Object, boolean)` and `getReader()` exist on `JSONUtil`. `container.getInstance(Class)` used for default-binding assertions. Bean names (`customTestWriter`/`customTestReader`) match between the override XML and the constant values. The deferred element's `name`/`class` match the line removed from `struts-plugin.xml`. ✓ 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 0000000000..5904eecdfd --- /dev/null +++ b/docs/superpowers/specs/2026-07-06-WW-5641-json-writer-reader-override-regression-design.md @@ -0,0 +1,161 @@ +# 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 + + +``` + +This worked on 7.1.x and regressed in 7.2.x. The extension point is still documented at +, 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`) + +The 7.2.x JSON hardening rework added a `` element to the +plugin's **`struts-plugin.xml`** to select the `JSONWriter` / `JSONReader` default binding via +`JSONBeanSelectionProvider` → `AbstractBeanSelectionProvider.alias()`. The defect is *where* +that element lives, which determines *when* it runs. + +- `XmlDocConfigurationProvider.registerBeanSelection()` (`:215-222`) invokes the bean-selection + provider **inline, the moment the `` element is parsed** inside the JSON + plugin's `struts-plugin.xml`. +- In `Dispatcher.init()` the config sources are registered in this order (see + `Dispatcher.java:696-704`): `struts-default.xml` → all `struts-plugin.xml` (plugin discovery) + → the app's `struts.xml` (`init_TraditionalXmlConfigurations`) → core's + `StrutsBeanSelectionProvider` (`init_AliasStandardObjects`) → `struts-deferred.xml` + (`init_DeferredXmlConfigurations`). +- So the JSON `` fires while parsing `struts-plugin.xml`, **before** the app's + `struts.xml` folds its `flexJSONWriter` bean and its `struts.json.writer` constant into the + shared `props`/builder. `alias()` reads `props.getProperty("struts.json.writer", "struts")` = + `"struts"` (the plugin default) and locks `JSONWriter/DEFAULT_NAME → StrutsJSONWriter`. + `alias()` binds the default name only once (guarded by `!builder.contains(type, DEFAULT_NAME)`), + so nothing re-selects when the app config loads afterward. `JSONUtil` injects that frozen + default binding and never sees the override. + +The 7.1.x code was immune because it 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: run the JSON `` late, via `struts-deferred.xml` + +The framework already provides the idiomatic, order-safe mechanism for exactly this. From +`Dispatcher.java`: + +> `struts-deferred.xml` can be used to load configuration which is sensitive to loading order +> such as 'bean-selection' elements + +`init_DeferredXmlConfigurations()` loads `struts-deferred.xml` **last** — after the app's +`struts.xml` and after core's `StrutsBeanSelectionProvider`. A `` placed there +runs once the app's beans and constants are already in `props`/builder, so `alias()` reads the +*effective* `struts.json.writer` value and aliases `JSONWriter/DEFAULT_NAME` to the application's +override. + +This is the established convention in-tree: the **velocity plugin** keeps its `` +definitions in `struts-plugin.xml` and places its `` +in `plugins/velocity/src/main/resources/struts-deferred.xml`. The JSON plugin is the outlier +that put `` in `struts-plugin.xml`. Core's own `StrutsBeanSelectionProvider` uses +the same late-running principle (registered by `init_AliasStandardObjects`, after all XML), +which is why application overrides of core beans (`struts.objectFactory`, converters, …) work. + +### Changes + +1. **Remove** the `` element from + `plugins/json/src/main/resources/struts-plugin.xml` (line 68). Leave the `` + writer/reader definitions, the `JSONUtil`/`JSONCacheDestroyable` beans, the constants, and the + `json-default` package exactly as they are — mirroring velocity. +2. **Create** `plugins/json/src/main/resources/struts-deferred.xml` containing the moved + ``. +3. **Revert `JSONUtil` to the 7.2.x state**: restore `@Inject` on `setWriter(JSONWriter)` and + `setReader(JSONReader)`, and remove the `setContainer(Container)` method and the + `org.apache.struts2.inject.Container` import introduced by the earlier (rejected) attempt. + No Java source change remains — `JSONUtil` again injects the (now correct) default binding. + +### Why this over resolving by-name inside `JSONUtil` + +An alternative (rejected) was to have `JSONUtil` inject the `Container` and resolve the +writer/reader by the constant value at runtime. That works but sidesteps the framework's standard +bean-override mechanism, leaves the default `JSONWriter`/`JSONReader` container binding pointing +at the wrong implementation, and only fixes `JSONUtil`'s own consumption. The deferred +`` fixes the binding itself: `container.getInstance(JSONWriter.class)` returns the +override for *any* consumer, `JSONUtil` needs no change, and the plugin now matches velocity and +core convention. + +### What is intentionally *not* changed + +- `StrutsJSONWriter` / `StrutsJSONReader` remain the shipped defaults (they are the `name="struts"` + beans, aliased to the default only when no app override is present). +- The JSON DoS limits (`struts.json.maxDepth`, `maxElements`, `maxLength`, `maxStringLength`, + `maxKeyLength`) and their `@Inject` setters are untouched. +- No core class changes. `JSONBeanSelectionProvider` and `AbstractBeanSelectionProvider` are + unchanged — only *when* the provider runs changes, driven by which config file carries the + element. + +### Thread-safety + +`JSONUtil` remains `prototype`; the writer/reader remain `prototype` (aliased with +`Scope.PROTOTYPE`). Lifecycle is identical to current `main` — no thread-safety change for the +stateful bean-info caching (`StrutsJSONWriter.clearBeanInfoCaches()`, `JSONCacheDestroyable`). + +## Test plan + +New files under `plugins/json/src/test/...`: + +1. **`CustomTestJSONWriter`** — implements the current `JSONWriter` interface; `write(...)` + returns a sentinel (`{"__customWriter__":true}`) for unambiguous serialization assertions. + Implements: `write(Object)`, `write(Object, Collection, Collection, boolean)`, + `setIgnoreHierarchy`, `setEnumAsBean`, `setDateFormatter`, `setCacheBeanInfo`, + `setExcludeProxyProperties`. +2. **`CustomTestJSONReader`** — implements the current `JSONReader` interface as a marker: + `read(String)` returns a sentinel, `setMaxElements`/`setMaxDepth`/`setMaxStringLength`/ + `setMaxKeyLength` are no-ops. +3. **Override config** `src/test/resources/struts-json-override.xml` — registers both custom + beans (`scope="prototype"`) and the `struts.json.writer` / `struts.json.reader` constants + pointing at them. +4. **`JSONWriterOverrideTest extends StrutsTestCase`** — boots the real `Dispatcher` config + chain so the deferred `struts-deferred.xml` runs after the override. The config init param is + `struts-default.xml,struts-plugin.xml,struts-json-override.xml` (defaults → JSON plugin beans + → app override). `struts-deferred.xml` is **not** listed: `Dispatcher.init()` always runs + `init_DeferredXmlConfigurations()` after the `config` chain, so the JSON plugin's + `struts-deferred.xml` (on the classpath) loads last on its own — listing it explicitly would + load it twice and emit a spurious "default mapping already assigned" warning. Because the fix + now corrects the binding itself, the test asserts at **both** levels: + - **Default container binding:** `container.getInstance(JSONWriter.class)` is a + `CustomTestJSONWriter`; `container.getInstance(JSONReader.class)` is a `CustomTestJSONReader`. + - **Effective use:** `jsonUtil.serialize(someBean, false)` returns the writer sentinel and + `jsonUtil.getReader().getClass()` is `CustomTestJSONReader`. + + > Note: `XWorkTestCase` alone uses only `StrutsDefaultConfigurationProvider` and loads neither + > the plugin `struts-plugin.xml` nor `struts-deferred.xml`, so it cannot reproduce the ordering. + > `StrutsTestCase` (full `Dispatcher` init) is required. + +Both assertions must **FAIL on current `main`** (before moving the element) 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, `JSONUtil` (net-zero + after revert), `JSONBeanSelectionProvider`, or any core class. +- Change is confined to two plugin resource files (`struts-plugin.xml`, new `struts-deferred.xml`) + plus new test fixtures. +- Delivered via a PR from a feature branch referencing WW-5641 (never pushed to `main`). diff --git a/plugins/json/src/main/resources/struts-deferred.xml b/plugins/json/src/main/resources/struts-deferred.xml new file mode 100644 index 0000000000..b9e8bd9a6d --- /dev/null +++ b/plugins/json/src/main/resources/struts-deferred.xml @@ -0,0 +1,30 @@ + + + + + + + + + diff --git a/plugins/json/src/main/resources/struts-plugin.xml b/plugins/json/src/main/resources/struts-plugin.xml index 3d246ff368..2cb27dcae3 100644 --- a/plugins/json/src/main/resources/struts-plugin.xml +++ b/plugins/json/src/main/resources/struts-plugin.xml @@ -64,6 +64,4 @@ - - diff --git a/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java b/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java new file mode 100644 index 0000000000..d39a166de4 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONReader.java @@ -0,0 +1,39 @@ +/* + * 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.json; + +/** + * Marker reader proving that a user-configured {@code struts.json.reader} override + * wins over the default {@link StrutsJSONReader}. + */ +public class CustomTestJSONReader implements JSONReader { + + public static final String SENTINEL = "__customReader__"; + + @Override + public Object read(String string) throws JSONException { + return SENTINEL; + } + + // Limit setters are intentionally ignored: this marker always returns SENTINEL. + @Override public void setMaxElements(int maxElements) { /* no-op marker */ } + @Override public void setMaxDepth(int maxDepth) { /* no-op marker */ } + @Override public void setMaxStringLength(int maxStringLength) { /* no-op marker */ } + @Override public void setMaxKeyLength(int maxKeyLength) { /* no-op marker */ } +} diff --git a/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java b/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java new file mode 100644 index 0000000000..d201b3bd42 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/CustomTestJSONWriter.java @@ -0,0 +1,51 @@ +/* + * 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.json; + +import java.util.Collection; +import java.util.regex.Pattern; + +/** + * Marker writer proving that a user-configured {@code struts.json.writer} override + * wins over the default {@link StrutsJSONWriter}. Emits a sentinel so output-level + * assertions are unambiguous. + */ +public class CustomTestJSONWriter implements JSONWriter { + + public static final String SENTINEL = "{\"__customWriter__\":true}"; + + @Override + public String write(Object object) throws JSONException { + return SENTINEL; + } + + @Override + public String write(Object object, Collection excludeProperties, + Collection includeProperties, + boolean excludeNullProperties) throws JSONException { + return SENTINEL; + } + + // Configuration setters are intentionally ignored: this marker always emits SENTINEL. + @Override public void setIgnoreHierarchy(boolean ignoreHierarchy) { /* no-op marker */ } + @Override public void setEnumAsBean(boolean enumAsBean) { /* no-op marker */ } + @Override public void setDateFormatter(String defaultDateFormat) { /* no-op marker */ } + @Override public void setCacheBeanInfo(boolean cacheBeanInfo) { /* no-op marker */ } + @Override public void setExcludeProxyProperties(boolean excludeProxyProperties) { /* no-op marker */ } +} diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java new file mode 100644 index 0000000000..155bf793b7 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java @@ -0,0 +1,78 @@ +/* + * 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.json; + +import org.apache.struts2.junit.StrutsTestCase; + +import java.util.HashMap; +import java.util.Map; + +/** + * Regression test for WW-5641: {@code struts.json.writer} / {@code struts.json.reader} + * overrides from an application config were ignored on the 7.2.x line because the JSON + * plugin's {@code } ran at plugin-parse time, before the app config. + * Moving it to {@code struts-deferred.xml} makes it run last, so the override wins. + * + *

Boots the real Dispatcher chain. {@code struts-deferred.xml} is loaded unconditionally + * by {@code Dispatcher.init()} after the {@code config} chain, so it is intentionally omitted + * from the config init param below.

+ */ +public class JSONWriterOverrideTest extends StrutsTestCase { + + @Override + protected void setupBeforeInitDispatcher() throws Exception { + Map params = new HashMap<>(); + params.put("config", "struts-default.xml,struts-plugin.xml,struts-json-override.xml"); + dispatcherInitParams = params; + } + + /** + * The default JSONWriter binding must resolve to the app override, not StrutsJSONWriter. + */ + public void testCustomWriterWinsAsDefaultBinding() { + JSONWriter writer = container.getInstance(JSONWriter.class); + assertEquals("struts.json.writer override was ignored; default StrutsJSONWriter was used", + CustomTestJSONWriter.class, writer.getClass()); + } + + /** + * The default JSONReader binding must resolve to the app override, not StrutsJSONReader. + */ + public void testCustomReaderWinsAsDefaultBinding() { + JSONReader reader = container.getInstance(JSONReader.class); + assertEquals("struts.json.reader override was ignored; default StrutsJSONReader was used", + CustomTestJSONReader.class, reader.getClass()); + } + + /** + * End-to-end: the writer JSONUtil actually serializes with must be the override. + */ + public void testJSONUtilUsesCustomWriter() throws Exception { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + assertEquals(CustomTestJSONWriter.SENTINEL, jsonUtil.serialize(new Object(), false)); + } + + /** + * End-to-end: the reader JSONUtil exposes must be the override. + */ + public void testJSONUtilUsesCustomReader() { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + assertEquals(CustomTestJSONReader.class, jsonUtil.getReader().getClass()); + } +} diff --git a/plugins/json/src/test/resources/struts-json-override.xml b/plugins/json/src/test/resources/struts-json-override.xml new file mode 100644 index 0000000000..097b16484b --- /dev/null +++ b/plugins/json/src/test/resources/struts-json-override.xml @@ -0,0 +1,32 @@ + + + + + + + + +