WW-5701 Compare the conversion marker by identity, not equals (6.x backport) - #1879
Open
lukaszlenart wants to merge 2 commits into
Open
WW-5701 Compare the conversion marker by identity, not equals (6.x backport)#1879lukaszlenart wants to merge 2 commits into
lukaszlenart wants to merge 2 commits into
Conversation
…ot equals Backport of the 7.4.0 fix (#1874) to the 6.x line. NO_CONVERSION_POSSIBLE is an ordinary String constant, so comparing with equals() also matched a genuinely converted element whose own text happens to be "ognl.NoConversionPossible" - and silently dropped it from the collection. Only the constant instance itself signals a failed conversion, so compare by identity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e guard paths Backport of the coverage tests added on main, where the Sonar quality gate failed at 77.8% coverage of new code: the marker guard was only exercised on the array-source path, leaving the false branch of the other two guards uncovered. Both added paths are reachable from a request - a Set-typed property fed from a List, and a single-valued parameter assigned to a collection property. The single-value holder is seeded before the assignment so that a setter which is never called cannot make the test pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #1874 to the 6.x line.
NO_CONVERSION_POSSIBLEis an ordinaryStringconstant, soCollectionConverter's!NO_CONVERSION_POSSIBLE.equals(convertedValue)guard also matched a successfully converted elementwhose own text happens to be
"ognl.NoConversionPossible"— and silently dropped it. Only the constantinstance itself signals a failed conversion, so the comparison is now by identity.
Verified affected on 6.x before fixing:
testElementWhoseTextEqualsTheMarkerIsKeptfails onsupport/struts-6-x-xwithexpected:<[alpha, ognl.NoConversionPossible, omega]> but was:<[alpha, omega]>.Note on the fixture, because it is easy to get wrong: the test value is built at runtime with
new String(...toCharArray())and guarded byassertNotSame. A String literal is interned to the verysame instance as the constant, so a literal-based test would pass vacuously even against the unfixed code.
No request-derived parameter is ever that instance — a servlet container builds parameter values from the
request bytes.
The companion test pins that the guard still works: a genuinely unconvertible element is still dropped.
Full
coresuite green: 2719 tests, 0 failures.Fixes WW-5701
🤖 Generated with Claude Code