diff --git a/spring-oxm/src/main/java/org/springframework/oxm/castor/CastorMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/castor/CastorMarshaller.java index adc403ccf0..4189c0eeb3 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/castor/CastorMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/castor/CastorMarshaller.java @@ -162,6 +162,11 @@ public class CastorMarshaller extends AbstractMarshaller implements Initializing this.encoding = encoding; } + @Override + protected String getDefaultEncoding() { + return this.encoding; + } + /** * Set the locations of the Castor XML mapping files. */ 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 0837695e1c..93fa1a4d1f 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 @@ -400,6 +400,13 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi this.processExternalEntities = processExternalEntities; } + /** + * @return the configured value for whether XML external entities are allowed. + */ + public boolean isProcessExternalEntities() { + return this.processExternalEntities; + } + @Override public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; diff --git a/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java index b184560457..715ef4e235 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -28,6 +28,7 @@ import javax.xml.stream.XMLEventWriter; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.stream.XMLStreamWriter; +import javax.xml.transform.OutputKeys; import javax.xml.transform.Result; import javax.xml.transform.Source; import javax.xml.transform.Transformer; @@ -149,6 +150,11 @@ public class JibxMarshaller extends AbstractMarshaller implements InitializingBe this.encoding = encoding; } + @Override + protected String getDefaultEncoding() { + return this.encoding; + } + /** * Set the document standalone flag for marshalling. By default, this flag is not present. */ @@ -338,7 +344,7 @@ public class JibxMarshaller extends AbstractMarshaller implements InitializingBe } catch (TransformerException ex) { throw new MarshallingFailureException( - "Could not transform to [" + ClassUtils.getShortName(result.getClass()) + "]"); + "Could not transform to [" + ClassUtils.getShortName(result.getClass()) + "]", ex); } } @@ -398,7 +404,7 @@ public class JibxMarshaller extends AbstractMarshaller implements InitializingBe @Override protected Object unmarshalDomNode(Node node) throws XmlMappingException { try { - return transformAndUnmarshal(new DOMSource(node)); + return transformAndUnmarshal(new DOMSource(node), null); } catch (IOException ex) { throw new UnmarshallingFailureException("JiBX unmarshalling exception", ex); @@ -409,12 +415,15 @@ public class JibxMarshaller extends AbstractMarshaller implements InitializingBe protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource) throws XmlMappingException, IOException { - return transformAndUnmarshal(new SAXSource(xmlReader, inputSource)); + return transformAndUnmarshal(new SAXSource(xmlReader, inputSource), inputSource.getEncoding()); } - private Object transformAndUnmarshal(Source source) throws IOException { + private Object transformAndUnmarshal(Source source, String encoding) throws IOException { try { Transformer transformer = this.transformerFactory.newTransformer(); + if (encoding != null) { + transformer.setOutputProperty(OutputKeys.ENCODING, encoding); + } ByteArrayOutputStream os = new ByteArrayOutputStream(); transformer.transform(source, new StreamResult(os)); ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray()); @@ -422,7 +431,7 @@ public class JibxMarshaller extends AbstractMarshaller implements InitializingBe } catch (TransformerException ex) { throw new MarshallingFailureException( - "Could not transform from [" + ClassUtils.getShortName(source.getClass()) + "]"); + "Could not transform from [" + ClassUtils.getShortName(source.getClass()) + "]", ex); } } 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 a118775137..2df808e574 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -73,6 +73,34 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { private final Object documentBuilderFactoryMonitor = new Object(); + private boolean processExternalEntities = false; + + + /** + * Indicates whether external XML entities are processed when unmarshalling. + *

Default is {@code false}, meaning that external entities are not resolved. + * Note that processing of external entities will only be enabled/disabled when the + * {@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. + */ + public void setProcessExternalEntities(boolean processExternalEntities) { + this.processExternalEntities = processExternalEntities; + } + + /** + * @return the configured value for whether XML external entities are allowed. + */ + public boolean isProcessExternalEntities() { + return this.processExternalEntities; + } + + /** + * @return the default encoding to use for marshalling or unmarshalling from + * a byte stream, or {@code null}. + */ + abstract protected String getDefaultEncoding(); + /** * Marshals the object graph with the given root into the provided {@code javax.xml.transform.Result}. @@ -133,7 +161,7 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { return unmarshalSaxSource((SAXSource) source); } else if (source instanceof StreamSource) { - return unmarshalStreamSource((StreamSource) source); + return unmarshalStreamSourceNoExternalEntitities((StreamSource) source); } else { throw new IllegalArgumentException("Unknown Source type: " + source.getClass()); @@ -175,7 +203,9 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { * @throws SAXException if thrown by JAXP methods */ protected XMLReader createXmlReader() throws SAXException { - return XMLReaderFactory.createXMLReader(); + XMLReader xmlReader = XMLReaderFactory.createXMLReader(); + xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", isProcessExternalEntities()); + return xmlReader; } @@ -357,9 +387,43 @@ public abstract class AbstractMarshaller implements Marshaller, Unmarshaller { return unmarshalSaxReader(saxSource.getXMLReader(), saxSource.getInputSource()); } + /** + * Template method for handling {@code StreamSource}s with protection against + * the XML External Entity (XXE) processing vulnerability taking into account + * the value of the {@link #setProcessExternalEntities(boolean)} property. + *

+ * The default implementation wraps the StreamSource as a SAXSource and delegates + * to {@link #unmarshalSaxSource(javax.xml.transform.sax.SAXSource)}. + * + * @param streamSource the {@code StreamSource} + * @return the object graph + * @throws IOException if an I/O exception occurs + * @throws XmlMappingException if the given source cannot be mapped to an object + * + * @see XML_External_Entity_(XXE)_Processing + */ + protected Object unmarshalStreamSourceNoExternalEntitities(StreamSource streamSource) throws XmlMappingException, IOException { + InputSource inputSource; + if (streamSource.getInputStream() != null) { + inputSource = new InputSource(streamSource.getInputStream()); + inputSource.setEncoding(getDefaultEncoding()); + } + else if (streamSource.getReader() != null) { + inputSource = new InputSource(streamSource.getReader()); + } + else { + inputSource = new InputSource(streamSource.getSystemId()); + } + return unmarshalSaxSource(new SAXSource(inputSource)); + } + /** * Template method for handling {@code StreamSource}s. *

This implementation defers to {@code unmarshalInputStream} or {@code unmarshalReader}. + *

As of 3.2.8 and 4.0.2 this method is no longer invoked from + * {@link #unmarshal(javax.xml.transform.Source)}. The method invoked instead is + * {@link #unmarshalStreamSourceNoExternalEntitities(javax.xml.transform.stream.StreamSource)}. + * * @param streamSource the {@code StreamSource} * @return the object graph * @throws IOException if an I/O exception occurs diff --git a/spring-oxm/src/main/java/org/springframework/oxm/xmlbeans/XmlBeansMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/xmlbeans/XmlBeansMarshaller.java index 1fd49402c1..b3bb5cf11f 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/xmlbeans/XmlBeansMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/xmlbeans/XmlBeansMarshaller.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -113,6 +113,10 @@ public class XmlBeansMarshaller extends AbstractMarshaller { return this.validating; } + @Override + protected String getDefaultEncoding() { + return null; + } /** * This implementation returns true if the given class is an implementation of {@link XmlObject}. diff --git a/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java index de42e5bbef..52c121e86e 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java @@ -27,11 +27,9 @@ import java.lang.reflect.Constructor; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import javax.xml.stream.XMLEventReader; -import javax.xml.stream.XMLEventWriter; -import javax.xml.stream.XMLStreamException; -import javax.xml.stream.XMLStreamReader; -import javax.xml.stream.XMLStreamWriter; +import javax.xml.stream.*; +import javax.xml.transform.stax.StAXSource; +import javax.xml.transform.stream.StreamSource; import com.thoughtworks.xstream.MarshallingStrategy; import com.thoughtworks.xstream.XStream; @@ -342,6 +340,11 @@ public class XStreamMarshaller extends AbstractMarshaller implements Initializin this.encoding = encoding; } + @Override + protected String getDefaultEncoding() { + return this.encoding; + } + /** * Set the classes supported by this marshaller. *

If this property is empty (the default), all classes are supported. @@ -700,6 +703,13 @@ public class XStreamMarshaller extends AbstractMarshaller implements Initializin // Unmarshalling + @Override + protected Object unmarshalStreamSourceNoExternalEntitities(StreamSource streamSource) + throws XmlMappingException, IOException { + + return super.unmarshalStreamSource(streamSource); + } + @Override protected Object unmarshalDomNode(Node node) throws XmlMappingException { HierarchicalStreamReader streamReader; 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 585640893b..5500642125 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -19,6 +19,8 @@ package org.springframework.oxm.castor; 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; import org.junit.Ignore; @@ -28,9 +30,13 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.oxm.AbstractUnmarshallerTests; import org.springframework.oxm.MarshallingException; import org.springframework.oxm.Unmarshaller; +import org.xml.sax.InputSource; +import org.xml.sax.XMLReader; +import static junit.framework.Assert.assertNotNull; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** * @author Arjen Poutsma @@ -203,4 +209,59 @@ public class CastorUnmarshallerTests extends AbstractUnmarshallerTests { StreamSource source = new StreamSource(new StringReader(xml)); return unmarshaller.unmarshal(source); } + + @Test + public void unmarshalStreamSourceExternalEntities() throws Exception { + + final AtomicReference result = new AtomicReference(); + CastorMarshaller marshaller = new CastorMarshaller() { + @Override + protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource) { + result.set(xmlReader); + return null; + } + }; + + // 1. external-general-entities disabled (default) + + marshaller.unmarshal(new StreamSource("1")); + assertNotNull(result.get()); + assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); + + // 2. external-general-entities disabled (default) + + result.set(null); + marshaller.setProcessExternalEntities(true); + marshaller.unmarshal(new StreamSource("1")); + assertNotNull(result.get()); + assertEquals(true, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); + } + + @Test + public void unmarshalSaxSourceExternalEntities() throws Exception { + + final AtomicReference result = new AtomicReference(); + CastorMarshaller marshaller = new CastorMarshaller() { + @Override + protected Object unmarshalSaxReader(XMLReader xmlReader, InputSource inputSource) { + result.set(xmlReader); + return null; + } + }; + + // 1. external-general-entities disabled (default) + + marshaller.unmarshal(new SAXSource(new InputSource("1"))); + assertNotNull(result.get()); + assertEquals(false, result.get().getFeature("http://xml.org/sax/features/external-general-entities")); + + // 2. external-general-entities disabled (default) + + result.set(null); + marshaller.setProcessExternalEntities(true); + marshaller.unmarshal(new SAXSource(new InputSource("1"))); + assertNotNull(result.get()); + 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 af99408803..921a4b21e6 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-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -31,9 +31,12 @@ import javax.xml.bind.annotation.XmlType; import javax.xml.namespace.QName; import javax.xml.transform.Result; import javax.xml.transform.sax.SAXResult; +import javax.xml.transform.sax.SAXSource; import javax.xml.transform.stream.StreamResult; +import javax.xml.transform.stream.StreamSource; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; @@ -47,9 +50,7 @@ import org.springframework.oxm.jaxb.test.ObjectFactory; import org.springframework.oxm.mime.MimeContainer; import org.springframework.util.FileCopyUtils; import org.springframework.util.ReflectionUtils; -import org.xml.sax.Attributes; -import org.xml.sax.ContentHandler; -import org.xml.sax.Locator; +import org.xml.sax.*; import static org.junit.Assert.*; import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual; @@ -289,7 +290,7 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests { public void marshalAWrappedObjectHoldingAnXmlElementDeclElement() throws Exception { // SPR-10714 marshaller = new Jaxb2Marshaller(); - marshaller.setPackagesToScan(new String[] { "org.springframework.oxm.jaxb" }); + marshaller.setPackagesToScan(new String[]{"org.springframework.oxm.jaxb"}); marshaller.afterPropertiesSet(); Airplane airplane = new Airplane(); airplane.setName("test"); @@ -300,6 +301,75 @@ public class Jaxb2MarshallerTests extends AbstractMarshallerTests { writer.toString(), "test"); } + // SPR-10806 + + @Test + public void unmarshalStreamSourceExternalEntities() throws Exception { + + final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class); + Jaxb2Marshaller marshaller = new Jaxb2Marshaller() { + @Override + protected javax.xml.bind.Unmarshaller createUnmarshaller() { + return unmarshaller; + } + }; + + // 1. external-general-entities disabled (default) + + marshaller.unmarshal(new StreamSource("1")); + ArgumentCaptor sourceCaptor = ArgumentCaptor.forClass(SAXSource.class); + verify(unmarshaller).unmarshal(sourceCaptor.capture()); + + SAXSource result = sourceCaptor.getValue(); + assertEquals(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); + + // 2. external-general-entities enabled + + reset(unmarshaller); + marshaller.setProcessExternalEntities(true); + + marshaller.unmarshal(new StreamSource("1")); + verify(unmarshaller).unmarshal(sourceCaptor.capture()); + + result = sourceCaptor.getValue(); + assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); + } + + // SPR-10806 + + @Test + public void unmarshalSaxSourceExternalEntities() throws Exception { + + final javax.xml.bind.Unmarshaller unmarshaller = mock(javax.xml.bind.Unmarshaller.class); + Jaxb2Marshaller marshaller = new Jaxb2Marshaller() { + @Override + protected javax.xml.bind.Unmarshaller createUnmarshaller() { + return unmarshaller; + } + }; + + // 1. external-general-entities 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(false, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); + + // 2. external-general-entities enabled + + reset(unmarshaller); + marshaller.setProcessExternalEntities(true); + + marshaller.unmarshal(new SAXSource(new InputSource("1"))); + verify(unmarshaller).unmarshal(sourceCaptor.capture()); + + result = sourceCaptor.getValue(); + assertEquals(true, result.getXMLReader().getFeature("http://xml.org/sax/features/external-general-entities")); + } + + @XmlRootElement @SuppressWarnings("unused") public static class DummyRootElement { diff --git a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java index 14ab19c9c0..f7d26afb31 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java @@ -16,21 +16,34 @@ package org.springframework.oxm.jibx; +import java.io.IOException; import java.io.StringWriter; +import java.util.concurrent.atomic.AtomicReference; +import javax.xml.transform.sax.SAXSource; import javax.xml.transform.stream.StreamResult; +import javax.xml.transform.stream.StreamSource; import org.custommonkey.xmlunit.XMLUnit; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.springframework.oxm.AbstractMarshallerTests; import org.springframework.oxm.Marshaller; +import org.springframework.oxm.XmlMappingException; +import org.springframework.oxm.jaxb.Jaxb2Marshaller; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; +import org.xml.sax.InputSource; +import org.xml.sax.XMLReader; 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 static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; /** * @author Arjen Poutsma @@ -107,5 +120,4 @@ public class JibxMarshallerTests extends AbstractMarshallerTests { assertFalse("JibxMarshaller supports illegal type", marshaller.supports(getClass())); } - } diff --git a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java index b1e460d835..5ceeab22ac 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java @@ -28,7 +28,9 @@ import org.springframework.oxm.Unmarshaller; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + /** * @author Arjen Poutsma 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 676f6d6056..ad8d7d90b1 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-2010 the original author or authors. + * Copyright 2002-2014 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. @@ -28,6 +28,9 @@ import javax.xml.bind.annotation.XmlRootElement; import javax.xml.bind.annotation.XmlType; import javax.xml.transform.Result; import javax.xml.transform.Source; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.stream.StreamSource; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.http.HttpHeaders; @@ -36,6 +39,11 @@ import org.springframework.http.converter.HttpMessageConversionException; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.http.converter.HttpMessageNotWritableException; import org.springframework.util.ClassUtils; +import org.springframework.util.xml.StaxUtils; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.XMLReaderFactory; /** * Implementation of {@link org.springframework.http.converter.HttpMessageConverter HttpMessageConverter} that can read @@ -49,6 +57,17 @@ import org.springframework.util.ClassUtils; */ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessageConverter { + private boolean processExternalEntities = false; + + + /** + * Indicates whether external XML entities are processed when converting to a Source. + *

Default is {@code false}, meaning that external entities are not resolved. + */ + public void setProcessExternalEntities(boolean processExternalEntities) { + this.processExternalEntities = processExternalEntities; + } + @Override public boolean canRead(Class clazz, MediaType mediaType) { return (clazz.isAnnotationPresent(XmlRootElement.class) || clazz.isAnnotationPresent(XmlType.class)) && @@ -69,6 +88,7 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa @Override protected Object readFromSource(Class clazz, HttpHeaders headers, Source source) throws IOException { try { + source = processSource(source); Unmarshaller unmarshaller = createUnmarshaller(clazz); if (clazz.isAnnotationPresent(XmlRootElement.class)) { return unmarshaller.unmarshal(source); @@ -87,6 +107,26 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa } } + protected Source processSource(Source source) { + if (source instanceof StreamSource) { + StreamSource streamSource = (StreamSource) source; + InputSource inputSource = new InputSource(streamSource.getInputStream()); + try { + XMLReader xmlReader = XMLReaderFactory.createXMLReader(); + String featureName = "http://xml.org/sax/features/external-general-entities"; + xmlReader.setFeature(featureName, this.processExternalEntities); + return new SAXSource(xmlReader, inputSource); + } + catch (SAXException ex) { + logger.warn("Processing of external entities could not be disabled", ex); + return source; + } + } + else { + return source; + } + } + @Override protected void writeToResult(Object o, HttpHeaders headers, Result result) throws IOException { try { 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 e970450cf3..ec7daec5fc 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 @@ -95,6 +95,12 @@ public class SourceHttpMessageConverter extends AbstractHttpMe this.processExternalEntities = processExternalEntities; } + /** + * @return the configured value for whether XML external entities are allowed. + */ + public boolean isProcessExternalEntities() { + return this.processExternalEntities; + } @Override public boolean supports(Class clazz) { @@ -159,8 +165,7 @@ public class SourceHttpMessageConverter extends AbstractHttpMe private Source readStAXSource(InputStream body) { try { XMLInputFactory inputFactory = XMLInputFactory.newFactory(); - inputFactory.setProperty( - "javax.xml.stream.isSupportingExternalEntities", this.processExternalEntities); + inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, this.processExternalEntities); XMLStreamReader streamReader = inputFactory.createXMLStreamReader(body); return new StAXSource(streamReader); } 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 30b7cc0e52..fe1e39283d 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 @@ -32,9 +32,13 @@ import org.junit.Test; import org.springframework.aop.framework.AdvisedSupport; import org.springframework.aop.framework.AopProxy; import org.springframework.aop.framework.DefaultAopProxyFactory; +import org.springframework.core.io.ClassPathResource; +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.xml.sax.SAXParseException; /** @author Arjen Poutsma */ public class Jaxb2RootElementHttpMessageConverterTests { @@ -95,6 +99,33 @@ public class Jaxb2RootElementHttpMessageConverterTests { assertEquals("Invalid result", "Hello World", result.s); } + @Test + public void readXmlRootElementExternalEntityDisabled() throws Exception { + Resource external = new ClassPathResource("external.txt", getClass()); + String content = "\n" + + " ]>" + + " &ext;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + RootElement rootElement = (RootElement) converter.read(RootElement.class, inputMessage); + + assertEquals("", rootElement.external); + } + + @Test + public void readXmlRootElementExternalEntityEnabled() throws Exception { + Resource external = new ClassPathResource("external.txt", getClass()); + String content = "\n" + + " ]>" + + " &ext;"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(content.getBytes("UTF-8")); + this.converter.setProcessExternalEntities(true); + RootElement rootElement = (RootElement) converter.read(RootElement.class, inputMessage); + + assertEquals("Foo Bar", rootElement.external); + } + @Test public void writeXmlRootElement() throws Exception { MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); @@ -120,6 +151,9 @@ public class Jaxb2RootElementHttpMessageConverterTests { private Type type = new Type(); + @XmlElement(required=false) + public String external; + public Type getType() { return this.type; }