[
https://issues.apache.org/jira/browse/WW-5700?focusedWorklogId=1038340&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1038340
]
ASF GitHub Bot logged work on WW-5700:
--------------------------------------
Author: ASF GitHub Bot
Created on: 27/Aug/26 16:54
Start Date: 27/Aug/26 16:54
Worklog Time Spent: 10m
Work Description: sonarqubecloud[bot] commented on PR #1878:
URL: https://github.com/apache/struts/pull/1878#issuecomment-5442412007
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1878)
**Quality Gate passed**
Issues
 [0 New
issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1878&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted
issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=1878&issueStatuses=ACCEPTED)
Measures
 [0 Security
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1878&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New
Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1878&metric=new_coverage&view=list)
 [0.0% Duplication on New
Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1878&metric=new_duplicated_lines_density&view=list)
<!
Issue Time Tracking
-------------------
Worklog Id: (was: 1038340)
Time Spent: 40m (was: 0.5h)
> 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: 40m
> 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)
