From b8974a292ab463a304eda987632be4d9c145f5f8 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 eb300b43f..a675fd885 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 603078871..e015c8702 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 cf4469cbd..23bff3cd8 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()));
 	}