Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible IndexOutOfBoundsException for issue #618 #619

Merged
merged 10 commits into from
Dec 6, 2023
8 changes: 7 additions & 1 deletion release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 3 additions & 1 deletion release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 12 additions & 2 deletions src/main/java/com/fasterxml/jackson/dataformat/xml/XmlFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Loading