From fd94d3145d6cd2c41b07cdc76d6aa84319c1ad4d Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Wed, 11 Apr 2018 17:21:46 +0200
Subject: [PATCH] DATACMNS-1292 - Improved default setup of XMLBeam.

We now explicitly disable entity expansion in the DocumentBuilderFactory used by XMLBeam.

Introduced constructor in XmlBeamHttpMessageConverter to allow dedicated configuration of an XBProjector instance in case the defaults need tweaking and augmented the web configuration setup to automatically pick up a custom XmlBeamHttpMessageConverter bean instead of our own default if present.
---
 .../data/web/XmlBeamHttpMessageConverter.java | 52 ++++++++++++++++++-
 .../config/SpringDataWebConfiguration.java    |  3 +-
 .../XmlBeamHttpMessageConverterUnitTests.java | 15 ++++++
 3 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/springframework/data/web/XmlBeamHttpMessageConverter.java b/src/main/java/org/springframework/data/web/XmlBeamHttpMessageConverter.java
index 9f6107d6c..0d139ea95 100644
--- a/src/main/java/org/springframework/data/web/XmlBeamHttpMessageConverter.java
+++ b/src/main/java/org/springframework/data/web/XmlBeamHttpMessageConverter.java
@@ -19,6 +19,7 @@ import java.io.IOException;
 import java.util.Map;
 
 import javax.annotation.Nullable;
+import javax.xml.parsers.DocumentBuilderFactory;
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotationUtils;
@@ -29,8 +30,11 @@ import org.springframework.http.converter.AbstractHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.http.converter.HttpMessageNotWritableException;
+import org.springframework.util.Assert;
 import org.springframework.util.ConcurrentReferenceHashMap;
+import org.xml.sax.SAXParseException;
 import org.xmlbeam.XBProjector;
+import org.xmlbeam.config.DefaultXMLFactoriesConfig;
 
 /**
  * A read-only {@link HttpMessageConverter} to create XMLBeam-based projection instances for interfaces.
@@ -50,9 +54,39 @@ public class XmlBeamHttpMessageConverter extends AbstractHttpMessageConverter<Ob
 	 */
 	public XmlBeamHttpMessageConverter() {
 
+		this(new XBProjector(new DefaultXMLFactoriesConfig() {
+
+			private static final long serialVersionUID = -1324345769124477493L;
+
+			/* 
+			 * (non-Javadoc)
+			 * @see org.xmlbeam.config.DefaultXMLFactoriesConfig#createDocumentBuilderFactory()
+			 */
+			@Override
+			public DocumentBuilderFactory createDocumentBuilderFactory() {
+
+				DocumentBuilderFactory factory = super.createDocumentBuilderFactory();
+
+				factory.setAttribute("http://apache.org/xml/features/disallow-doctype-decl", true);
+				factory.setAttribute("http://xml.org/sax/features/external-general-entities", false);
+
+				return factory;
+			}
+		}));
+	}
+
+	/**
+	 * Creates a new {@link XmlBeamHttpMessageConverter} using the given {@link XBProjector}.
+	 * 
+	 * @param projector must not be {@literal null}.
+	 */
+	public XmlBeamHttpMessageConverter(XBProjector projector) {
+
 		super(MediaType.APPLICATION_XML, MediaType.parseMediaType("application/*+xml"));
 
-		this.projectionFactory = new XBProjector();
+		Assert.notNull(projector, "XBProjector must not be null!");
+
+		this.projectionFactory = projector;
 	}
 
 	/*
@@ -92,7 +126,21 @@ public class XmlBeamHttpMessageConverter extends AbstractHttpMessageConverter<Ob
 	@Override
 	protected Object readInternal(Class<? extends Object> clazz, HttpInputMessage inputMessage)
 			throws IOException, HttpMessageNotReadableException {
-		return projectionFactory.io().stream(inputMessage.getBody()).read(clazz);
+
+		try {
+
+			return projectionFactory.io().stream(inputMessage.getBody()).read(clazz);
+
+		} catch (RuntimeException o_O) {
+
+			Throwable cause = o_O.getCause();
+
+			if (SAXParseException.class.isInstance(cause)) {
+				throw new HttpMessageNotReadableException("Cannot read input message!", cause);
+			} else {
+				throw o_O;
+			}
+		}
 	}
 
 	/* 
diff --git a/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java b/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java
index c7beccf48..9b971c8b6 100644
--- a/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java
+++ b/src/main/java/org/springframework/data/web/config/SpringDataWebConfiguration.java
@@ -65,6 +65,7 @@ public class SpringDataWebConfiguration implements WebMvcConfigurer, BeanClassLo
 
 	private @Autowired Optional<PageableHandlerMethodArgumentResolverCustomizer> pageableResolverCustomizer;
 	private @Autowired Optional<SortHandlerMethodArgumentResolverCustomizer> sortResolverCustomizer;
+	private @Autowired Optional<XmlBeamHttpMessageConverter> xmlBeamHttpMessageConverter;
 
 	public SpringDataWebConfiguration(ApplicationContext context,
 			@Qualifier("mvcConversionService") ObjectFactory<ConversionService> conversionService) {
@@ -167,7 +168,7 @@ public class SpringDataWebConfiguration implements WebMvcConfigurer, BeanClassLo
 		}
 
 		if (ClassUtils.isPresent("org.xmlbeam.XBProjector", context.getClassLoader())) {
-			converters.add(0, new XmlBeamHttpMessageConverter());
+			converters.add(0, xmlBeamHttpMessageConverter.orElseGet(() -> new XmlBeamHttpMessageConverter()));
 		}
 	}
 
diff --git a/src/test/java/org/springframework/data/web/XmlBeamHttpMessageConverterUnitTests.java b/src/test/java/org/springframework/data/web/XmlBeamHttpMessageConverterUnitTests.java
index f6d0758ec..48be3c10f 100755
--- a/src/test/java/org/springframework/data/web/XmlBeamHttpMessageConverterUnitTests.java
+++ b/src/test/java/org/springframework/data/web/XmlBeamHttpMessageConverterUnitTests.java
@@ -28,6 +28,8 @@ import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.data.web.ProjectingJackson2HttpMessageConverterUnitTests.UnannotatedInterface;
 import org.springframework.http.HttpInputMessage;
 import org.springframework.http.MediaType;
+import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.xml.sax.SAXParseException;
 import org.xmlbeam.annotation.XBRead;
 
 /**
@@ -87,6 +89,19 @@ public class XmlBeamHttpMessageConverterUnitTests {
 		assertThat(converter.canRead(Customer.class, MediaType.APPLICATION_XML)).isTrue();
 	}
 
+	@Test // DATACMNS-1292
+	public void doesNotSupportEntityExpansion() throws Exception {
+
+		preparePayload("<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?>\n" //
+				+ "<!DOCTYPE foo [\n" //
+				+ "<!ELEMENT foo ANY >\n" //
+				+ "<!ENTITY xxe \"Bar\" >]><user><firstname>&xxe;</firstname><lastname>Matthews</lastname></user>");
+
+		assertThatExceptionOfType(HttpMessageNotReadableException.class) //
+				.isThrownBy(() -> converter.read(Customer.class, message)) //
+				.withCauseInstanceOf(SAXParseException.class);
+	}
+
 	private void preparePayload(String payload) throws IOException {
 		when(message.getBody()).thenReturn(new ByteArrayInputStream(payload.getBytes()));
 	}