From a2a80d32dda4fb63442ec967ff4231ddf99f2b70 Mon Sep 17 00:00:00 2001
From: Devin Nusbaum <dnusbaum@cloudbees.com>
Date: Fri, 23 Aug 2024 18:41:43 -0400
Subject: [PATCH 1/2] Improve resilience against malformed XML files during
 deserialization

---
 .../java/hudson/util/RobustMapConverter.java  |  6 +++
 .../hudson/util/RobustMapConverterTest.java   | 50 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/core/src/main/java/hudson/util/RobustMapConverter.java b/core/src/main/java/hudson/util/RobustMapConverter.java
index 896138ace0e9..1b27f4ea364a 100644
--- a/core/src/main/java/hudson/util/RobustMapConverter.java
+++ b/core/src/main/java/hudson/util/RobustMapConverter.java
@@ -25,6 +25,7 @@
 package hudson.util;
 
 import com.thoughtworks.xstream.XStreamException;
+import com.thoughtworks.xstream.converters.ConversionException;
 import com.thoughtworks.xstream.converters.UnmarshallingContext;
 import com.thoughtworks.xstream.converters.collections.MapConverter;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
@@ -64,6 +65,11 @@ final class RobustMapConverter extends MapConverter {
     }
 
     private Object read(HierarchicalStreamReader reader, UnmarshallingContext context, Map map) {
+        if (!reader.hasMoreChildren()) {
+            // TODO: Can we improve the error message?
+            RobustReflectionConverter.addErrorInContext(context, new ConversionException("Invalid format for entry in map"));
+            return ERROR;
+        }
         reader.moveDown();
         try {
             return readBareItem(reader, context, map);
diff --git a/core/src/test/java/hudson/util/RobustMapConverterTest.java b/core/src/test/java/hudson/util/RobustMapConverterTest.java
index d5ac80124ea8..a342e9cfa04b 100644
--- a/core/src/test/java/hudson/util/RobustMapConverterTest.java
+++ b/core/src/test/java/hudson/util/RobustMapConverterTest.java
@@ -25,6 +25,7 @@
 package hudson.util;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
@@ -142,4 +143,53 @@ private Map<Object, Object> preparePayload() {
         }
         return map;
     }
+
+    @Test
+    public void robustAgainstInvalidEntry() {
+        XStream2 xstream2 = new XStream2();
+        String xml =
+            """
+            <hudson.util.RobustMapConverterTest_-Data>
+              <map>
+                <string>key1</string>
+                <entry>
+                  <string>key2</string>
+                  <string>value2</string>
+                </entry>
+              </map>
+            </hudson.util.RobustMapConverterTest_-Data>
+            """;
+        Data data = (Data) xstream2.fromXML(xml);
+        assertThat(data.map, equalTo(Map.of("key2", "value2")));
+    }
+
+    @Test
+    public void robustAgainstInvalidEntryWithNoValue() {
+        XStream2 xstream2 = new XStream2();
+        String xml =
+            """
+            <hudson.util.RobustMapConverterTest_-Data>
+              <map>
+                <entry>
+                  <string>key1</string>
+                </entry>
+                <entry>
+                  <string>key2</string>
+                  <string>value2</string>
+                </entry>
+              </map>
+            </hudson.util.RobustMapConverterTest_-Data>
+            """;
+        Data data = (Data) xstream2.fromXML(xml);
+        assertThat(data.map, equalTo(Map.of("key2", "value2")));
+    }
+
+    private static final class Data {
+        Map<String, String> map;
+
+        @Override
+        public String toString() {
+            return "Data[" + map + "]";
+        }
+    }
 }

From 41433d8c886ab58deaf82058229e55d83774fe4b Mon Sep 17 00:00:00 2001
From: Devin Nusbaum <dnusbaum@cloudbees.com>
Date: Mon, 26 Aug 2024 15:58:18 -0400
Subject: [PATCH 2/2] Improve error message

---
 core/src/main/java/hudson/util/RobustMapConverter.java     | 5 +++--
 core/src/test/java/hudson/util/RobustMapConverterTest.java | 5 -----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/hudson/util/RobustMapConverter.java b/core/src/main/java/hudson/util/RobustMapConverter.java
index 1b27f4ea364a..f845e38771cc 100644
--- a/core/src/main/java/hudson/util/RobustMapConverter.java
+++ b/core/src/main/java/hudson/util/RobustMapConverter.java
@@ -66,8 +66,9 @@ final class RobustMapConverter extends MapConverter {
 
     private Object read(HierarchicalStreamReader reader, UnmarshallingContext context, Map map) {
         if (!reader.hasMoreChildren()) {
-            // TODO: Can we improve the error message?
-            RobustReflectionConverter.addErrorInContext(context, new ConversionException("Invalid format for entry in map"));
+            var exception = new ConversionException("Invalid map entry");
+            reader.appendErrors(exception);
+            RobustReflectionConverter.addErrorInContext(context, exception);
             return ERROR;
         }
         reader.moveDown();
diff --git a/core/src/test/java/hudson/util/RobustMapConverterTest.java b/core/src/test/java/hudson/util/RobustMapConverterTest.java
index a342e9cfa04b..b74ac9303f71 100644
--- a/core/src/test/java/hudson/util/RobustMapConverterTest.java
+++ b/core/src/test/java/hudson/util/RobustMapConverterTest.java
@@ -186,10 +186,5 @@ public void robustAgainstInvalidEntryWithNoValue() {
 
     private static final class Data {
         Map<String, String> map;
-
-        @Override
-        public String toString() {
-            return "Data[" + map + "]";
-        }
     }
 }