Disable ext entities in SourceHttpMessageConverter

This change disables the processing of external entities in
SourceHttpMessageConverter by default and provides an option to enable
it if required.

spring-web/src/main/java/org/springframework/http/converter/xml/SourceHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -17,73 +17,144 @@
 package org.springframework.http.converter.xml;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
 import javax.xml.transform.TransformerException;
-import javax.xml.transform.dom.DOMResult;
+import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stax.StAXSource;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 
+import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLReaderFactory;
 
-import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpInputMessage;
+import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
+import org.springframework.http.converter.AbstractHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConversionException;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.http.converter.HttpMessageNotWritableException;
+import org.springframework.util.StreamUtils;
 
 /**
- * Implementation of {@link org.springframework.http.converter.HttpMessageConverter} that can read and write {@link
- * Source} objects.
+ * Implementation of {@link org.springframework.http.converter.HttpMessageConverter}
+ * that can read and write {@link Source} objects.
  *
  * @author Arjen Poutsma
  * @since 3.0
  */
-public class SourceHttpMessageConverter<T extends Source> extends AbstractXmlHttpMessageConverter<T> {
+public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMessageConverter<T> {
+
+	private final TransformerFactory transformerFactory = TransformerFactory.newInstance();
+
+	private boolean processExternalEntities = false;
+
+	/**
+	 * Sets the {@link #setSupportedMediaTypes(java.util.List) supportedMediaTypes}
+	 * to {@code text/xml} and {@code application/xml}, and {@code application/*-xml}.
+	 */
+	public SourceHttpMessageConverter() {
+		super(MediaType.APPLICATION_XML, MediaType.TEXT_XML, new MediaType("application", "*+xml"));
+	}
+
+
+	/**
+	 * Indicates whether external XML entities are processed when converting
+	 * to a Source.
+	 * <p>Default is {@code false}, meaning that external entities are not resolved.
+	 */
+	public void setProcessExternalEntities(boolean processExternalEntities) {
+		this.processExternalEntities = processExternalEntities;
+	}
 
 	@Override
 	public boolean supports(Class<?> clazz) {
-		return DOMSource.class.equals(clazz) || SAXSource.class.equals(clazz) || StreamSource.class.equals(clazz) ||
-				Source.class.equals(clazz);
+		return DOMSource.class.equals(clazz) || SAXSource.class.equals(clazz)
+				|| StreamSource.class.equals(clazz) || Source.class.equals(clazz);
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
-	protected T readFromSource(Class clazz, HttpHeaders headers, Source source) throws IOException {
+	protected T readInternal(Class<? extends T> clazz, HttpInputMessage inputMessage)
+			throws IOException, HttpMessageNotReadableException {
+
+		InputStream body = inputMessage.getBody();
+		if (DOMSource.class.equals(clazz)) {
+			return (T) readDOMSource(body);
+		}
+		else if (SAXSource.class.equals(clazz)) {
+			return (T) readSAXSource(body);
+		}
+		else if (StAXSource.class.equals(clazz)) {
+			return (T) readStAXSource(body);
+		}
+		else if (StreamSource.class.equals(clazz) || Source.class.equals(clazz)) {
+			return (T) readStreamSource(body);
+		}
+		else {
+			throw new HttpMessageConversionException("Could not read class [" + clazz +
+					"]. Only DOMSource, SAXSource, and StreamSource are supported.");
+		}
+	}
+
+	private DOMSource readDOMSource(InputStream body) throws IOException {
 		try {
-			if (DOMSource.class.equals(clazz)) {
-				DOMResult domResult = new DOMResult();
-				transform(source, domResult);
-				return (T) new DOMSource(domResult.getNode());
-			}
-			else if (SAXSource.class.equals(clazz)) {
-				ByteArrayInputStream bis = transformToByteArrayInputStream(source);
-				return (T) new SAXSource(new InputSource(bis));
-			}
-			else if (StreamSource.class.equals(clazz) || Source.class.equals(clazz)) {
-				ByteArrayInputStream bis = transformToByteArrayInputStream(source);
-				return (T) new StreamSource(bis);
-			}
-			else {
-				throw new HttpMessageConversionException("Could not read class [" + clazz +
-						"]. Only DOMSource, SAXSource, and StreamSource are supported.");
-			}
+			DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+			documentBuilderFactory.setNamespaceAware(true);
+			documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", processExternalEntities);
+			DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder();
+			Document document = documentBuilder.parse(body);
+			return new DOMSource(document);
 		}
-		catch (TransformerException ex) {
-			throw new HttpMessageNotReadableException("Could not transform from [" + source + "] to [" + clazz + "]",
-					ex);
+		catch (ParserConfigurationException ex) {
+			throw new HttpMessageNotReadableException("Could not set feature: " + ex.getMessage(), ex);
+		}
+		catch (SAXException ex) {
+			throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex);
 		}
 	}
 
-	private ByteArrayInputStream transformToByteArrayInputStream(Source source) throws TransformerException {
-		ByteArrayOutputStream bos = new ByteArrayOutputStream();
-		transform(source, new StreamResult(bos));
-		return new ByteArrayInputStream(bos.toByteArray());
+	private SAXSource readSAXSource(InputStream body) throws IOException {
+		try {
+			XMLReader reader = XMLReaderFactory.createXMLReader();
+			reader.setFeature("http://xml.org/sax/features/external-general-entities", processExternalEntities);
+			byte[] bytes = StreamUtils.copyToByteArray(body);
+			return new SAXSource(reader, new InputSource(new ByteArrayInputStream(bytes)));
+		}
+		catch (SAXException ex) {
+			throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex);
+		}
+	}
+
+	private Source readStAXSource(InputStream body) {
+		try {
+			XMLInputFactory inputFactory = XMLInputFactory.newFactory();
+			inputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", processExternalEntities);
+			XMLStreamReader streamReader = inputFactory.createXMLStreamReader(body);
+			return new StAXSource(streamReader);
+		}
+		catch (XMLStreamException ex) {
+			throw new HttpMessageNotReadableException("Could not parse document: " + ex.getMessage(), ex);
+		}
+	}
+
+	private StreamSource readStreamSource(InputStream body) throws IOException {
+		byte[] bytes = StreamUtils.copyToByteArray(body);
+		return new StreamSource(new ByteArrayInputStream(bytes));
 	}
 
 	@Override
@@ -102,15 +173,22 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractXmlHtt
 	}
 
 	@Override
-	protected void writeToResult(T t, HttpHeaders headers, Result result) throws IOException {
+	protected void writeInternal(T t, HttpOutputMessage outputMessage)
+			throws IOException, HttpMessageNotWritableException {
 		try {
+			Result result = new StreamResult(outputMessage.getBody());
 			transform(t, result);
 		}
 		catch (TransformerException ex) {
-			throw new HttpMessageNotWritableException("Could not transform [" + t + "] to [" + result + "]", ex);
+			throw new HttpMessageNotWritableException("Could not transform [" + t + "] to output message", ex);
 		}
 	}
 
+	private void transform(Source source, Result result) throws TransformerException {
+		this.transformerFactory.newTransformer().transform(source, result);
+	}
+
+
 	private static class CountingOutputStream extends OutputStream {
 
 		private long count = 0;

spring-web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java

@@ -17,21 +17,29 @@
 package org.springframework.http.converter.xml;
 
 import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
+import static org.junit.Assert.assertNotEquals;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
 import java.nio.charset.Charset;
 
 import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
 import javax.xml.transform.Source;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stax.StAXSource;
 import javax.xml.transform.stream.StreamSource;
 
 import org.junit.Before;
 import org.junit.Test;
+
+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;
@@ -39,17 +47,29 @@ import org.springframework.util.FileCopyUtils;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.DefaultHandler;
 
 /**
  * @author Arjen Poutsma
  */
 public class SourceHttpMessageConverterTests {
 
+	private static final String BODY = "<root>Hello World</root>";
+
 	private SourceHttpMessageConverter<Source> converter;
 
+	private String bodyExternal;
+
 	@Before
-	public void setUp() {
+	public void setUp() throws IOException {
 		converter = new SourceHttpMessageConverter<Source>();
+		Resource external = new ClassPathResource("external.txt", getClass());
+
+		bodyExternal = "<!DOCTYPE root [" +
+				"  <!ELEMENT root ANY >\n" +
+				"  <!ENTITY ext SYSTEM \"" + external.getURI() + "\" >]><root>&ext;</root>";
 	}
 
 	@Test
@@ -67,39 +87,94 @@ public class SourceHttpMessageConverterTests {
 
 	@Test
 	public void readDOMSource() throws Exception {
-		String body = "<root>Hello World</root>";
-		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
 		DOMSource result = (DOMSource) converter.read(DOMSource.class, inputMessage);
 		Document document = (Document) result.getNode();
 		assertEquals("Invalid result", "root", document.getDocumentElement().getLocalName());
 	}
 
+	@Test
+	public void readDOMSourceExternal() throws Exception {
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8"));
+		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		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 readSAXSource() throws Exception {
-		String body = "<root>Hello World</root>";
-		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
 		SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage);
 		InputSource inputSource = result.getInputSource();
 		String s = FileCopyUtils.copyToString(new InputStreamReader(inputSource.getByteStream()));
-		assertXMLEqual("Invalid result", body, s);
+		assertXMLEqual("Invalid result", BODY, s);
+	}
+
+	@Test
+	public void readSAXSourceExternal() throws Exception {
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8"));
+		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage);
+		InputSource inputSource = result.getInputSource();
+		XMLReader reader = result.getXMLReader();
+		reader.setContentHandler(new DefaultHandler() {
+			@Override
+			public void characters(char[] ch, int start, int length) throws SAXException {
+				String s = new String(ch, start, length);
+				assertNotEquals("Invalid result", "Foo Bar", s);
+			}
+		});
+		reader.parse(inputSource);
 	}
 
+	@Test
+	public void readStAXSource() throws Exception {
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
+		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		StAXSource result = (StAXSource) converter.read(StAXSource.class, inputMessage);
+		XMLStreamReader streamReader = result.getXMLStreamReader();
+		assertTrue(streamReader.hasNext());
+		streamReader.nextTag();
+		String s = streamReader.getLocalName();
+		assertEquals("root", s);
+		s = streamReader.getElementText();
+		assertEquals("Hello World", s);
+		streamReader.close();
+	}
+
+	@Test
+	public void readStAXSourceExternal() throws Exception {
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(bodyExternal.getBytes("UTF-8"));
+		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
+		StAXSource result = (StAXSource) converter.read(StAXSource.class, inputMessage);
+		XMLStreamReader streamReader = result.getXMLStreamReader();
+		assertTrue(streamReader.hasNext());
+		streamReader.next();
+		streamReader.next();
+		String s = streamReader.getLocalName();
+		assertEquals("root", s);
+		s = streamReader.getElementText();
+		assertNotEquals("Foo Bar", s);
+		streamReader.close();
+	}
+
+
 	@Test
 	public void readStreamSource() throws Exception {
-		String body = "<root>Hello World</root>";
-		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
 		StreamSource result = (StreamSource) converter.read(StreamSource.class, inputMessage);
 		String s = FileCopyUtils.copyToString(new InputStreamReader(result.getInputStream()));
-		assertXMLEqual("Invalid result", body, s);
+		assertXMLEqual("Invalid result", BODY, s);
 	}
 
 	@Test
 	public void readSource() throws Exception {
-		String body = "<root>Hello World</root>";
-		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(BODY.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
 		converter.read(Source.class, inputMessage);
 	}

spring-web/src/test/resources/org/springframework/http/converter/xml/external.txt

@@ -0,0 +1 @@
+Foo Bar