[
https://issues.apache.org/jira/browse/WW-5700?focusedWorklogId=1038335&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1038335
]
ASF GitHub Bot logged work on WW-5700:
--------------------------------------
Author: ASF GitHub Bot
Created on: 27/Aug/26 16:21
Start Date: 27/Aug/26 16:21
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1878:
URL: https://github.com/apache/struts/pull/1878
Backport of [#1873](https://github.com/apache/struts/pull/1873) to the 6.x
line.
When `XWorkConverter` cannot convert a value it returns the marker string
`NO_CONVERSION_POSSIBLE`
(`"ognl.NoConversionPossible"`). `XWorkMapPropertyAccessor` and
`XWorkListPropertyAccessor` stored that
marker straight into the target collection, so a `Map<Long, Integer>` could
be left holding the marker
`String` — under a `String` key, when the *key* was the unconvertible half —
and the next read of that
collection failed with a `ClassCastException` far from the cause. The
accessors now skip the assignment
and log at debug instead.
Verified affected on 6.x before fixing: all four tests below fail on
`support/struts-6-x-x` with
`ognl.NoConversionPossible` found in the typed collection, and pass with the
fix.
- `XWorkMapPropertyAccessorTest` — value half and key half
- `XWorkListPropertyAccessorTest` — indexed element
- `ParametersInterceptorTest` — end to end through parameter binding, using
the real-world trigger
(an unchecked `s:checkbox` with `submitUnchecked="true"` submits `"false"`
into a `Map<Long, Integer>`)
`XWorkCollectionPropertyAccessor` is deliberately left alone, as on `main`:
its scalar `setProperty` is
not reachable through the value stack, so no failing test can be written for
it.
Full `core` suite green: 2721 tests, 0 failures.
Fixes [WW-5700](https://issues.apache.org/jira/browse/WW-5700)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1038335)
Time Spent: 0.5h (was: 20m)
> 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
> Fix For: 6.12.0, 7.4.0
>
> Time Spent: 0.5h
> 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 as "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.java line 49 declares the marker as an Object field whose value
> is the ordinary text "ognl.NoConversionPossible". XWorkConverter.convertValue
> returns that marker on failure (lines 339, 351 and 361), _after_ correctly
> registering the conversion error via handleConversionException.
> Three property accessors then store that return value with no guard:
> * XWorkMapPropertyAccessor.setProperty, around line 127 - both the key and
> the value are unguarded
> * XWorkListPropertyAccessor.getRealValue, line 188
> * XWorkCollectionPropertyAccessor.getRealValue, line 263
> By contrast OGNL itself guards this correctly in OgnlRuntime at 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
> annotation enforcement enabled. 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 the accepted-parameter-name patterns in
> DefaultAcceptedPatternsChecker are asymmetric - the bare-bracket branch
> accepts digits only, but the quoted-key branch accepts word characters:
> {noformat}
> (\[\d+]) bare brackets: digits only
> (\['(\w-?|[\u4e00-\u9fa5]-?)+']) quoted key: word characters
> {noformat}
> 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. Fix
> Guard for the marker and skip the store; the error has already been
> registered by convertValue, so nothing is lost. In the map accessor the key
> is guarded before the value is even converted, because a bad key poisons
> iteration over the whole map rather than one entry. In the list accessor the
> guard sits before the auto-grow block, so an unconvertible value does not
> grow the list.
> The comparison is by reference rather than equals(). That is correct rather
> than incidental: the marker field is declared Object, not String, so it is
> not a JLS constant variable and is not inlined into referencing class files -
> every reference resolves to the one field value at runtime, third-party
> converters included. A parameter value built by a servlet container from
> request bytes is a distinct object, so reference comparison separates "the
> converter signalled failure" from "the user submitted this text".
> To be precise about the limit: this protects values arriving from a request,
> which is the case that matters here. It does not protect a value that happens
> to be interned, since all identical String literals share one instance -
> application code calling the converter programmatically with such a literal
> would still lose it. Closing that as well would mean giving the marker an
> identity no user string can share, which changes a published constant and is
> a binary-compatibility question rather than a bug fix.
> h2. Not included
> XWorkCollectionPropertyAccessor carries the same unguarded pattern but is
> left untouched: its scalar setProperty path is not reachable through the
> value stack. Setting {{ids[0]}} on a Set is rejected by OGNL before it gets
> there, so no failing test could be written for it and it was not changed
> blind. Verified independently during review.
> h2. Precedent
> WW-3762 fixed this same bug class in
> XWorkBasicConverter.doConvertToCollection back in 2.3.3, and
> CollectionConverter still carries that guard today. The property accessors
> were simply never given the equivalent check.
> WW-5701 is the mirror-image defect found while reviewing this fix:
> CollectionConverter's guard uses equals() and therefore drops a legitimate
> element whose text genuinely is the marker.
> h2. Backward compatibility
> Narrow. Previously an unconvertible entry was stored as the marker; 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.
> h2. Status
> Fixed in PR https://github.com/apache/struts/pull/1873 - three regression
> tests, written test-first and mutation-checked, plus an end-to-end test of
> the reported checkbox scenario. Full core suite green.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
