[JENKINS-73687] Make deserialization of Map fields in XML files mor…

…e robust (#9653) * Improve resilience against malformed XML files during deserialization * Improve error message
jenkinsci · Sep 13, 2024 · dab6597 · dab6597
1 parent d5271c8
commit dab6597
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 0 deletions.
diff --git 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,12 @@ final class RobustMapConverter extends MapConverter {
     }
 
     private Object read(HierarchicalStreamReader reader, UnmarshallingContext context, Map map) {
+        if (!reader.hasMoreChildren()) {
+            var exception = new ConversionException("Invalid map entry");
+            reader.appendErrors(exception);
+            RobustReflectionConverter.addErrorInContext(context, exception);
+            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
@@ -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,48 @@ 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;
+    }
 }