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 c2b97be29 WW-5700 fix(ognl): skip the store when a map or list element
cannot be converted (#1878)
c2b97be29 is described below
commit c2b97be299ce1ed2311587d9e8e24106f5d5d134
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon Aug 31 20:02:43 2026 +0200
WW-5700 fix(ognl): skip the store when a map or list element cannot be
converted (#1878)
Backport of the 7.4.0 fix (#1873) to the 6.x line.
When XWorkConverter cannot convert a value it returns the marker string
NO_CONVERSION_POSSIBLE. The map and list property accessors stored that
marker straight into the target collection, so a Map<Long, Integer> could
end up holding the String "ognl.NoConversionPossible" under a String key,
and the next read of that collection failed with a ClassCastException far
from the cause. Skip the assignment instead and log at debug.
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../ognl/accessor/XWorkListPropertyAccessor.java | 9 +++++
.../ognl/accessor/XWorkMapPropertyAccessor.java | 12 +++++-
.../accessor/XWorkListPropertyAccessorTest.java | 16 ++++++++
.../accessor/XWorkMapPropertyAccessorTest.java | 45 ++++++++++++++++++++++
.../parameter/ParametersInterceptorTest.java | 39 +++++++++++++++++++
5 files changed, 120 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
index 47a640438..4e5048527 100644
---
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
+++
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessor.java
@@ -20,6 +20,7 @@ package com.opensymphony.xwork2.ognl.accessor;
import com.opensymphony.xwork2.ObjectFactory;
import com.opensymphony.xwork2.conversion.ObjectTypeDeterminer;
+import com.opensymphony.xwork2.conversion.TypeConverter;
import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.ognl.OgnlUtil;
@@ -29,6 +30,8 @@ import ognl.OgnlException;
import ognl.PropertyAccessor;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.StrutsException;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import java.util.Collection;
import java.util.List;
@@ -43,6 +46,8 @@ import java.util.Map;
*/
public class XWorkListPropertyAccessor extends ListPropertyAccessor {
+ private static final Logger LOG =
LogManager.getLogger(XWorkListPropertyAccessor.class);
+
private XWorkCollectionPropertyAccessor _sAcc = new
XWorkCollectionPropertyAccessor();
private XWorkConverter xworkConverter;
@@ -167,6 +172,10 @@ public class XWorkListPropertyAccessor extends
ListPropertyAccessor {
}
Object realValue = getRealValue(context, value, convertToClass);
+ if (realValue == TypeConverter.NO_CONVERSION_POSSIBLE) {
+ LOG.debug("Unable to convert value for index [{}] to the declared
element type, skipping assignment", name);
+ return;
+ }
if (target instanceof List && name instanceof Number) {
//make sure there are enough spaces in the List to set
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessor.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessor.java
index bad7932ce..dd465fd48 100644
---
a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessor.java
+++
b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessor.java
@@ -20,6 +20,7 @@ package com.opensymphony.xwork2.ognl.accessor;
import com.opensymphony.xwork2.ObjectFactory;
import com.opensymphony.xwork2.conversion.ObjectTypeDeterminer;
+import com.opensymphony.xwork2.conversion.TypeConverter;
import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
@@ -126,8 +127,17 @@ public class XWorkMapPropertyAccessor extends
MapPropertyAccessor {
LOG.trace("Entering setProperty({},{},{},{})", context, target, name,
value);
Object key = getKey(context, name);
+ if (key == TypeConverter.NO_CONVERSION_POSSIBLE) {
+ LOG.debug("Unable to convert key [{}] to the declared key type,
skipping assignment", name);
+ return;
+ }
+ Object convertedValue = getValue(context, value);
+ if (convertedValue == TypeConverter.NO_CONVERSION_POSSIBLE) {
+ LOG.debug("Unable to convert value for key [{}] to the declared
element type, skipping assignment", key);
+ return;
+ }
Map map = (Map) target;
- map.put(key, getValue(context, value));
+ map.put(key, convertedValue);
}
private Object getValue(Map context, Object value) {
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
index 4a89d8935..9df833198 100644
---
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkListPropertyAccessorTest.java
@@ -64,6 +64,22 @@ public class XWorkListPropertyAccessorTest extends
XWorkTestCase {
assertEquals(myList.size(), vs.findValue("strings.size"));
}
+ public void testUnconvertibleElementIsNotStored() {
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ ListHolder listHolder = new ListHolder();
+ listHolder.setLongs(new ArrayList<>());
+ vs.push(listHolder);
+
+ vs.setValue("longs[0]", "1");
+ vs.setValue("longs[1]", "not-a-number");
+
+ assertEquals(Long.valueOf(1), listHolder.getLongs().get(0));
+ for (Object element : (List) listHolder.getLongs()) {
+ assertTrue("list must not hold a non-Long element: " + element,
+ element == null || element instanceof Long);
+ }
+ }
+
public void testAutoGrowthCollectionLimit() {
PropertyAccessor accessor =
container.getInstance(PropertyAccessor.class, ArrayList.class.getName());
((XWorkListPropertyAccessor) accessor).setAutoGrowCollectionLimit("2");
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessorTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessorTest.java
index 1835e938a..b0629c1ef 100644
---
a/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessorTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/ognl/accessor/XWorkMapPropertyAccessorTest.java
@@ -25,6 +25,7 @@ import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import java.util.Collections;
+import java.util.HashMap;
import java.util.Map;
public class XWorkMapPropertyAccessorTest extends XWorkTestCase {
@@ -57,6 +58,50 @@ public class XWorkMapPropertyAccessorTest extends
XWorkTestCase {
assertNull(vs.findValue("map['key']"));
}
+ public void testUnconvertibleValueIsNotStored() {
+ TypedMapHolder holder = new TypedMapHolder();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(holder);
+
+ vs.setValue("counts[1]", "5");
+ vs.setValue("counts[2]", "not-a-number");
+
+ assertEquals(Integer.valueOf(5), holder.getCounts().get(1L));
+ assertOnlyDeclaredTypes(holder.getCounts());
+ }
+
+ public void testUnconvertibleKeyIsNotStored() {
+ TypedMapHolder holder = new TypedMapHolder();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(holder);
+
+ vs.setValue("counts[1]", "5");
+ vs.setValue("counts['abc']", "6");
+
+ assertEquals(Integer.valueOf(5), holder.getCounts().get(1L));
+ assertOnlyDeclaredTypes(holder.getCounts());
+ }
+
+ /**
+ * A Map declared to hold Long keys and Integer values must never be left
holding anything else.
+ */
+ private static void assertOnlyDeclaredTypes(Map<Long, Integer> map) {
+ for (Object o : ((Map) map).entrySet()) {
+ Map.Entry entry = (Map.Entry) o;
+ assertTrue("key is not a Long: " + entry.getKey(), entry.getKey()
instanceof Long);
+ assertTrue("value is not an Integer: " + entry.getValue(),
entry.getValue() instanceof Integer);
+ }
+ }
+
+ public static class TypedMapHolder {
+ @Element(value = Integer.class)
+ private final Map<Long, Integer> counts = new HashMap<>();
+
+ public Map<Long, Integer> getCounts() {
+ return counts;
+ }
+ }
+
private static class MapHolder {
private final Map map;
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
index cf87495bc..2101b701d 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
@@ -38,6 +38,7 @@ import com.opensymphony.xwork2.ognl.OgnlValueStack;
import com.opensymphony.xwork2.ognl.OgnlValueStackFactory;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
+import com.opensymphony.xwork2.util.Element;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.ValueStackFactory;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
@@ -1012,6 +1013,44 @@ public class ParametersInterceptorTest extends
XWorkTestCase {
container.inject(config.getInterceptors().get(0).getInterceptor());
}
+
+ /**
+ * WW-5700: a value that cannot be converted to the map's element type
must not be stored.
+ * An unchecked s:checkbox with submitUnchecked="true" submits the
CheckboxInterceptor's
+ * uncheckedValue, "false", which cannot become an Integer.
+ */
+ public void testUnconvertibleValueIsNotBoundIntoTypedMap() {
+ CheckboxAction action = new CheckboxAction();
+ ValueStack vs = ActionContext.getContext().getValueStack();
+ vs.push(action);
+
+ ParametersInterceptor pi = new ParametersInterceptor();
+ container.inject(pi);
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("capDeferral[100]", "1");
+ params.put("capDeferral[200]", "false");
+
+ pi.applyParameters(action, vs, HttpParameters.create(params).build());
+
+ Map<Long, Integer> capDeferral = action.getCapDeferral();
+ assertEquals("sanity: the convertible value must still bind",
Integer.valueOf(1), capDeferral.get(100L));
+ for (Object entry : ((Map) capDeferral).entrySet()) {
+ Map.Entry e = (Map.Entry) entry;
+ assertTrue("key is not a Long: " + e.getKey(), e.getKey()
instanceof Long);
+ assertTrue("value is not an Integer: " + e.getValue(),
e.getValue() instanceof Integer);
+ }
+ }
+
+ public static class CheckboxAction {
+ @Element(value = Integer.class)
+ private final Map<Long, Integer> capDeferral = new HashMap<>();
+
+ @StrutsParameter(depth = 1)
+ public Map<Long, Integer> getCapDeferral() {
+ return capDeferral;
+ }
+ }
}
class ValidateAction implements ValidationAware {
