This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5701-collection-converter-identity-6x in repository https://gitbox.apache.org/repos/asf/struts.git
commit 727c23cded486f3ad74e9d831df777e1de3b215d Author: Lukasz Lenart <[email protected]> AuthorDate: Thu Aug 27 18:20:40 2026 +0200 WW-5701 fix(conversion): compare the conversion marker by identity, not 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 <[email protected]> --- .../conversion/impl/CollectionConverter.java | 6 +- .../conversion/impl/CollectionConverterTest.java | 86 ++++++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/conversion/impl/CollectionConverter.java b/core/src/main/java/com/opensymphony/xwork2/conversion/impl/CollectionConverter.java index b7f707f40..32a32accb 100644 --- a/core/src/main/java/com/opensymphony/xwork2/conversion/impl/CollectionConverter.java +++ b/core/src/main/java/com/opensymphony/xwork2/conversion/impl/CollectionConverter.java @@ -61,7 +61,7 @@ public class CollectionConverter extends DefaultTypeConverter { for (Object anObjArray : objArray) { Object convertedValue = converter.convertValue(context, target, member, propertyName, anObjArray, memberType); - if (!TypeConverter.NO_CONVERSION_POSSIBLE.equals(convertedValue)) { + if (convertedValue != TypeConverter.NO_CONVERSION_POSSIBLE) { result.add(convertedValue); } } @@ -72,7 +72,7 @@ public class CollectionConverter extends DefaultTypeConverter { for (Object aCol : col) { Object convertedValue = converter.convertValue(context, target, member, propertyName, aCol, memberType); - if (!TypeConverter.NO_CONVERSION_POSSIBLE.equals(convertedValue)) { + if (convertedValue != TypeConverter.NO_CONVERSION_POSSIBLE) { result.add(convertedValue); } } @@ -80,7 +80,7 @@ public class CollectionConverter extends DefaultTypeConverter { result = createCollection(toType, memberType, -1); TypeConverter converter = getTypeConverter(context); Object convertedValue = converter.convertValue(context, target, member, propertyName, value, memberType); - if (!TypeConverter.NO_CONVERSION_POSSIBLE.equals(convertedValue)) { + if (convertedValue != TypeConverter.NO_CONVERSION_POSSIBLE) { result.add(convertedValue); } } diff --git a/core/src/test/java/com/opensymphony/xwork2/conversion/impl/CollectionConverterTest.java b/core/src/test/java/com/opensymphony/xwork2/conversion/impl/CollectionConverterTest.java new file mode 100644 index 000000000..ac62ff5b2 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/conversion/impl/CollectionConverterTest.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.opensymphony.xwork2.conversion.impl; + +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.XWorkTestCase; +import com.opensymphony.xwork2.conversion.TypeConverter; +import com.opensymphony.xwork2.util.ValueStack; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class CollectionConverterTest extends XWorkTestCase { + + /** + * WW-5701: the marker constant's value is ordinary text, so an element that genuinely holds + * that text converts successfully and must be kept. + * <p> + * The value is built at runtime rather than written as a literal on purpose: a literal would be + * interned to the very same instance as the constant's value, which no request-derived + * parameter ever is. A servlet container builds parameter values from the request bytes. + */ + public void testElementWhoseTextEqualsTheMarkerIsKept() { + String asSubmittedByAUser = new String("ognl.NoConversionPossible".toCharArray()); + assertNotSame("fixture must not be interned", TypeConverter.NO_CONVERSION_POSSIBLE, asSubmittedByAUser); + + Holder holder = new Holder(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(holder); + + vs.setValue("names", new String[]{"alpha", asSubmittedByAUser, "omega"}); + + assertEquals(Arrays.asList("alpha", "ognl.NoConversionPossible", "omega"), holder.getNames()); + } + + /** + * The guard must still do its job: a genuinely unconvertible element is dropped. + */ + public void testUnconvertibleElementIsStillDropped() { + Holder holder = new Holder(); + ValueStack vs = ActionContext.getContext().getValueStack(); + vs.push(holder); + + vs.setValue("numbers", new String[]{"1", "not-a-number", "3"}); + + assertEquals(Arrays.asList(1L, 3L), holder.getNumbers()); + } + + public static class Holder { + private List<String> names = new ArrayList<>(); + private List<Long> numbers = new ArrayList<>(); + + public List<String> getNames() { + return names; + } + + public void setNames(List<String> names) { + this.names = names; + } + + public List<Long> getNumbers() { + return numbers; + } + + public void setNumbers(List<Long> numbers) { + this.numbers = numbers; + } + } +}
