diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 423068581..a898cf219 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -166,3 +166,5 @@ Christian Beikov (@beikov) (3.3.0) * Fixed #882: Translate base64 name-decode errors to `StreamReadException` (3.3.0) + * Fixed #883: Translate `XMLStreamReader` creation errors to `StreamReadException` + (3.3.0) diff --git a/release-notes/VERSION b/release-notes/VERSION index bc7b20901..082c59c27 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -15,6 +15,9 @@ Version: 3.x (for earlier see VERSION-2.x) (fix by @Sahana2524) #882: Translate base64 name-decode errors to `StreamReadException` (fix by @Sahana2524) +#883: Translate `XMLStreamReader` creation errors to `StreamReadException` + (restores guard lost in 3.x port; see 2.x `_createParser(byte[])` for #618) + (fix by @Sahana2524) 3.2.2 (not yet released) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java index 9754ea5bb..7f06fd729 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java @@ -14,6 +14,8 @@ import tools.jackson.core.exc.StreamWriteException; import tools.jackson.core.io.IOContext; +import tools.jackson.databind.util.ClassUtil; + import tools.jackson.dataformat.xml.deser.FromXmlParser; import tools.jackson.dataformat.xml.ser.ToXmlGenerator; import tools.jackson.dataformat.xml.util.StaxUtil; @@ -484,6 +486,8 @@ protected FromXmlParser _createParser(ObjectReadContext readCtxt, IOContext ioCt sr = _xmlInputFactory.createXMLStreamReader(in); } catch (XMLStreamException e) { return StaxUtil.throwAsReadException(e, null); + } catch (IndexOutOfBoundsException e) { + return _reportBadXmlReaderCreation(e); } return _fromXmlParser(readCtxt, ioCtxt, _initializeXmlReader(sr)); } @@ -497,6 +501,8 @@ protected FromXmlParser _createParser(ObjectReadContext readCtxt, IOContext ioCt sr = _xmlInputFactory.createXMLStreamReader(r); } catch (XMLStreamException e) { return StaxUtil.throwAsReadException(e, null); + } catch (IndexOutOfBoundsException e) { + return _reportBadXmlReaderCreation(e); } return _fromXmlParser(readCtxt, ioCtxt, _initializeXmlReader(sr)); } @@ -519,6 +525,8 @@ protected FromXmlParser _createParser(ObjectReadContext readCtxt, IOContext ioCt } } catch (XMLStreamException e) { return StaxUtil.throwAsReadException(e, null); + } catch (IndexOutOfBoundsException e) { + return _reportBadXmlReaderCreation(e); } return _fromXmlParser(readCtxt, ioCtxt, _initializeXmlReader(sr)); } @@ -538,10 +546,29 @@ protected FromXmlParser _createParser(ObjectReadContext readCtxt, IOContext ioCt } } catch (XMLStreamException e) { return StaxUtil.throwAsReadException(e, null); + } catch (IndexOutOfBoundsException e) { + return _reportBadXmlReaderCreation(e); } return _fromXmlParser(readCtxt, ioCtxt, _initializeXmlReader(sr)); } + // 24-Jul-2026, tatu: [dataformat-xml#883] JDK's built-in Stax implementation (SJSXP) + // has been seen to fail with unchecked exceptions -- instead of the expected + // `XMLStreamException` -- while creating the stream reader. 2.x guards against + // this in `_createParser(byte[])` (added 04-Dec-2023 while working on + // [dataformat-xml#618]) but the check was lost in the 3.x port. Only the `byte[]` + // case has actually been seen in the wild; other overloads guarded for symmetry. + // Translate to standard read-exception type so callers only ever need to catch + // `JacksonException`; same as `Stax2JacksonReaderAdapter.next()` does for the + // `next()`-time SJSXP failures that #618 itself was about. + private T _reportBadXmlReaderCreation(Throwable e) { + throw new StreamReadException(null, + "Internal processing error by `XMLInputFactory` of type " + +ClassUtil.classNameOf(_xmlInputFactory) + +" when trying to create a parser (consider using Woodstox instead): " + +e.getMessage(), e); + } + /** * Overridable method to allow using custom FromXmlParser sub-classes. */ diff --git a/src/test/java/tools/jackson/dataformat/xml/fuzz/Fuzz618_64655_InvalidXMLTest.java b/src/test/java/tools/jackson/dataformat/xml/fuzz/Fuzz618_64655_InvalidXMLTest.java index b251f3b22..c515d618e 100644 --- a/src/test/java/tools/jackson/dataformat/xml/fuzz/Fuzz618_64655_InvalidXMLTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/fuzz/Fuzz618_64655_InvalidXMLTest.java @@ -7,6 +7,8 @@ import tools.jackson.dataformat.xml.XmlMapper; import tools.jackson.dataformat.xml.XmlTestUtil; +import static org.junit.jupiter.api.Assertions.fail; + // [dataformat-xml#618]: Issues found by OSS-Fuzz (64655 etc) public class Fuzz618_64655_InvalidXMLTest extends XmlTestUtil { @@ -38,6 +40,10 @@ private void _testWithInvalidXml(int ix, String... errorToMatch) throws Exceptio byte[] doc = readResource("/data/fuzz-618-"+ix+".xml"); try { MAPPER.readTree(doc); + // 24-Jul-2026, tatu: Must not pass silently: without this the test would + // also pass if no exception at all was thrown (which is how the + // `XMLInputFactory` guard went missing in the 3.x port, see #883) + fail("Should not pass, invalid XML"); } catch (StreamReadException e) { verifyException(e, errorToMatch); } diff --git a/src/test/java/tools/jackson/dataformat/xml/stream/BrokenReaderFactoryTest.java b/src/test/java/tools/jackson/dataformat/xml/stream/BrokenReaderFactoryTest.java new file mode 100644 index 000000000..4d4b56818 --- /dev/null +++ b/src/test/java/tools/jackson/dataformat/xml/stream/BrokenReaderFactoryTest.java @@ -0,0 +1,224 @@ +package tools.jackson.dataformat.xml.stream; + +import java.io.*; + +import javax.xml.stream.*; +import javax.xml.stream.util.XMLEventAllocator; +import javax.xml.transform.Source; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +import com.ctc.wstx.stax.WstxInputFactory; + +import tools.jackson.core.exc.StreamReadException; + +import tools.jackson.dataformat.xml.XmlFactory; +import tools.jackson.dataformat.xml.XmlMapper; +import tools.jackson.dataformat.xml.XmlTestUtil; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +// [dataformat-xml#883]: `XmlFactory` must translate unchecked failures of +// `XMLInputFactory.createXMLStreamReader()` into `StreamReadException`, so that callers +// only ever need to catch `JacksonException`. 2.x guards the `byte[]` + non-Stax2 case +// (added 04-Dec-2023 while working on [dataformat-xml#618]) but the check was lost in +// the 3.x port. +// +// Two stand-in factories are needed because `XmlFactory` branches on +// `instanceof XMLInputFactory2` for `byte[]`/`char[]` input: Stax2 impls get handed +// `Stax2ByteArraySource`/`Stax2CharArraySource`, plain ones a +// `ByteArrayInputStream`/`CharArrayReader` -- and it is the latter branch that +// JDK-bundled SJSXP was seen failing on. +public class BrokenReaderFactoryTest extends XmlTestUtil +{ + private final static String DOC = "value"; + + /** + * Stand-in for a Stax2-capable impl whose reader creation throws + * non-{@code XMLStreamException}. + */ + static class BrokenStax2InputFactory extends WstxInputFactory { + @Override + public XMLStreamReader createXMLStreamReader(InputStream in) { + throw new ArrayIndexOutOfBoundsException("Index 5 out of bounds for length 4"); + } + + @Override + public XMLStreamReader createXMLStreamReader(Reader r) { + throw new ArrayIndexOutOfBoundsException("Index 5 out of bounds for length 4"); + } + + @Override + public XMLStreamReader createXMLStreamReader(Source s) { + throw new ArrayIndexOutOfBoundsException("Index 5 out of bounds for length 4"); + } + } + + /** + * Stand-in for a plain (non-Stax2) impl, like JDK-bundled SJSXP: throws + * {@link StringIndexOutOfBoundsException} so that widening of the guard beyond + * {@code ArrayIndexOutOfBoundsException} gets covered too. + */ + static class BrokenBasicInputFactory extends XMLInputFactory { + private final XMLInputFactory _delegate = XMLInputFactory.newDefaultFactory(); + + // The 3 creation methods `XmlFactory` may call: all fail + + @Override + public XMLStreamReader createXMLStreamReader(Reader r) { + throw new StringIndexOutOfBoundsException("begin 0, end 3, length 2"); + } + + @Override + public XMLStreamReader createXMLStreamReader(InputStream in) { + throw new StringIndexOutOfBoundsException("begin 0, end 3, length 2"); + } + + @Override + public XMLStreamReader createXMLStreamReader(Source s) { + throw new StringIndexOutOfBoundsException("begin 0, end 3, length 2"); + } + + // ... and plain delegation for the rest (`setProperty()` at least does get called) + + @Override + public XMLStreamReader createXMLStreamReader(InputStream in, String enc) throws XMLStreamException { + return _delegate.createXMLStreamReader(in, enc); + } + + @Override + public XMLStreamReader createXMLStreamReader(String sysId, InputStream in) throws XMLStreamException { + return _delegate.createXMLStreamReader(sysId, in); + } + + @Override + public XMLStreamReader createXMLStreamReader(String sysId, Reader r) throws XMLStreamException { + return _delegate.createXMLStreamReader(sysId, r); + } + + @Override + public XMLEventReader createXMLEventReader(Reader r) throws XMLStreamException { + return _delegate.createXMLEventReader(r); + } + + @Override + public XMLEventReader createXMLEventReader(String sysId, Reader r) throws XMLStreamException { + return _delegate.createXMLEventReader(sysId, r); + } + + @Override + public XMLEventReader createXMLEventReader(XMLStreamReader sr) throws XMLStreamException { + return _delegate.createXMLEventReader(sr); + } + + @Override + public XMLEventReader createXMLEventReader(Source s) throws XMLStreamException { + return _delegate.createXMLEventReader(s); + } + + @Override + public XMLEventReader createXMLEventReader(InputStream in) throws XMLStreamException { + return _delegate.createXMLEventReader(in); + } + + @Override + public XMLEventReader createXMLEventReader(InputStream in, String enc) throws XMLStreamException { + return _delegate.createXMLEventReader(in, enc); + } + + @Override + public XMLEventReader createXMLEventReader(String sysId, InputStream in) throws XMLStreamException { + return _delegate.createXMLEventReader(sysId, in); + } + + @Override + public XMLStreamReader createFilteredReader(XMLStreamReader sr, StreamFilter f) throws XMLStreamException { + return _delegate.createFilteredReader(sr, f); + } + + @Override + public XMLEventReader createFilteredReader(XMLEventReader er, EventFilter f) throws XMLStreamException { + return _delegate.createFilteredReader(er, f); + } + + @Override + public XMLResolver getXMLResolver() { return _delegate.getXMLResolver(); } + + @Override + public void setXMLResolver(XMLResolver r) { _delegate.setXMLResolver(r); } + + @Override + public XMLReporter getXMLReporter() { return _delegate.getXMLReporter(); } + + @Override + public void setXMLReporter(XMLReporter r) { _delegate.setXMLReporter(r); } + + @Override + public void setProperty(String name, Object value) { _delegate.setProperty(name, value); } + + @Override + public Object getProperty(String name) { return _delegate.getProperty(name); } + + @Override + public boolean isPropertySupported(String name) { return _delegate.isPropertySupported(name); } + + @Override + public void setEventAllocator(XMLEventAllocator a) { _delegate.setEventAllocator(a); } + + @Override + public XMLEventAllocator getEventAllocator() { return _delegate.getEventAllocator(); } + } + + private final XmlMapper STAX2_MAPPER = new XmlMapper(new XmlFactory(new BrokenStax2InputFactory())); + + private final XmlMapper BASIC_MAPPER = new XmlMapper(new XmlFactory(new BrokenBasicInputFactory())); + + /* + /********************************************************************** + /* Stax2-capable factory: `Stax2ByteArraySource`/`Stax2CharArraySource` branch + /********************************************************************** + */ + + @Test + public void testStax2ByteArrayInput() throws Exception { + _verifyBadReaderCreation(() -> STAX2_MAPPER.readTree(utf8Bytes(DOC))); + } + + @Test + public void testStax2CharArrayInput() throws Exception { + _verifyBadReaderCreation(() -> STAX2_MAPPER.createParser(DOC.toCharArray())); + } + + @Test + public void testStax2InputStreamInput() throws Exception { + _verifyBadReaderCreation(() -> STAX2_MAPPER.readTree(new ByteArrayInputStream(utf8Bytes(DOC)))); + } + + @Test + public void testStax2ReaderInput() throws Exception { + _verifyBadReaderCreation(() -> STAX2_MAPPER.readTree(new StringReader(DOC))); + } + + /* + /********************************************************************** + /* Plain factory: `ByteArrayInputStream`/`CharArrayReader` branch + /********************************************************************** + */ + + @Test + public void testBasicByteArrayInput() throws Exception { + _verifyBadReaderCreation(() -> BASIC_MAPPER.readTree(utf8Bytes(DOC))); + } + + @Test + public void testBasicCharArrayInput() throws Exception { + _verifyBadReaderCreation(() -> BASIC_MAPPER.createParser(DOC.toCharArray())); + } + + private void _verifyBadReaderCreation(Executable exec) { + StreamReadException e = assertThrows(StreamReadException.class, exec); + verifyException(e, "Internal processing error by `XMLInputFactory`"); + verifyException(e, "when trying to create a parser"); + } +}