Fix possible IndexOutOfBoundsException for issue #618 (#619)

Author: arthurscchan

release-notes/CREDITS-2.x

@@ -238,8 +238,14 @@ Marco Belladelli (mbladel@github)
   not working as expected
  (2.15.0)
 
-Motonori IWAMURO (vmi@github)
+Motonori IWAMURO (@vmi)
 
 * Contributed fix for #616: Fix mismatch in `setNextIsUnwrapped(boolean)` in
   `XmlBeanSerializerBase#serializeFieldsFiltered()`
  (2.16.1)
+
+Arthur Chan (@arthurscchan)
+
+* Reported, contributed fix for #618: `ArrayIndexOutOfBoundsException` thrown for invalid
+  ending XML string when using JDK default Stax XML parser
+ (2.17.0)

release-notes/VERSION-2.x

@@ -6,7 +6,9 @@ Project: jackson-dataformat-xml
 
 2.17.0 (not yet released)
 
--
+#618: `ArrayIndexOutOfBoundsException` thrown for invalid ending XML string
+  when using JDK default Stax XML parser
+ (reported by Arthur C)
 
 2.16.1 (not yet released)
 

src/main/java/com/fasterxml/jackson/dataformat/xml/XmlFactory.java

@@ -13,7 +13,7 @@
 import com.fasterxml.jackson.core.format.MatchStrength;
 import com.fasterxml.jackson.core.io.IOContext;
 import com.fasterxml.jackson.core.util.VersionUtil;
-
+import com.fasterxml.jackson.databind.util.ClassUtil;
 import com.fasterxml.jackson.dataformat.xml.deser.FromXmlParser;
 import com.fasterxml.jackson.dataformat.xml.ser.ToXmlGenerator;
 import com.fasterxml.jackson.dataformat.xml.util.StaxUtil;
