[
https://issues.apache.org/jira/browse/WW-5700?focusedWorklogId=1038233&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1038233
]
ASF GitHub Bot logged work on WW-5700:
--------------------------------------
Author: ASF GitHub Bot
Created on: 27/Aug/26 08:32
Start Date: 27/Aug/26 08:32
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1873:
URL: https://github.com/apache/struts/pull/1873
Fixes [WW-5700](https://issues.apache.org/jira/browse/WW-5700)
### Problem
`XWorkConverter.convertValue()` signals a failed conversion by returning
`TypeConverter.NO_CONVERSION_POSSIBLE` — which is itself a plain `String`,
`"ognl.NoConversionPossible"`. `XWorkMapPropertyAccessor` and
`XWorkListPropertyAccessor` stored that return value into the target
collection
without checking for it.
Generics are erased at that point, so the `put` succeeds silently and the
`ClassCastException` only surfaces later, when application code reads the
entry
back — with a stack trace pointing away from the framework, which makes this
very hard to recognise from a bug report.
Reported on user@ as *"Struts setting a String object instead of Integer in
the
form"*: an unchecked `<s:checkbox submitUnchecked="true"/>` causes
`CheckboxInterceptor` to submit its `uncheckedValue`, `"false"`, which cannot
become an `Integer`.
### Fix
Guard both accessors and skip the assignment. The conversion error has
already
been registered by `convertValue()`, so nothing is lost — validation-driven
actions see no change at all.
Details worth a reviewer's attention:
- The map accessor guards the **key** as well as the value. An unconvertible
key
poisons iteration over the whole map rather than a single entry, and is
reachable because `ACCEPTED_PATTERNS` is asymmetric: the bare-bracket
branch
accepts digits only, `(\[\d+])`, but the quoted-key branch accepts word
characters, `(\['(\w-?|[一-龥]-?)+'])`. So `capDeferral['abc']` is an
accepted
parameter name even where the declared key type is numeric.
- **Identity comparison**, not `equals()`, matching OGNL's own guard in
`OgnlRuntime`. The constant is a String literal and the converter returns
that
exact reference, so `equals()` would silently discard a form legitimately
submitting the text `ognl.NoConversionPossible` into a String-valued
collection.
- The guard **skips rather than throws**: on the
`XWorkMethodAccessor.callMethod`
path an exception here would be swallowed and only logged.
- In the list accessor the guard sits **before** the auto-grow block, so an
unconvertible value does not grow the list.
### Not included
`XWorkCollectionPropertyAccessor` carries the same unguarded pattern, but its
scalar `setProperty` path is not reachable through the value stack —
`ids[0]` on
a `Set` is rejected by OGNL before it gets there. No failing test could be
written, so it is deliberately left untouched rather than changed blind.
### Precedent
[WW-3762](https://issues.apache.org/jira/browse/WW-3762) fixed this same bug
class
in `XWorkBasicConverter.doConvertToCollection` in 2.3.3, and
`CollectionConverter`
still carries that guard today, three times. The property accessors were
simply
never given the equivalent check.
### Testing
Written test-first; each test was watched failing for the right reason
before the
fix, with a valid entry binding first so none can pass vacuously. For the
end-to-end test the production change was stashed to confirm it fails
without it.
- `XWorkMapPropertyAccessorTest` — unconvertible value, unconvertible key
- `XWorkListPropertyAccessorTest` — unconvertible element
- `ParametersInterceptorTest` — the reported checkbox scenario, end to end
Full core suite: **3201 tests, 0 failures**.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1038233)
Remaining Estimate: 0h
Time Spent: 10m
> Failed type conversion stores the NO_CONVERSION_POSSIBLE marker string into
> typed Maps, Lists and Collections
> -------------------------------------------------------------------------------------------------------------
>
> Key: WW-5700
> URL: https://issues.apache.org/jira/browse/WW-5700
> Project: Struts 2
> Issue Type: Bug
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Summary
> When conversion of a request parameter into a typed collection fails, Struts
> stores its internal
> "conversion failed" marker into the collection instead of skipping the
> assignment. The marker is
> itself a {{java.lang.String}}, so it lands in a collection declared to hold
> some other type. Because
> generics are erased at that point the store succeeds silently, and the
> {{ClassCastException}} is
> deferred until application code reads the entry back.
> The resulting stack trace points at _application_ code rather than at Struts,
> which makes this very
> hard to recognise from a bug report.
> Reported on the user list:
> "Struts setting a String object instead of Integer in the form" (2026-08-14),
> against 7.2.1 and 7.3.0.
> h2. Root cause
> {{TypeConverter.NO\_CONVERSION\_POSSIBLE}} is not a sentinel object. It is a
> plain String:
> {code:java}
> // core/src/main/java/org/apache/struts2/conversion/TypeConverter.java:49
> Object NO\_CONVERSION\_POSSIBLE = "ognl.NoConversionPossible";
> {code}
> {{XWorkConverter.convertValue}} returns it on failure (lines 339, 351, 361),
> _after_ correctly
> registering the conversion error via {{handleConversionException}}. Three
> property accessors then
> store that return value with no guard:
> * {{XWorkMapPropertyAccessor.setProperty}} (~line 127) - both {{getKey()}}
> and {{getValue()}} unguarded
> * {{XWorkListPropertyAccessor.getRealValue}} (line 188)
> * {{XWorkCollectionPropertyAccessor.getRealValue}} (line 263)
> By contrast OGNL itself guards this correctly at {{OgnlRuntime}} line 1323,
> which is why a plain
> (non-collection) property is left untouched on a failed conversion.
> h2. Reproduced
> On {{main}} at 05ad78a06, end-to-end through {{ParametersInterceptor}} with
> {{struts.parameters.requireAnnotations=true}}. Both halves reproduce.
> _Value half_ - the reporter's case. An unchecked {{s:checkbox}} with
> {{submitUnchecked="true"}}
> causes {{CheckboxInterceptor}} to submit the parameter with its
> {{uncheckedValue}}, default
> {{"false"}}. Bound into a HashMap with Long keys and Integer values:
> {noformat}
> key=100 value=[1] (java.lang.Integer)
> key=200 value=[ognl.NoConversionPossible] (java.lang.String)
> conversionErrors={capDeferral[200]=ConversionData@...}
> {noformat}
> _Key half_ - the marker is stored as a map _key_, which breaks iteration over
> the entire map rather
> than a single entry:
> {noformat}
> acceptable=[capDeferral['abc'], capDeferral[7]]
> key=[ognl.NoConversionPossible] (java.lang.String) value=[1]
> key=[7] (java.lang.Long) value=[2]
> conversionErrors={capDeferral['abc']=ConversionData@...}
> {noformat}
> The key half is reachable because {{ACCEPTED\_PATTERNS}} in
> {{DefaultAcceptedPatternsChecker}} is asymmetric: the bare-bracket branch
> accepts digits only,
> {{(\[\d+])}}, but the quoted-key branch accepts word characters,
> {{(\['(\w-?|[一-龥]-?)+'])}}. So {{capDeferral['abc']}} is an accepted
> parameter name even
> where the declared map key type is numeric, and nothing downstream re-checks
> the key against that
> type. This is worth stating explicitly because the natural "map indices are
> numeric" intuition does
> not hold.
> Note the conversion error _is_ reported in both halves. This is therefore not
> a validation bypass:
> it only bites an action that reads the collection without acting on
> conversion errors.
> h2. Proposed fix
> Guard for the marker and skip the store; the error has already been
> registered, so nothing is lost.
> {code:java}
> Object key = getKey(context, name);
> if (key == TypeConverter.NO\_CONVERSION\_POSSIBLE) {
> return;
> }
> Object convertedValue = getValue(context, value);
> if (convertedValue == TypeConverter.NO\_CONVERSION\_POSSIBLE) {
> return;
> }
> map.put(key, convertedValue);
> {code}
> Same guard in the List and Collection accessors, placed immediately after
> {{getRealValue}} and, in
> the List case, _before_ the auto-grow block so an unconvertible value does
> not grow the list.
> Use identity comparison rather than {{equals}}, matching OGNL's own guard:
> the constant is a String
> literal and the converter returns that exact reference, so with {{equals}} a
> form legitimately
> submitting the text "ognl.NoConversionPossible" into a String-valued map
> would be silently
> discarded.
> h2. Precedent
> WW-3762 fixed exactly this class of bug in
> {{XWorkBasicConverter.doConvertToCollection}} back in
> 2.3.3. {{CollectionConverter}} still carries that guard today, three times.
> The property accessors
> were simply never given the equivalent check.
> h2. Backward compatibility
> Narrow. Previously an unconvertible entry was stored as the marker String;
> now nothing is stored.
> Nothing can reasonably depend on the old behaviour, and the conversion error
> is reported either way,
> so validation-driven actions see no change at all.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
