WW-5700 fix(ognl): skip the store when a map or list element cannot be converted

[lukaszlenart]

Fixes 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 declared Object, not String
  (TypeConverter.java:49), 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. 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". Please do not
  simplify this back to equals().

  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 — application code calling the converter programmatically
  with a String literal shares the constant's exact instance, since all identical
  literals are interned together. 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. Out of scope here,
  and noted so the limit is not misread as an oversight.

• 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 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

[Copilot]

Pull request overview

This PR fixes a type-conversion edge case in Struts’ OGNL property accessors where XWorkConverter.convertValue() can return TypeConverter.NO_CONVERSION_POSSIBLE, and that sentinel value was previously being stored into Maps/Lists during parameter binding. The change prevents silent poisoning of typed collections (and delayed ClassCastExceptions) by skipping the assignment when conversion fails, with new regression tests covering the reported checkbox scenario and direct accessor behavior.

Changes:

• Add guards in XWorkMapPropertyAccessor to skip map.put(...) when either the converted key or converted value is TypeConverter.NO_CONVERSION_POSSIBLE.
• Add a guard in XWorkListPropertyAccessor to skip list[index]=... (and avoid auto-grow) when the converted element is TypeConverter.NO_CONVERSION_POSSIBLE.
• Add test coverage for unconvertible key/value/element cases, including an end-to-end ParametersInterceptor binding test reproducing WW-5700.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMapPropertyAccessor.java	Prevents storing the “no conversion possible” sentinel into typed Maps by skipping assignment when key/value conversion fails.
core/src/main/java/org/apache/struts2/ognl/accessor/XWorkListPropertyAccessor.java	Prevents storing the sentinel into typed Lists (and avoids list auto-growth) when element conversion fails.
core/src/test/java/org/apache/struts2/ognl/accessor/XWorkMapPropertyAccessorTest.java	Adds regression tests ensuring unconvertible Map keys/values are not stored into typed Maps.
core/src/test/java/org/apache/struts2/ognl/accessor/XWorkListPropertyAccessorTest.java	Adds regression test ensuring an unconvertible List element is not stored into a typed List.
core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java	Adds an end-to-end regression test for WW-5700 reproducing the checkbox “false” → Integer conversion failure scenario.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud