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 29bfea5a6c3b0453a100945ecd810b79cefc6f8a Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Jul 6 19:25:30 2026 +0200 WW-5641 docs: implementation plan for JSON writer/reader override fix Co-Authored-By: Claude Opus 4.8 <[email protected]> --- ...26-07-06-WW-5641-json-writer-reader-override.md | 337 +++++++++++++++++++++ 1 file changed, 337 insertions(+) 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 000000000..90829da20 --- /dev/null +++ b/docs/superpowers/plans/2026-07-06-WW-5641-json-writer-reader-override.md @@ -0,0 +1,337 @@ +# 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:** Revert `JSONUtil` to 7.1.x-style deferred resolution: inject the `Container` and resolve the writer/reader by the *effective* constant value at injection time, instead of relying on the build-time default alias set by `JSONBeanSelectionProvider`. Change is confined to `JSONUtil` plus new test fixtures. `StrutsJSONWriter`/`StrutsJSONReader`, the DoS-limit constants, and the `<bean-selection>` machinery are untouched. + +**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 <type>: <desc>`. +- Work on the feature branch `WW-5641-json-writer-reader-override` (already checked out). Never commit to `main`. +- Do NOT change `StrutsJSONWriter`, `StrutsJSONReader`, the DoS-limit constants (`struts.json.maxDepth`, `maxElements`, `maxLength`, `maxStringLength`, `maxKeyLength`), or any core class. +- `JSONUtil.setWriter(JSONWriter)` and `JSONUtil.setReader(JSONReader)` MUST remain public plain setters — existing tests (`JSONResultTest`, `JSONInterceptorTest`, `JSONUtilTest`) call them directly on `new JSONUtil()`. Only the `@Inject` annotation is removed from them. +- `JSONUtil` stays `prototype` scope; the writer/reader stay `prototype`. `serialize(...)` must not depend on `container` (only the injected `writer`/`reader` fields). +- Test idiom for this module: extend `StrutsTestCase`, name tests `testXxx()`, use inherited `assertEquals` (JUnit 3/4 style). No `@Test` annotations. + +--- + +### Task 1: Reproduce the regression, then fix it (RED → GREEN) + +The fix (~6 lines) and its regression test are inseparable — neither is independently shippable — so they form one task with a full TDD cycle and a single green commit. + +**Files:** +- 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` +- Modify: `plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java` (imports; `setReader` `:65-68`; `setWriter` `:74-77`) + +**Interfaces:** +- Consumes: `org.apache.struts2.json.JSONWriter` (methods: `String write(Object)`; `String write(Object, Collection<Pattern>, Collection<Pattern>, boolean)`; `void setIgnoreHierarchy(boolean)`; `void setEnumAsBean(boolean)`; `void setDateFormatter(String)`; `void setCacheBeanInfo(boolean)`; `void setExcludeProxyProperties(boolean)`). `org.apache.struts2.json.JSONReader` (methods: `Object read(String)`; `void setMaxElements(int)`; `void setMaxDepth(int)`; `void setMaxStringLength(int)`; ` [...] +- Produces: the corrected `JSONUtil.setContainer(Container)` injection point. No new API for later tasks. + +- [ ] **Step 1: 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<Pattern> excludeProperties, + Collection<Pattern> includeProperties, + boolean excludeNullProperties) throws JSONException { + return SENTINEL; + } + + @Override public void setIgnoreHierarchy(boolean ignoreHierarchy) {} + @Override public void setEnumAsBean(boolean enumAsBean) {} + @Override public void setDateFormatter(String defaultDateFormat) {} + @Override public void setCacheBeanInfo(boolean cacheBeanInfo) {} + @Override public void setExcludeProxyProperties(boolean excludeProxyProperties) {} +} +``` + +- [ ] **Step 2: 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; + } + + @Override public void setMaxElements(int maxElements) {} + @Override public void setMaxDepth(int maxDepth) {} + @Override public void setMaxStringLength(int maxStringLength) {} + @Override public void setMaxKeyLength(int maxKeyLength) {} +} +``` + +- [ ] **Step 3: Create the application-style override config** + +Create `plugins/json/src/test/resources/struts-json-override.xml`. This mimics an application overriding both components the documented way; it is loaded *after* the JSON plugin's `struts-plugin.xml` (see Step 4), reproducing the real provider ordering: + +```xml +<?xml version="1.0" encoding="UTF-8" ?> +<!DOCTYPE struts PUBLIC + "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" + "https://struts.apache.org/dtds/struts-6.0.dtd";> +<struts> + <bean type="org.apache.struts2.json.JSONWriter" name="customTestWriter" + class="org.apache.struts2.json.CustomTestJSONWriter" scope="prototype"/> + <bean type="org.apache.struts2.json.JSONReader" name="customTestReader" + class="org.apache.struts2.json.CustomTestJSONReader" scope="prototype"/> + <constant name="struts.json.writer" value="customTestWriter"/> + <constant name="struts.json.reader" value="customTestReader"/> +</struts> +``` + +- [ ] **Step 4: Create the failing regression test** + +Create `plugins/json/src/test/java/org/apache/struts2/json/JSONWriterOverrideTest.java`. It boots the real `Dispatcher` config chain `struts-default.xml,struts-plugin.xml,struts-json-override.xml` — defaults → JSON plugin `<bean-selection>` → app override, in that order — which is what reproduces the bug (`XWorkTestCase` alone does not load the plugin `struts-plugin.xml`). Assertions check the *effective* writer/reader used by `JSONUtil`: + +```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. + * + * <p>Boots the real Dispatcher config chain so the provider ordering that causes the bug + * (JSON plugin {@code <bean-selection>} running before the app override) is reproduced.</p> + */ +public class JSONWriterOverrideTest extends StrutsTestCase { + + @Override + protected void setupBeforeInitDispatcher() throws Exception { + Map<String, String> params = new HashMap<>(); + params.put("config", "struts-default.xml,struts-plugin.xml,struts-json-override.xml"); + dispatcherInitParams = params; + } + + /** + * The effective writer used by JSONUtil must be the override, not StrutsJSONWriter. + */ + public void testCustomWriterIsUsedBySerialize() throws Exception { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + String output = jsonUtil.serialize(new Object(), false); + assertEquals("struts.json.writer override was ignored; default StrutsJSONWriter was used", + CustomTestJSONWriter.SENTINEL, output); + } + + /** + * The effective reader used by JSONUtil must be the override, not StrutsJSONReader. + */ + public void testCustomReaderIsUsed() { + JSONUtil jsonUtil = container.getInstance(JSONUtil.class); + assertEquals("struts.json.reader override was ignored; default StrutsJSONReader was used", + CustomTestJSONReader.class, jsonUtil.getReader().getClass()); + } +} +``` + +- [ ] **Step 5: Run the test to verify it FAILS (reproduces the bug)** + +Run: `mvn test -DskipAssembly -pl plugins/json -Dtest=JSONWriterOverrideTest` +Expected: BOTH tests FAIL. +- `testCustomWriterIsUsedBySerialize`: actual output is `{}` (StrutsJSONWriter on an empty `Object`), not the sentinel. +- `testCustomReaderIsUsed`: `getReader()` returns a `StrutsJSONReader`, not `CustomTestJSONReader`. + +If either test unexpectedly PASSES, the bootstrap is not exercising the real ordering — stop and re-check the `config` init param before proceeding. + +- [ ] **Step 6: Apply the fix — add the `Container` import to `JSONUtil`** + +In `plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java`, add the import next to the existing `org.apache.struts2.inject.Inject` import (line 43): + +```java +import org.apache.struts2.inject.Container; +import org.apache.struts2.inject.Inject; +``` + +- [ ] **Step 7: Apply the fix — defer writer/reader resolution to a container lookup** + +Replace the current annotated setters (`JSONUtil.java:65-77`): + +```java + @Inject + public void setReader(JSONReader reader) { + this.reader = reader; + } + + public JSONReader getReader() { + return reader; + } + + @Inject + public void setWriter(JSONWriter writer) { + this.writer = writer; + } +``` + +with (remove `@Inject` from both setters; add `setContainer` that resolves by the effective constant value — mirroring 7.1.x): + +```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; + } +``` + +Rationale: `container.getInstance(String.class, "struts.json.writer")` reads the constant from the fully-built container, so it returns the application's override value (`customTestWriter`); `getInstance(JSONWriter.class, "customTestWriter")` then returns the user's bean. The DoS-limit `@Inject` setters on the resolved `StrutsJSONWriter`/`StrutsJSONReader` still fire because the container instantiates whatever bean it resolves. Reusing `setWriter`/`setReader` keeps the field assignments [...] + +- [ ] **Step 8: Run the regression test to verify it PASSES** + +Run: `mvn test -DskipAssembly -pl plugins/json -Dtest=JSONWriterOverrideTest` +Expected: BOTH tests PASS. + +- [ ] **Step 9: Run the full JSON plugin suite to verify no regressions** + +Run: `mvn test -DskipAssembly -pl plugins/json` +Expected: BUILD SUCCESS, all tests pass — including the DoS-limit and bean-info caching tests, and the `new JSONUtil()` + `setWriter(...)`/`setReader(...)` tests in `JSONResultTest` / `JSONInterceptorTest` / `JSONUtilTest` (unaffected because the setters remain public and `serialize` never touches `container`). + +- [ ] **Step 10: Commit the fix and its regression test together** + +```bash +git add plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java \ + 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: resolve JSON writer/reader from constant at runtime + +Restore deferred by-name resolution in JSONUtil so struts.json.writer and +struts.json.reader overrides from an application config are honored again. +The 7.2.x <bean-selection> alias is chosen at plugin-parse time, before the +app config is folded in, so it locked the default StrutsJSONWriter/Reader. + +Co-Authored-By: Claude Opus 4.8 <[email protected]>" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Root cause / Option A fix in `JSONUtil` → Steps 6-7. ✓ +- "Only `@Inject` removed, setters stay public" constraint → Step 7 + Global Constraints. ✓ +- `<bean-selection>` / `StrutsJSONWriter` / DoS limits untouched → no task modifies them; Global Constraints forbids it. ✓ +- Thread-safety (prototype preserved, `serialize` container-free) → Global Constraints + Step 9 verification. ✓ +- Regression test covers BOTH writer and reader → Steps 1-4, asserted in Step 4. ✓ +- `StrutsTestCase` bootstrap (real Dispatcher ordering), not `XWorkTestCase` → Step 4 + Step 5 guard. ✓ +- Effective-writer/reader test contract (not default binding) → Step 4 assertions. ✓ +- Verify FAIL-before / PASS-after → Steps 5 and 8. ✓ +- Full module suite green → Step 9. ✓ + +**Placeholder scan:** No TBD/TODO/"handle edge cases"; every code step shows complete code. ✓ + +**Type consistency:** `JSONWriter`/`JSONReader` method signatures in the fixtures match the interfaces verified in the repo. `serialize(Object, boolean)` and `getReader()` exist on `JSONUtil`. `Container.getInstance(Class)` / `getInstance(Class, String)` and `JSONConstants.JSON_WRITER`/`JSON_READER` names are exact. Bean names (`customTestWriter`/`customTestReader`) match between the override XML and the constant values. ✓
