This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch support/struts-6-x-x
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/support/struts-6-x-x by this
push:
new 4f2ab4443 WW-5701 Compare the conversion marker by identity, not
equals (6.x backport) (#1879)
4f2ab4443 is described below
commit 4f2ab44430c2013a13500307ed77af1ef62bdb1b
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon Aug 31 20:03:42 2026 +0200
WW-5701 Compare the conversion marker by identity, not equals (6.x
backport) (#1879)
* 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]>
* WW-5701 test(conversion): cover the collection-source and single-value
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 <[email protected]>
---------
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../conversion/impl/CollectionConverter.java | 6 +-
.../conversion/impl/CollectionConverterTest.java | 128 +++++++++++++++++++++
2 files changed, 131 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..25b9f951d
--- /dev/null
+++
b/core/src/test/java/com/opensymphony/xwork2/conversion/impl/CollectionConverterTest.java
@@ -0,0 +1,128 @@
+/*
+ * 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.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+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());
+ }
+
+ /**
+ * The same guard on the path taken when the submitted value is itself a
collection rather than
+ * an array - here a List feeding a Set-typed property.
+ */
+ public void testUnconvertibleElementIsDroppedFromACollectionSource() {
+ Holder holder = new Holder();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(holder);
+
+ vs.setValue("numberSet", Arrays.asList("1", "not-a-number", "3"));
+
+ assertEquals(new HashSet<>(Arrays.asList(1L, 3L)),
holder.getNumberSet());
+ }
+
+ /**
+ * The same guard on the path taken when a single value is assigned to a
collection property.
+ * The property is seeded first so that a setter which is never called
cannot pass vacuously.
+ */
+ public void testUnconvertibleSingleValueIsDropped() {
+ Holder holder = new Holder();
+ holder.setNumbers(new ArrayList<>(Arrays.asList(99L)));
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(holder);
+
+ vs.setValue("numbers", "not-a-number");
+
+ assertEquals(Collections.emptyList(), holder.getNumbers());
+ }
+
+ public static class Holder {
+ private List<String> names = new ArrayList<>();
+ private List<Long> numbers = new ArrayList<>();
+ private Set<Long> numberSet = new LinkedHashSet<>();
+
+ 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;
+ }
+
+ public Set<Long> getNumberSet() {
+ return numberSet;
+ }
+
+ public void setNumberSet(Set<Long> numberSet) {
+ this.numberSet = numberSet;
+ }
+ }
+}
