From 0b023df0bd6ea6e9d6c8b8534e69f0e0e8b608ff Mon Sep 17 00:00:00 2001 From: Sahana Bogar Date: Wed, 15 Jul 2026 18:35:16 +0530 Subject: [PATCH 1/4] re-apply entity/DTD hardening after JDK deserialization of XmlFactory --- .../jackson/dataformat/xml/XmlFactory.java | 7 +++- .../dataformat/xml/XmlFactoryBuilder.java | 11 +++++ .../xml/misc/DTDAfterSerializationTest.java | 40 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java index ccbb503cf..64814a086 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java @@ -240,7 +240,12 @@ protected Object readResolve() { final XMLInputFactory inf; final XMLOutputFactory outf; try { - inf = (XMLInputFactory) Class.forName(_jdkXmlInFactory).getDeclaredConstructor().newInstance(); + // Only the factory class name survives serialization, so we get a bare + // default instance back: re-apply the entity/DTD hardening the builder + // would have set, otherwise a securely-built factory comes back with + // external entity + DTD processing re-enabled (see [dataformat-xml#190]/#211). + inf = XmlFactoryBuilder.secureXmlInputFactory( + (XMLInputFactory) Class.forName(_jdkXmlInFactory).getDeclaredConstructor().newInstance()); outf = (XMLOutputFactory) Class.forName(_jdkXmlOutFactory).getDeclaredConstructor().newInstance(); } catch (Exception e) { throw new IllegalArgumentException(e); diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java b/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java index 1297f5fc2..f5c7cfcd6 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java @@ -119,6 +119,17 @@ protected static XMLInputFactory defaultXmlInputFactory(ClassLoader cl) { // 24-Oct-2022, tatu: as per [dataformat-xml#550] need extra care xmlIn = XMLInputFactory.newFactory(); } + return secureXmlInputFactory(xmlIn); + } + + /** + * Applies the default entity/DTD hardening to a freshly created + * {@link XMLInputFactory}. Shared so that any code path building a factory + * internally (including JDK-deserialization reconstruction in + * {@code XmlFactory.readResolve()}) applies the same protections and the two + * can not drift apart. + */ + protected static XMLInputFactory secureXmlInputFactory(XMLInputFactory xmlIn) { // as per [dataformat-xml#190], disable external entity expansion by default xmlIn.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); // and ditto wrt [dataformat-xml#211], SUPPORT_DTD diff --git a/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java b/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java new file mode 100644 index 000000000..909efb359 --- /dev/null +++ b/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java @@ -0,0 +1,40 @@ +package tools.jackson.dataformat.xml.misc; + +import java.io.*; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import tools.jackson.dataformat.xml.XmlMapper; +import tools.jackson.dataformat.xml.XmlTestUtil; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +// [dataformat-xml]: entity/DTD hardening must survive JDK serialization of the mapper +public class DTDAfterSerializationTest extends XmlTestUtil +{ + // Internal entity that only expands when DTD processing is enabled + private static final String ENTITY_XML = + " ]>\n" + + "&x;"; + + private XmlMapper jdkRoundtrip(XmlMapper mapper) throws Exception { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream os = new ObjectOutputStream(bytes)) { + os.writeObject(mapper); + } + try (ObjectInputStream is = new ObjectInputStream( + new ByteArrayInputStream(bytes.toByteArray()))) { + return (XmlMapper) is.readObject(); + } + } + + @Test + public void testDTDStaysDisabledAfterRoundtrip() throws Exception + { + XmlMapper mapper = jdkRoundtrip(new XmlMapper()); + // Before the fix the reconstructed factory had DTD/entity processing + // re-enabled, so this would expand `&x;` instead of failing. + assertThrows(Exception.class, () -> mapper.readValue(ENTITY_XML, Map.class)); + } +} From 1ad6e02414de1f303ca440058bc1484e507034d8 Mon Sep 17 00:00:00 2001 From: Sahana Bogar Date: Fri, 17 Jul 2026 15:40:10 +0530 Subject: [PATCH 2/4] add @since 3.3 to secureXmlInputFactory javadoc --- .../java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java b/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java index f5c7cfcd6..60851efa5 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java @@ -128,6 +128,8 @@ protected static XMLInputFactory defaultXmlInputFactory(ClassLoader cl) { * internally (including JDK-deserialization reconstruction in * {@code XmlFactory.readResolve()}) applies the same protections and the two * can not drift apart. + * + * @since 3.3 */ protected static XMLInputFactory secureXmlInputFactory(XMLInputFactory xmlIn) { // as per [dataformat-xml#190], disable external entity expansion by default From b8cf75b704c48133941b6960e34899c759229b9f Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 22 Jul 2026 17:40:01 -0700 Subject: [PATCH 3/4] Add release notes --- release-notes/CREDITS | 5 ++++- release-notes/VERSION | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index fcc836c52..3e693059a 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -154,5 +154,8 @@ Christian Beikov (@beikov) (3.3.0) @Sahana2524 - * Fixed #879: Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet` + * Fixed #878: Re-apply entity/DTD hardening after JDK deserialization of + `XmlFactory` + (3.3.0) + * Fixed #879: Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet` (3.3.0) diff --git a/release-notes/VERSION b/release-notes/VERSION index fc1abdf0d..ae83eb42e 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -9,6 +9,8 @@ Version: 3.x (for earlier see VERSION-2.x) #873: Fix handling of `@JsonApplyView` (fix by @cowtowncoder, w/ Claude code) +#878: Re-apply entity/DTD hardening after JDK deserialization of `XmlFactory` + (fix by @Sahana2524) #879: Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet` (fix by @Sahana2524) From 883a01508df69f22f9f4d5208d6b77f84830303c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 22 Jul 2026 17:48:03 -0700 Subject: [PATCH 4/4] Minor test tweak --- .../java/tools/jackson/dataformat/xml/XmlFactory.java | 2 +- .../dataformat/xml/misc/DTDAfterSerializationTest.java | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java index 64814a086..9754ea5bb 100644 --- a/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java +++ b/src/main/java/tools/jackson/dataformat/xml/XmlFactory.java @@ -243,7 +243,7 @@ protected Object readResolve() { // Only the factory class name survives serialization, so we get a bare // default instance back: re-apply the entity/DTD hardening the builder // would have set, otherwise a securely-built factory comes back with - // external entity + DTD processing re-enabled (see [dataformat-xml#190]/#211). + // external entity + DTD processing re-enabled (see [dataformat-xml#190], [dataformat-xml#211]). inf = XmlFactoryBuilder.secureXmlInputFactory( (XMLInputFactory) Class.forName(_jdkXmlInFactory).getDeclaredConstructor().newInstance()); outf = (XMLOutputFactory) Class.forName(_jdkXmlOutFactory).getDeclaredConstructor().newInstance(); diff --git a/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java b/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java index 909efb359..e35d15935 100644 --- a/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java +++ b/src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; +import tools.jackson.core.exc.StreamReadException; import tools.jackson.dataformat.xml.XmlMapper; import tools.jackson.dataformat.xml.XmlTestUtil; @@ -33,8 +34,11 @@ private XmlMapper jdkRoundtrip(XmlMapper mapper) throws Exception { public void testDTDStaysDisabledAfterRoundtrip() throws Exception { XmlMapper mapper = jdkRoundtrip(new XmlMapper()); - // Before the fix the reconstructed factory had DTD/entity processing - // re-enabled, so this would expand `&x;` instead of failing. - assertThrows(Exception.class, () -> mapper.readValue(ENTITY_XML, Map.class)); + // Must fail specifically because the parser refuses the DTD-declared + // entity (DTD support off), leaving `&x;` unexpanded -- not for some + // unrelated binding reason. Before the fix this expanded to "HELLO". + StreamReadException e = assertThrows(StreamReadException.class, + () -> mapper.readValue(ENTITY_XML, Map.class)); + verifyException(e, "Undeclared general entity", "entity"); } }