[
https://issues.apache.org/jira/browse/WW-5650?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart resolved WW-5650.
-------------------------------
Resolution: Fixed
> Refactor JSON plugin reader/writer to per-request instances instead of shared
> mutable state
> -------------------------------------------------------------------------------------------
>
> Key: WW-5650
> URL: https://issues.apache.org/jira/browse/WW-5650
> Project: Struts 2
> Issue Type: Improvement
> Components: Plugin - JSON
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.3.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> h2. Background
> The JSON plugin's request parser (StrutsJSONReader) and response serializer
> (StrutsJSONWriter) hold per-operation working state as instance fields:
> * StrutsJSONReader: character iterator, current char, current token, string
> buffer,
> and the nesting-depth counter used for the maxDepth/maxElements limits.
> * StrutsJSONWriter: output buffer, cyclic-reference stack, root object, and
> the
> expression-path state (buildExpr / exprStack / include-exclude patterns).
> Both are obtained through JSONUtil (getReader() / the injected writer), and
> JSONUtil
> is held once by the singleton JSONInterceptor. As a result a single reader
> and a single
> writer instance are reused across all concurrent requests handled by that
> interceptor.
> Because the working state lives on those shared instances, concurrent
> parse/serialize
> calls can interfere with each other, producing incorrect results and, for the
> reader,
> undermining the maxDepth/maxElements limits under load.
> h2. Interim fixes
> PR #1775 (WW-5643) and PR #1776 (WW-5644) address the immediate correctness
> problem by
> confining the per-operation state to a ThreadLocal, created fresh per call
> and cleared in
> a finally block. These are correct but rely on a per-request cleanup contract
> and leave
> some configuration fields shared. This ticket tracks the proper structural
> fix that
> supersedes that approach.
> h2. Proposed approach
> Stop sharing stateful reader/writer instances. The JSONReader/JSONWriter
> beans are already
> declared scope="prototype", so a fresh instance can be obtained per operation
> via the
> container. Refactor JSONUtil so that:
> * Each deserialize/serialize call obtains a fresh reader/writer, configures
> it (limits,
> ignoreHierarchy, dateFormat, include/exclude patterns, etc.), uses it, and
> discards it --
> all within a single scope. This removes the current configure-then-use split
> (applyLimitsToReader() + deserializeInput(); serialize() setters + write())
> that assumes
> a stable shared instance.
> * The standalone JSONUtil.getReader() accessor that returns a shared instance
> is deprecated
> / removed in favour of the per-operation flow.
> * The StrutsJSONReader/StrutsJSONWriter public extension points (the protected
> next()/object()/array()/string()/add()/bean()/map() methods used by
> subclasses) remain
> unchanged, so custom reader/writer subclasses and the struts.json.reader /
> struts.json.writer overrides keep working.
> * Once instances are no longer shared, the per-field ThreadLocal state
> introduced by the
> interim PRs is unwound (state returns to plain fields on the now-single-use
> instance).
> Performance note: the expensive bean-introspection cache in StrutsJSONWriter
> is a static
> ConcurrentMap shared across instances, so creating a fresh writer per request
> does not lose
> the reflection cache and is effectively free.
> h2. Scope
> * plugins/json -- JSONUtil, JSONInterceptor, StrutsJSONReader,
> StrutsJSONWriter
> * No change to public template/config surface (struts.json.reader /
> struts.json.writer,
> interceptor params).
> h2. Acceptance criteria
> * No reader/writer instance carries per-operation state across concurrent
> requests.
> * The concurrency regression tests added in #1775 and #1776 are preserved and
> pass against
> the refactored code.
> * Existing StrutsJSONReaderTest / JSONReaderTest / StrutsJSONWriterTest /
> JSONInterceptorTest
> suites pass unchanged.
> * Custom reader/writer subclasses and struts.json.reader/writer overrides
> continue to work.
> h2. Related
> * Supersedes the interim approach in #1775 (WW-5643) and #1776 (WW-5644).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
