Lukasz Lenart created WW-5700:
---------------------------------
Summary: 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
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)
