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)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to