@@ -661,7 +661,17 @@ protected FromXmlParser _createParser(byte[] data, int offset, int len, IOContex
             if (_xmlInputFactory instanceof XMLInputFactory2) {
                 sr = _xmlInputFactory.createXMLStreamReader(new Stax2ByteArraySource(data, offset, len));
             } else {
-                sr = _xmlInputFactory.createXMLStreamReader(new ByteArrayInputStream(data, offset, len));
+                // 04-Dec-2023, tatu: As per [dataformat-xml#618], JDK's crappy in-built
+                //    Stax implementation barfs here. Hence:
+                try {
+                    sr = _xmlInputFactory.createXMLStreamReader(new ByteArrayInputStream(data, offset, len));
+                } catch (ArrayIndexOutOfBoundsException e) {
+                    throw new JsonParseException(null,
+                            "Internal processing error by `XMLInputFactory` of type "
+                            +ClassUtil.classNameOf(_xmlInputFactory)+" when trying to create a parser ("
+                            +"consider using Woodstox instead): "
+                            +e.getMessage());
+                }
             }
         } catch (XMLStreamException e) {
             return StaxUtil.throwAsParseException(e, null);

src/main/java/com/fasterxml/jackson/dataformat/xml/deser/XmlTokenStream.java

@@ -5,14 +5,15 @@
 import javax.xml.XMLConstants;
 import javax.xml.stream.*;
 
-import com.fasterxml.jackson.dataformat.xml.XmlNameProcessor;
 import org.codehaus.stax2.XMLStreamLocation2;
 import org.codehaus.stax2.XMLStreamReader2;
-import org.codehaus.stax2.ri.Stax2ReaderAdapter;
 
 import com.fasterxml.jackson.core.JsonLocation;
 import com.fasterxml.jackson.core.io.ContentReference;
 
+import com.fasterxml.jackson.dataformat.xml.XmlNameProcessor;
+import com.fasterxml.jackson.dataformat.xml.util.Stax2JacksonReaderAdapter;
+
 /**
  * Simple helper class used on top of STAX {@link XMLStreamReader} to further
  * abstract out all irrelevant details, and to expose equivalent of flat token
@@ -168,7 +169,8 @@ public XmlTokenStream(XMLStreamReader xmlReader, ContentReference sourceRef,
         _sourceReference = sourceRef;
         _formatFeatures = formatFeatures;
         _cfgProcessXsiNil = FromXmlParser.Feature.PROCESS_XSI_NIL.enabledIn(_formatFeatures);
-        _xmlReader = Stax2ReaderAdapter.wrapIfNecessary(xmlReader);
+        // 04-Dec-2023, tatu: [dataformat-xml#618] Need further customized adapter:
+        _xmlReader = Stax2JacksonReaderAdapter.wrapIfNecessary(xmlReader);
         _nameProcessor = nameProcessor;
     }
 
@@ -557,14 +559,15 @@ private final String _collectUntilTag() throws XMLStreamException
         }
 
         CharSequence chars = null;
-        while (true) {
+        main_loop:
+        while (_xmlReader.hasNext()) {
             switch (_xmlReader.next()) {
             case XMLStreamConstants.START_ELEMENT:
-                return (chars == null) ? "" : chars.toString();
+                break main_loop;
 
             case XMLStreamConstants.END_ELEMENT:
             case XMLStreamConstants.END_DOCUMENT:
-                return (chars == null) ? "" : chars.toString();
+                break main_loop;
 
             // note: SPACE is ignorable (and seldom seen), not to be included
             case XMLStreamConstants.CHARACTERS:
@@ -586,6 +589,7 @@ private final String _collectUntilTag() throws XMLStreamException
                 // any other type (proc instr, comment etc) is just ignored
             }
         }
+        return (chars == null) ? "" : chars.toString();
     }
 
     // Called to skip tokens until start/end tag (or end-of-document) found, but

src/main/java/com/fasterxml/jackson/dataformat/xml/util/Stax2JacksonReaderAdapter.java

@@ -0,0 +1,51 @@
+package com.fasterxml.jackson.dataformat.xml.util;
+
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+
+import org.codehaus.stax2.XMLStreamReader2;
+import org.codehaus.stax2.ri.Stax2ReaderAdapter;
+
+import com.fasterxml.jackson.databind.util.ClassUtil;
+
+/**
+ * Refinement of {@link Stax2ReaderAdapter} to override certain methods,
+ * to patch over flaws of JDK-provided default Stax implementation, SJSXP
+ *
+ * @since 2.17
+ */
+public class Stax2JacksonReaderAdapter
+    extends Stax2ReaderAdapter
+{
+    private final XMLStreamReader _delegate;
+
+    public Stax2JacksonReaderAdapter(XMLStreamReader sr) {
+        super(sr);
+        _delegate = sr;
+    }
+
+    public static XMLStreamReader2 wrapIfNecessary(XMLStreamReader sr)
+    {
+        if (sr instanceof XMLStreamReader2) {
+            return (XMLStreamReader2) sr;
+        }
+        return new Stax2JacksonReaderAdapter(sr);
+    }
+
+    // 04-Dec-2023, tatu: Needed to catch exceptions from buggy SJSXP decoder...
+    @Override
+    public int next() throws XMLStreamException
+    {
+        try {
+            return super.next();
+        } catch (ArrayIndexOutOfBoundsException e) {
+            // Use IllegalStateException since that is guaranteed to be translated
+            // appropriately into Jackson type by caller:
+            throw new IllegalStateException(
+                    "Internal processing error by `XMLStreamReader` of type "
+                    +ClassUtil.classNameOf(_delegate)+" when calling `next()` ("
+                    +"consider using Woodstox instead): "
+                    +e.getMessage(), e);
+        }
+    }
+}

src/test/java/com/fasterxml/jackson/dataformat/xml/deser/TestDeserialization.java

@@ -0,0 +1,39 @@
+package com.fasterxml.jackson.dataformat.xml.fuzz;
+
+import com.fasterxml.jackson.core.exc.StreamReadException;
+
+import com.fasterxml.jackson.dataformat.xml.XmlMapper;
+import com.fasterxml.jackson.dataformat.xml.XmlTestBase;
+
+public class Fuzz618_64655_InvalidXMLTest extends XmlTestBase
+{
+    private final XmlMapper MAPPER = newMapper();
+
+    public void testWithInvalidXml1() throws Exception {
+        _testWithInvalidXml(1, "Unexpected end of input", // Woodstox
+                "Internal processing error by `XMLStreamReader` of type" // SJSXP
+        );
+    }
+
+    public void testWithInvalidXml2() throws Exception {
+        _testWithInvalidXml(2, "Unexpected character 'a'", // Woodstox
+                "Internal processing error by `XMLInputFactory` of type " // SJSXP
+        );
+    }
+
+    public void testWithInvalidXml3() throws Exception {
+        _testWithInvalidXml(3, "Unexpected EOF; was expecting a close tag", // Woodstox
+                "XML document structures must start and end" // SJSXP
+        );
+    }
+
+    private void _testWithInvalidXml(int ix, String... errorToMatch) throws Exception
+    {
+        byte[] doc = readResource("/data/fuzz-618-"+ix+".xml");
+        try {
+            MAPPER.readTree(doc);
+        } catch (StreamReadException e) {
+            verifyException(e, errorToMatch);
+        }
+    }
+}