diff --git a/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java index d1a8d548c43..5721b725cfd 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/jaxb/Jaxb2Marshaller.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,7 @@ import java.util.Calendar; import java.util.Date; import java.util.Map; import java.util.UUID; + import javax.activation.DataHandler; import javax.activation.DataSource; import javax.xml.XMLConstants; @@ -177,6 +178,8 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi private Schema schema; + private boolean supportDtd = false; + private boolean processExternalEntities = false; @@ -391,6 +394,21 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi this.mappedClass = mappedClass; } + /** + * Indicates whether DTD parsing should be supported. + *

Default is {@code false} meaning that DTD is disabled. + */ + public void setSupportDtd(boolean supportDtd) { + this.supportDtd = supportDtd; + } + + /** + * Whether DTD parsing is supported. + */ + public boolean isSupportDtd() { + return this.supportDtd; + } + /** * Indicates whether external XML entities are processed when unmarshalling. *

Default is {@code false}, meaning that external entities are not resolved. @@ -398,9 +416,14 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi * {@code Source} passed to {@link #unmarshal(Source)} is a {@link SAXSource} or * {@link StreamSource}. It has no effect for {@link DOMSource} or {@link StAXSource} * instances. + *

Note: setting this option to {@code true} also + * automatically sets {@link #setSupportDtd} to {@code true}. */ public void setProcessExternalEntities(boolean processExternalEntities) { this.processExternalEntities = processExternalEntities; + if (processExternalEntities) { + setSupportDtd(true); + } } /** @@ -410,6 +433,7 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi return this.processExternalEntities; } + @Override public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; @@ -754,6 +778,14 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi return unmarshaller.unmarshal(source); } } + catch (NullPointerException ex) { + if (!isSupportDtd()) { + throw new UnmarshallingFailureException("NPE while unmarshalling. " + + "This can happen on JDK 1.6 due to the presence of DTD " + + "declarations, which are disabled.", ex); + } + throw ex; + } catch (JAXBException ex) { throw convertJaxbException(ex); } @@ -809,6 +841,7 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi if (xmlReader == null) { xmlReader = XMLReaderFactory.createXMLReader(); } + xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); String name = "http://xml.org/sax/features/external-general-entities"; xmlReader.setFeature(name, isProcessExternalEntities()); if (!isProcessExternalEntities()) { diff --git a/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java index 1e8402ee0d0..86227a0d768 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/support/AbstractMarshaller.java @@ -72,6 +72,8 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { /** Logger available to subclasses */ protected final Log logger = LogFactory.getLog(getClass()); + private boolean supportDtd = false; + private boolean processExternalEntities = false; private DocumentBuilderFactory documentBuilderFactory; @@ -79,6 +81,21 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { private final Object documentBuilderFactoryMonitor = new Object(); + /** + * Indicates whether DTD parsing should be supported. + *

Default is {@code false} meaning that DTD is disabled. + */ + public void setSupportDtd(boolean supportDtd) { + this.supportDtd = supportDtd; + } + + /** + * Whether DTD parsing is supported. + */ + public boolean isSupportDtd() { + return this.supportDtd; + } + /** * Indicates whether external XML entities are processed when unmarshalling. *

Default is {@code false}, meaning that external entities are not resolved. @@ -86,9 +103,14 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { * {@code Source} passed to {@link #unmarshal(Source)} is a {@link SAXSource} or * {@link StreamSource}. It has no effect for {@link DOMSource} or {@link StAXSource} * instances. + *

Note: setting this option to {@code true} also + * automatically sets {@link #setSupportDtd} to {@code true}. */ public void setProcessExternalEntities(boolean processExternalEntities) { this.processExternalEntities = processExternalEntities; + if (processExternalEntities) { + setSupportDtd(true); + } } /** @@ -133,6 +155,8 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setValidating(false); factory.setNamespaceAware(true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); + factory.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); return factory; } @@ -147,7 +171,11 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { protected DocumentBuilder createDocumentBuilder(DocumentBuilderFactory factory) throws ParserConfigurationException { - return factory.newDocumentBuilder(); + DocumentBuilder documentBuilder = factory.newDocumentBuilder(); + if (!isProcessExternalEntities()) { + documentBuilder.setEntityResolver(NO_OP_ENTITY_RESOLVER); + } + return documentBuilder; } /** @@ -157,6 +185,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { */ protected XMLReader createXmlReader() throws SAXException { XMLReader xmlReader = XMLReaderFactory.createXMLReader(); + xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); if (!isProcessExternalEntities()) { xmlReader.setEntityResolver(NO_OP_ENTITY_RESOLVER); @@ -343,7 +372,17 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { if (domSource.getNode() == null) { domSource.setNode(buildDocument()); } - return unmarshalDomNode(domSource.getNode()); + try { + return unmarshalDomNode(domSource.getNode()); + } + catch (NullPointerException ex) { + if (!isSupportDtd()) { + throw new UnmarshallingFailureException("NPE while unmarshalling. " + + "This can happen on JDK 1.6 due to the presence of DTD " + + "declarations, which are disabled.", ex); + } + throw ex; + } } /** @@ -391,7 +430,17 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { if (saxSource.getInputSource() == null) { saxSource.setInputSource(new InputSource()); } - return unmarshalSaxReader(saxSource.getXMLReader(), saxSource.getInputSource()); + try { + return unmarshalSaxReader(saxSource.getXMLReader(), saxSource.getInputSource()); + } + catch (NullPointerException ex) { + if (!isSupportDtd()) { + throw new UnmarshallingFailureException("NPE while unmarshalling. " + + "This can happen on JDK 1.6 due to the presence of DTD " + + "declarations, which are disabled."); + } + throw ex; + } } /** @@ -404,7 +453,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { */ protected Object unmarshalStreamSource(StreamSource streamSource) throws XmlMappingException, IOException { if (streamSource.getInputStream() != null) { - if (isProcessExternalEntities()) { + if (isProcessExternalEntities() && isSupportDtd()) { return unmarshalInputStream(streamSource.getInputStream()); } else { @@ -414,7 +463,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { } } else if (streamSource.getReader() != null) { - if (isProcessExternalEntities()) { + if (isProcessExternalEntities() && isSupportDtd()) { return unmarshalReader(streamSource.getReader()); } else { diff --git a/spring-oxm/src/test/java/org/springframework/oxm/castor/CastorUnmarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/castor/CastorUnmarshallerTests.java index 374df68faac..afdc18d1424 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/castor/CastorUnmarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/castor/CastorUnmarshallerTests.java @@ -16,10 +16,18 @@ package org.springframework.oxm.castor; +import static junit.framework.Assert.assertEquals; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.*; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.StringReader; import java.util.concurrent.atomic.AtomicReference; + import javax.xml.transform.sax.SAXSource; import javax.xml.transform.stream.StreamSource; @@ -33,9 +41,6 @@ import org.springframework.oxm.AbstractUnmarshallerTests; import org.springframework.oxm.MarshallingException; import org.springframework.oxm.Unmarshaller; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; - /** * @author Arjen Poutsma * @author Jakub Narloch @@ -86,7 +91,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { @Test public void unmarshalTargetClass() throws Exception { CastorMarshaller unmarshaller = new CastorMarshaller(); - unmarshaller.setTargetClasses(new Class[] { Flights.class } ); + unmarshaller.setTargetClasses(new Class[] {Flights.class}); unmarshaller.afterPropertiesSet(); StreamSource source = new StreamSource(new ByteArrayInputStream(INPUT_STRING.getBytes("UTF-8"))); Object flights = unmarshaller.unmarshal(source); @@ -97,7 +102,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { public void testSetBothTargetClassesAndMapping() throws IOException { CastorMarshaller unmarshaller = new CastorMarshaller(); unmarshaller.setMappingLocation(new ClassPathResource("order-mapping.xml", CastorMarshaller.class)); - unmarshaller.setTargetClasses(new Class[] { Order.class } ); + unmarshaller.setTargetClasses(new Class[] {Order.class}); unmarshaller.afterPropertiesSet(); String xml = "" + @@ -183,7 +188,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { @Ignore("Fails on the build server for some reason") public void testClearCollectionsFalse() throws Exception { Flights flights = new Flights(); - flights.setFlight(new Flight[]{new Flight(), null}); + flights.setFlight(new Flight[] {new Flight(), null}); getCastorUnmarshaller().setRootObject(flights); getCastorUnmarshaller().setClearCollections(false); Object result = unmarshalFlights(); @@ -196,7 +201,7 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { } @Test - public void unmarshalStreamSourceExternalEntities() throws Exception { + public void unmarshalStreamSourceWithXmlOptions() throws Exception { final AtomicReference result = new AtomicReference(); CastorMarshaller marshaller = new CastorMarshaller() { @Override @@ -206,21 +211,24 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { } }; - // 1. external-general-entities disabled (default) + // 1. external-general-entities and dtd support disabled (default) marshaller.unmarshal(new StreamSource("1")); assertNotNull(result.get()); + assertEquals(true, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); - // 2. external-general-entities disabled (default) + // 2. external-general-entities and dtd support enabled result.set(null); + marshaller.setSupportDtd(true); marshaller.setProcessExternalEntities(true); marshaller.unmarshal(new StreamSource("1")); assertNotNull(result.get()); + assertEquals(false, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); } @Test - public void unmarshalSaxSourceExternalEntities() throws Exception { + public void unmarshalSaxSourceWithXmlOptions() throws Exception { final AtomicReference result = new AtomicReference(); CastorMarshaller marshaller = new CastorMarshaller() { @Override @@ -230,16 +238,19 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { } }; - // 1. external-general-entities disabled (default) + // 1. external-general-entities and dtd support disabled (default) marshaller.unmarshal(new SAXSource(new InputSource("1"))); assertNotNull(result.get()); + assertEquals(true, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); - // 2. external-general-entities disabled (default) + // 2. external-general-entities and dtd support enabled result.set(null); + marshaller.setSupportDtd(true); marshaller.setProcessExternalEntities(true); marshaller.unmarshal(new SAXSource(new InputSource("1"))); assertNotNull(result.get()); + assertEquals(false, result.get().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); } diff --git a/spring-oxm/src/test/java/org/springframework/oxm/jaxb/Jaxb2MarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/jaxb/Jaxb2MarshallerTests.java index 622c46231a0..676b0203945 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/jaxb/Jaxb2MarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/jaxb/Jaxb2MarshallerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -310,7 +310,7 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests { // SPR-10806 @Test - public void unmarshalStreamSourceExternalEntities() throws Exception { + public void unmarshalStreamSourceWithXmlOptions() throws Exception { final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class); Jaxb2Marshaller marshaller = new Jaxb2Marshaller() { @@ -320,31 +320,34 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests { } }; - // 1. external-general-entities disabled (default) + // 1. external-general-entities and dtd support disabled (default) marshaller.unmarshal(new StreamSource("1")); ArgumentCaptor sourceCaptor = ArgumentCaptor.forClass(SAXSource.class); verify(unmarshaller).unmarshal(sourceCaptor.capture()); SAXSource result = sourceCaptor.getValue(); + assertEquals(true, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); - // 2. external-general-entities enabled + // 2. external-general-entities and dtd support enabled reset(unmarshaller); marshaller.setProcessExternalEntities(true); + marshaller.setSupportDtd(true); marshaller.unmarshal(new StreamSource("1")); verify(unmarshaller).unmarshal(sourceCaptor.capture()); result = sourceCaptor.getValue(); + assertEquals(false, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); } // SPR-10806 @Test - public void unmarshalSaxSourceExternalEntities() throws Exception { + public void unmarshalSaxSourceWithXmlOptions() throws Exception { final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class); Jaxb2Marshaller marshaller = new Jaxb2Marshaller() { @@ -354,24 +357,27 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests { } }; - // 1. external-general-entities disabled (default) + // 1. external-general-entities and dtd support disabled (default) marshaller.unmarshal(new SAXSource(new InputSource("1"))); ArgumentCaptor sourceCaptor = ArgumentCaptor.forClass(SAXSource.class); verify(unmarshaller).unmarshal(sourceCaptor.capture()); SAXSource result = sourceCaptor.getValue(); + assertEquals(true, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); - // 2. external-general-entities enabled + // 2. external-general-entities and dtd support enabled reset(unmarshaller); marshaller.setProcessExternalEntities(true); + marshaller.setSupportDtd(true); marshaller.unmarshal(new SAXSource(new InputSource("1"))); verify(unmarshaller).unmarshal(sourceCaptor.capture()); result = sourceCaptor.getValue(); + assertEquals(false, result.getXMLReader().getFeature("http://apache.org/xml/features/disallow-doctype-decl")); assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); } diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java b/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java index a19cd5fb8a2..b007fa603a8 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java @@ -16,6 +16,7 @@ package org.springframework.http.converter.json; +import java.io.ByteArrayInputStream; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Arrays; @@ -27,6 +28,9 @@ import java.util.Locale; import java.util.Map; import java.util.TimeZone; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLResolver; + import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; @@ -506,22 +510,15 @@ public class Jackson2ObjectMapperBuilder { */ @SuppressWarnings("unchecked") public T build() { - ObjectMapper objectMapper; + ObjectMapper mapper; if (this.createXmlMapper) { - try { - Class xmlMapper = (Class) - ClassUtils.forName("com.fasterxml.jackson.dataformat.xml.XmlMapper", this.moduleClassLoader); - objectMapper = BeanUtils.instantiate(xmlMapper); - } - catch (ClassNotFoundException ex) { - throw new IllegalStateException("Could not instantiate XmlMapper - not found on classpath"); - } + mapper = new XmlObjectMapperInitializer().create(); } else { - objectMapper = new ObjectMapper(); + mapper = new ObjectMapper(); } - configure(objectMapper); - return (T) objectMapper; + configure(mapper); + return (T) mapper; } /** @@ -691,4 +688,23 @@ public class Jackson2ObjectMapperBuilder { return new Jackson2ObjectMapperBuilder().createXmlMapper(true); } + + private static class XmlObjectMapperInitializer { + + public ObjectMapper create() { + XMLInputFactory inputFactory = XMLInputFactory.newInstance(); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + inputFactory.setXMLResolver(NO_OP_XML_RESOLVER); + return new XmlMapper(inputFactory); + } + + private static final XMLResolver NO_OP_XML_RESOLVER = new XMLResolver() { + @Override + public Object resolveEntity(String publicID, String systemID, String base, String ns) { + return new ByteArrayInputStream(new byte[0]); + } + }; + } + } diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java index cb81421b2ba..a4dcd859eb3 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverter.java @@ -229,6 +229,7 @@ public class Jaxb2CollectionHttpMessageConverter */ protected XMLInputFactory createXmlInputFactory() { XMLInputFactory inputFactory = XMLInputFactory.newInstance(); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); inputFactory.setXMLResolver(NO_OP_XML_RESOLVER); return inputFactory; diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java index b6ff303472a..fe077e7b51c 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.http.converter.xml; import java.io.IOException; import java.io.StringReader; + import javax.xml.bind.JAXBElement; import javax.xml.bind.JAXBException; import javax.xml.bind.MarshalException; @@ -60,15 +61,37 @@ import org.springframework.util.ClassUtils; */ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessageConverter { + private boolean supportDtd = false; + private boolean processExternalEntities = false; + /** + * Indicates whether DTD parsing should be supported. + *

Default is {@code false} meaning that DTD is disabled. + */ + public void setSupportDtd(boolean supportDtd) { + this.supportDtd = supportDtd; + } + + /** + * Whether DTD parsing is supported. + */ + public boolean isSupportDtd() { + return this.supportDtd; + } + /** * Indicates whether external XML entities are processed when converting to a Source. *

Default is {@code false}, meaning that external entities are not resolved. + *

Note: setting this option to {@code true} also + * automatically sets {@link #setSupportDtd} to {@code true}. */ public void setProcessExternalEntities(boolean processExternalEntities) { this.processExternalEntities = processExternalEntities; + if (processExternalEntities) { + setSupportDtd(true); + } } /** @@ -109,6 +132,14 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa return jaxbElement.getValue(); } } + catch (NullPointerException ex) { + if (!isSupportDtd()) { + throw new HttpMessageNotReadableException("NPE while unmarshalling. " + + "This can happen on JDK 1.6 due to the presence of DTD " + + "declarations, which are disabled.", ex); + } + throw ex; + } catch (UnmarshalException ex) { throw new HttpMessageNotReadableException("Could not unmarshal to [" + clazz + "]: " + ex.getMessage(), ex); @@ -124,6 +155,7 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa InputSource inputSource = new InputSource(streamSource.getInputStream()); try { XMLReader xmlReader = XMLReaderFactory.createXMLReader(); + xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); String featureName = "http://xml.org/sax/features/external-general-entities"; xmlReader.setFeature(featureName, isProcessExternalEntities()); if (!isProcessExternalEntities()) { diff --git a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java index 8c9c4ec9ef8..8fe5cd0fab0 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import java.io.OutputStream; import java.io.StringReader; import java.util.HashSet; import java.util.Set; + import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -79,6 +80,8 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private final TransformerFactory transformerFactory = TransformerFactory.newInstance(); + private boolean supportDtd = false; + private boolean processExternalEntities = false; @@ -91,12 +94,32 @@ public class SourceHttpMessageConverter extends AbstractHttpMe } + /** + * Indicates whether DTD parsing should be supported. + *

Default is {@code false} meaning that DTD is disabled. + */ + public void setSupportDtd(boolean supportDtd) { + this.supportDtd = supportDtd; + } + + /** + * Whether DTD parsing is supported. + */ + public boolean isSupportDtd() { + return this.supportDtd; + } + /** * Indicates whether external XML entities are processed when converting to a Source. *

Default is {@code false}, meaning that external entities are not resolved. + *

Note: setting this option to {@code true} also + * automatically sets {@link #setSupportDtd} to {@code true}. */ public void setProcessExternalEntities(boolean processExternalEntities) { this.processExternalEntities = processExternalEntities; + if (processExternalEntities) { + setSupportDtd(true); + } } /** @@ -140,6 +163,8 @@ public class SourceHttpMessageConverter extends AbstractHttpMe try { DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); documentBuilderFactory.setNamespaceAware(true); + documentBuilderFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); documentBuilderFactory.setFeature( "http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder(); @@ -149,6 +174,14 @@ public class SourceHttpMessageConverter extends AbstractHttpMe Document document = documentBuilder.parse(body); return new DOMSource(document); } + catch (NullPointerException ex) { + if (!isSupportDtd()) { + throw new HttpMessageNotReadableException("NPE while unmarshalling. " + + "This can happen on JDK 1.6 due to the presence of DTD " + + "declarations, which are disabled.", ex); + } + throw ex; + } catch (ParserConfigurationException ex) { throw new HttpMessageNotReadableException("Could not set feature: " + ex.getMessage(), ex); } @@ -160,11 +193,12 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private SAXSource readSAXSource(InputStream body) throws IOException { try { XMLReader reader = XMLReaderFactory.createXMLReader(); + reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", !isSupportDtd()); reader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); - byte[] bytes = StreamUtils.copyToByteArray(body); if (!isProcessExternalEntities()) { reader.setEntityResolver(NO_OP_ENTITY_RESOLVER); } + byte[] bytes = StreamUtils.copyToByteArray(body); return new SAXSource(reader, new InputSource(new ByteArrayInputStream(bytes))); } catch (SAXException ex) { @@ -174,7 +208,8 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private Source readStAXSource(InputStream body) { try { - XMLInputFactory inputFactory = XMLInputFactory.newFactory(); + XMLInputFactory inputFactory = XMLInputFactory.newInstance(); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, isSupportDtd()); inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, isProcessExternalEntities()); if (!isProcessExternalEntities()) { inputFactory.setXMLResolver(NO_OP_XML_RESOLVER); diff --git a/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverterTests.java index 0dfcc9df4ab..46983dfd7eb 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2CollectionHttpMessageConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,10 +16,13 @@ package org.springframework.http.converter.xml; +import static org.junit.Assert.*; + import java.lang.reflect.Type; import java.util.Collection; import java.util.List; import java.util.Set; + import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; @@ -27,20 +30,22 @@ import javax.xml.bind.annotation.XmlType; import javax.xml.stream.XMLInputFactory; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.http.MockHttpInputMessage; - -import static org.junit.Assert.*; +import org.springframework.http.converter.HttpMessageNotReadableException; /** * Test fixture for {@link Jaxb2CollectionHttpMessageConverter}. * * @author Arjen Poutsma */ +@SuppressWarnings("unused") public class Jaxb2CollectionHttpMessageConverterTests { private Jaxb2CollectionHttpMessageConverter converter; @@ -53,6 +58,9 @@ public class Jaxb2CollectionHttpMessageConverterTests { private Type typeSetType; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Before public void setUp() { @@ -133,6 +141,16 @@ public class Jaxb2CollectionHttpMessageConverterTests { " &ext;"; MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + converter = new Jaxb2CollectionHttpMessageConverter>() { + + @Override + protected XMLInputFactory createXmlInputFactory() { + XMLInputFactory inputFactory = super.createXmlInputFactory(); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, true); + return inputFactory; + } + }; + Collection result = converter.read(rootElementListType, null, inputMessage); assertEquals(1, result.size()); assertEquals("", result.iterator().next().external); @@ -163,6 +181,30 @@ public class Jaxb2CollectionHttpMessageConverterTests { assertEquals("Foo Bar", result.iterator().next().external); } + @Test + public void testXmlBomb() throws Exception { + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String content = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + this.thrown.expect(HttpMessageNotReadableException.class); + this.thrown.expectMessage("\"lol9\""); + this.converter.read(this.rootElementListType, null, inputMessage); + } @XmlRootElement public static class RootElement { diff --git a/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverterTests.java index 5d33f015b8c..dda09647ef0 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverterTests.java @@ -16,7 +16,13 @@ package org.springframework.http.converter.xml; +import static org.custommonkey.xmlunit.XMLAssert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.nio.charset.Charset; + import javax.xml.bind.Marshaller; import javax.xml.bind.Unmarshaller; import javax.xml.bind.annotation.XmlAttribute; @@ -27,7 +33,9 @@ import javax.xml.bind.annotation.adapters.XmlAdapter; import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.aop.framework.AdvisedSupport; import org.springframework.aop.framework.AopProxy; @@ -37,11 +45,7 @@ import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.http.MockHttpInputMessage; import org.springframework.http.MockHttpOutputMessage; - -import static org.custommonkey.xmlunit.XMLAssert.*; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import org.springframework.http.converter.HttpMessageNotReadableException; /** * Tests for {@link Jaxb2RootElementHttpMessageConverter}. @@ -49,6 +53,7 @@ import static org.junit.Assert.assertTrue; * @author Arjen Poutsma * @author Sebastien Deleuze */ +@SuppressWarnings("unused") public class Jaxb2RootElementHttpMessageConverterTests { private Jaxb2RootElementHttpMessageConverter converter; @@ -57,6 +62,10 @@ public class Jaxb2RootElementHttpMessageConverterTests { private RootElement rootElementCglib; + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Before public void setUp() { converter = new Jaxb2RootElementHttpMessageConverter(); @@ -115,6 +124,7 @@ public class Jaxb2RootElementHttpMessageConverterTests { " ]>" + " &ext;"; MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + converter.setSupportDtd(true); RootElement rootElement = (RootElement) converter.read(RootElement.class, inputMessage); assertEquals("", rootElement.external); @@ -134,6 +144,31 @@ public class Jaxb2RootElementHttpMessageConverterTests { assertEquals("Foo Bar", rootElement.external); } + @Test + public void testXmlBomb() throws Exception { + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String content = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + this.thrown.expect(HttpMessageNotReadableException.class); + this.thrown.expectMessage("DOCTYPE is disallowed"); + this.converter.read(RootElement.class, inputMessage); + } + @Test public void writeXmlRootElement() throws Exception { MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); diff --git a/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java index e561f585e90..6ee95828e08 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java @@ -23,8 +23,13 @@ import java.nio.charset.Charset; import com.fasterxml.jackson.annotation.JsonView; import com.fasterxml.jackson.dataformat.xml.XmlMapper; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.xml.sax.SAXException; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; import org.springframework.http.HttpOutputMessage; import org.springframework.http.MediaType; import org.springframework.http.MockHttpInputMessage; @@ -45,6 +50,9 @@ public class MappingJackson2XmlHttpMessageConverterTests { private final MappingJackson2XmlHttpMessageConverter converter = new MappingJackson2XmlHttpMessageConverter(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test public void canRead() { @@ -70,9 +78,9 @@ public class MappingJackson2XmlHttpMessageConverterTests { assertEquals("Foo", result.getString()); assertEquals(42, result.getNumber()); assertEquals(42F, result.getFraction(), 0F); - assertArrayEquals(new String[]{"Foo", "Bar"}, result.getArray()); + assertArrayEquals(new String[] {"Foo", "Bar"}, result.getArray()); assertTrue(result.isBool()); - assertArrayEquals(new byte[]{0x1, 0x2}, result.getBytes()); + assertArrayEquals(new byte[] {0x1, 0x2}, result.getBytes()); } @Test @@ -139,6 +147,55 @@ public class MappingJackson2XmlHttpMessageConverterTests { // Assert no exception is thrown } + @Test + public void readWithExternalReference() throws IOException { + + String body = "\n" + + " ]>&ext;"; + + MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + + this.thrown.expect(HttpMessageNotReadableException.class); + this.thrown.expectMessage("entity \"ext\""); + + this.converter.read(MyBean.class, inputMessage); + } + + @Test + public void readWithXmlBomb() throws IOException { + + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String body = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + + MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + + this.thrown.expect(HttpMessageNotReadableException.class); + this.thrown.expectMessage("entity \"lol9\""); + + this.converter.read(MyBean.class, inputMessage); + } + + private void writeInternal(Object object, HttpOutputMessage outputMessage) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { diff --git a/spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java index 53c05e2666c..e60192e6333 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,10 +16,16 @@ package org.springframework.http.converter.xml; +import static org.custommonkey.xmlunit.XMLAssert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; +import static org.junit.Assert.assertTrue; + import java.io.IOException; import java.io.InputStreamReader; import java.io.StringReader; import java.nio.charset.Charset; + import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.stream.XMLStreamReader; import javax.xml.transform.Source; @@ -29,7 +35,9 @@ import javax.xml.transform.stax.StAXSource; import javax.xml.transform.stream.StreamSource; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; @@ -42,13 +50,10 @@ import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.http.MockHttpInputMessage; import org.springframework.http.MockHttpOutputMessage; +import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.util.FileCopyUtils; -import static org.custommonkey.xmlunit.XMLAssert.*; // Do NOT statically import org.junit.Assert.*, since XMLAssert extends junit.framework.Assert -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; /** * @author Arjen Poutsma @@ -61,6 +66,10 @@ public class SourceHttpMessageConverterTests { private String bodyExternal; + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Before public void setUp() throws IOException { converter = new SourceHttpMessageConverter(); @@ -97,12 +106,40 @@ public class SourceHttpMessageConverterTests { public void readDOMSourceExternal() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + converter.setSupportDtd(true); DOMSource result = (DOMSource) converter.read(DOMSource.class, inputMessage); Document document = (Document) result.getNode(); assertEquals("Invalid result", "root", document.getDocumentElement().getLocalName()); assertNotEquals("Invalid result", "Foo Bar", document.getDocumentElement().getTextContent()); } + @Test + public void readDomSourceWithXmlBomb() throws Exception { + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String content = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + + this.thrown.expect(HttpMessageNotReadableException.class); + this.thrown.expectMessage("DOCTYPE is disallowed"); + + this.converter.read(DOMSource.class, inputMessage); + } + @Test public void readSAXSource() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); @@ -117,6 +154,7 @@ public class SourceHttpMessageConverterTests { public void readSAXSourceExternal() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + converter.setSupportDtd(true); SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage); InputSource inputSource = result.getInputSource(); XMLReader reader = result.getXMLReader(); @@ -130,6 +168,37 @@ public class SourceHttpMessageConverterTests { reader.parse(inputSource); } + @Test + public void readSAXSourceWithXmlBomb() throws Exception { + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String content = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + SAXSource result = (SAXSource) this.converter.read(SAXSource.class, inputMessage); + + this.thrown.expect(SAXException.class); + this.thrown.expectMessage("DOCTYPE is disallowed"); + + InputSource inputSource = result.getInputSource(); + XMLReader reader = result.getXMLReader(); + reader.parse(inputSource); + } + @Test public void readStAXSource() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8")); @@ -149,6 +218,7 @@ public class SourceHttpMessageConverterTests { public void readStAXSourceExternal() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8")); inputMessage.getHeaders().setContentType(new MediaType("application", "xml")); + converter.setSupportDtd(true); StAXSource result = (StAXSource) converter.read(StAXSource.class, inputMessage); XMLStreamReader streamReader = result.getXMLStreamReader(); assertTrue(streamReader.hasNext()); @@ -161,6 +231,39 @@ public class SourceHttpMessageConverterTests { streamReader.close(); } + @Test + public void readStAXSourceWithXmlBomb() throws Exception { + // https://en.wikipedia.org/wiki/Billion_laughs + // https://msdn.microsoft.com/en-us/magazine/ee335713.aspx + String content = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "]>\n" + + "&lol9;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + StAXSource result = (StAXSource) this.converter.read(StAXSource.class, inputMessage); + + XMLStreamReader streamReader = result.getXMLStreamReader(); + assertTrue(streamReader.hasNext()); + streamReader.next(); + streamReader.next(); + String s = streamReader.getLocalName(); + assertEquals("root", s); + + this.thrown.expectMessage("\"lol9\""); + s = streamReader.getElementText(); + } + @Test public void readStreamSource() throws Exception { MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));