Improve charset management in XpathResultMatchers

Prior to this change, `XpathResultMatchers` and more generally the
`MockHttpServletResponse` would default to ISO-8859-1 encoding even when
it's not supposed to. The Servlet/HTTP specs mention this encoding
for all `text/*` mime types when decoding bodies to Strings, but this
issue is about XML Parsers.

XML Parsers should use the encoding:

* defined in the `Content-Type` response header (if available)
* written in the XML declaration of the document
* "guessed" by a built-in auto-detection mechanism

This commit changes the following:

* XPathMatchers now feed the XML parser with byte arrays instead of
decoded Strings
* the response should be written to `MockHttpServletResponse` using
its OutputStream, and not a PrintWriter which defaults to ISO-8859-1

Issue: SPR-12676

spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java

@@ -140,6 +140,14 @@ public class MockHttpServletResponse implements HttpServletResponse {
 		return this.writerAccessAllowed;
 	}
 
+	/**
+	 * Return whether the character encoding has been set.
+	 * <p>If {@code false}, {@link #getCharacterEncoding()} will return a default encoding value.
+	 */
+	public boolean isCharset() {
+		return charset;
+	}
+
 	@Override
 	public void setCharacterEncoding(String characterEncoding) {
 		this.characterEncoding = characterEncoding;

spring-test/src/main/java/org/springframework/test/util/XpathExpectationsHelper.java

@@ -16,9 +16,13 @@
 
 package org.springframework.test.util;
 
-import java.io.StringReader;
+import static org.hamcrest.MatcherAssert.*;
+import static org.springframework.test.util.AssertionErrors.*;
+
+import java.io.ByteArrayInputStream;
 import java.util.Collections;
 import java.util.Map;
+
 import javax.xml.namespace.QName;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -35,11 +39,9 @@ import org.w3c.dom.NodeList;
 import org.xml.sax.InputSource;
 
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.StringUtils;
 import org.springframework.util.xml.SimpleNamespaceContext;
 
-import static org.hamcrest.MatcherAssert.*;
-import static org.springframework.test.util.AssertionErrors.*;
-
 /**
  * A helper class for applying assertions via XPath expressions.
  *
@@ -93,8 +95,8 @@ public class XpathExpectationsHelper {
 	 * Parse the content, evaluate the XPath expression as a {@link Node}, and
 	 * assert it with the given {@code Matcher<Node>}.
 	 */
-	public void assertNode(String content, final Matcher<? super Node> matcher) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertNode(byte[] content, String encoding, final Matcher<? super Node> matcher) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		Node node = evaluateXpath(document, XPathConstants.NODE, Node.class);
 		assertThat("XPath " + this.expression, node, matcher);
 	}
@@ -102,14 +104,18 @@ public class XpathExpectationsHelper {
 	/**
 	 * Parse the given XML content to a {@link Document}.
 	 * @param xml the content to parse
+	 * @param encoding optional content encoding, if provided as metadata (e.g. in HTTP headers)
 	 * @return the parsed document
-	 * @throws Exception in case of errors
+	 * @throws Exception
 	 */
-	protected Document parseXmlString(String xml) throws Exception  {
+	protected Document parseXmlByteArray(byte[] xml, String encoding) throws Exception {
 		DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
 		factory.setNamespaceAware(this.hasNamespaces);
 		DocumentBuilder documentBuilder = factory.newDocumentBuilder();
-		InputSource inputSource = new InputSource(new StringReader(xml));
+		InputSource inputSource = new InputSource(new ByteArrayInputStream(xml));
+		if(StringUtils.hasText(encoding)) {
+			inputSource.setEncoding(encoding);
+		}
 		return documentBuilder.parse(inputSource);
 	}
 
@@ -128,8 +134,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content exists.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void exists(String content) throws Exception {
-		Document document = parseXmlString(content);
+	public void exists(byte[] content, String encoding) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		Node node = evaluateXpath(document, XPathConstants.NODE, Node.class);
 		assertTrue("XPath " + this.expression + " does not exist", node != null);
 	}
@@ -138,8 +144,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content does not exist.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void doesNotExist(String content) throws Exception {
-		Document document = parseXmlString(content);
+	public void doesNotExist(byte[] content, String encoding) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		Node node = evaluateXpath(document, XPathConstants.NODE, Node.class);
 		assertTrue("XPath " + this.expression + " exists", node == null);
 	}
@@ -149,8 +155,8 @@ public class XpathExpectationsHelper {
 	 * given Hamcrest matcher.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertNodeCount(String content, Matcher<Integer> matcher) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertNodeCount(byte[] content, String encoding, Matcher<Integer> matcher) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		NodeList nodeList = evaluateXpath(document, XPathConstants.NODESET, NodeList.class);
 		assertThat("nodeCount for XPath " + this.expression, nodeList.getLength(), matcher);
 	}
@@ -159,8 +165,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content as an integer.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertNodeCount(String content, int expectedCount) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertNodeCount(byte[] content, String encoding, int expectedCount) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		NodeList nodeList = evaluateXpath(document, XPathConstants.NODESET, NodeList.class);
 		assertEquals("nodeCount for XPath " + this.expression, expectedCount, nodeList.getLength());
 	}
@@ -170,8 +176,8 @@ public class XpathExpectationsHelper {
 	 * given Hamcrest matcher.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertString(String content, Matcher<? super String> matcher) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertString(byte[] content, String encoding, Matcher<? super String> matcher) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		String result = evaluateXpath(document,  XPathConstants.STRING, String.class);
 		assertThat("XPath " + this.expression, result, matcher);
 	}
@@ -180,8 +186,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content as a String.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertString(String content, String expectedValue) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertString(byte[] content, String encoding, String expectedValue) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		String actual = evaluateXpath(document,  XPathConstants.STRING, String.class);
 		assertEquals("XPath " + this.expression, expectedValue, actual);
 	}
@@ -191,8 +197,8 @@ public class XpathExpectationsHelper {
 	 * given Hamcrest matcher.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertNumber(String content, Matcher<? super Double> matcher) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertNumber(byte[] content, String encoding, Matcher<? super Double> matcher) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		Double result = evaluateXpath(document, XPathConstants.NUMBER, Double.class);
 		assertThat("XPath " + this.expression, result, matcher);
 	}
@@ -201,8 +207,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content as a Double.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertNumber(String content, Double expectedValue) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertNumber(byte[] content, String encoding, Double expectedValue) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		Double actual = evaluateXpath(document, XPathConstants.NUMBER, Double.class);
 		assertEquals("XPath " + this.expression, expectedValue, actual);
 	}
@@ -211,8 +217,8 @@ public class XpathExpectationsHelper {
 	 * Apply the XPath expression and assert the resulting content as a Boolean.
 	 * @throws Exception if content parsing or expression evaluation fails
 	 */
-	public void assertBoolean(String content, boolean expectedValue) throws Exception {
-		Document document = parseXmlString(content);
+	public void assertBoolean(byte[] content, String encoding, boolean expectedValue) throws Exception {
+		Document document = parseXmlByteArray(content, encoding);
 		String actual = evaluateXpath(document, XPathConstants.STRING, String.class);
 		assertEquals("XPath " + this.expression, expectedValue, Boolean.parseBoolean(actual));
 	}

spring-test/src/main/java/org/springframework/test/web/client/match/XpathRequestMatchers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 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,8 @@ import org.springframework.test.web.client.RequestMatcher;
  */
 public class XpathRequestMatchers {
 
+	private static final String DEFAULT_ENCODING = "UTF-8";
+
 	private final XpathExpectationsHelper xpathHelper;
 
 
@@ -65,7 +67,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertNode(request.getBodyAsString(), matcher);
+				xpathHelper.assertNode(request.getBodyAsBytes(), DEFAULT_ENCODING, matcher);
 			}
 		};
 	}
@@ -77,7 +79,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.exists(request.getBodyAsString());
+				xpathHelper.exists(request.getBodyAsBytes(), DEFAULT_ENCODING);
 			}
 		};
 	}
@@ -89,7 +91,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.doesNotExist(request.getBodyAsString());
+				xpathHelper.doesNotExist(request.getBodyAsBytes(), DEFAULT_ENCODING);
 			}
 		};
 	}
@@ -102,7 +104,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertNodeCount(request.getBodyAsString(), matcher);
+				xpathHelper.assertNodeCount(request.getBodyAsBytes(), DEFAULT_ENCODING, matcher);
 			}
 		};
 	}
@@ -114,7 +116,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertNodeCount(request.getBodyAsString(), expectedCount);
+				xpathHelper.assertNodeCount(request.getBodyAsBytes(), DEFAULT_ENCODING, expectedCount);
 			}
 		};
 	}
@@ -126,7 +128,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertString(request.getBodyAsString(), matcher);
+				xpathHelper.assertString(request.getBodyAsBytes(), DEFAULT_ENCODING, matcher);
 			}
 		};
 	}
@@ -138,7 +140,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertString(request.getBodyAsString(), value);
+				xpathHelper.assertString(request.getBodyAsBytes(), DEFAULT_ENCODING, value);
 			}
 		};
 	}
@@ -150,7 +152,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertNumber(request.getBodyAsString(), matcher);
+				xpathHelper.assertNumber(request.getBodyAsBytes(), DEFAULT_ENCODING, matcher);
 			}
 		};
 	}
@@ -162,7 +164,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertNumber(request.getBodyAsString(), value);
+				xpathHelper.assertNumber(request.getBodyAsBytes(), DEFAULT_ENCODING, value);
 			}
 		};
 	}
@@ -174,7 +176,7 @@ public class XpathRequestMatchers {
 		return new AbstractXpathRequestMatcher() {
 			@Override
 			protected void matchInternal(MockClientHttpRequest request) throws Exception {
-				xpathHelper.assertBoolean(request.getBodyAsString(), value);
+				xpathHelper.assertBoolean(request.getBodyAsBytes(), DEFAULT_ENCODING, value);
 			}
 		};
 	}

spring-test/src/main/java/org/springframework/test/web/servlet/result/XpathResultMatchers.java

@@ -22,6 +22,7 @@ import javax.xml.xpath.XPathExpressionException;
 import org.hamcrest.Matcher;
 import org.w3c.dom.Node;
 
+import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.test.util.XpathExpectationsHelper;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.ResultMatcher;
@@ -62,12 +63,19 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertNode(content, matcher);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertNode(response.getContentAsByteArray(), getDefinedEncoding(response), matcher);
 			}
 		};
 	}
 
+	/**
+	 * Get the response encoding if explicitely defined in the response, null otherwise
+	 */
+	private String getDefinedEncoding(MockHttpServletResponse response) {
+		return response.isCharset() ? response.getCharacterEncoding() : null;
+	}
+
 	/**
 	 * Evaluate the XPath and assert that content exists.
 	 */
@@ -75,8 +83,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.exists(content);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.exists(response.getContentAsByteArray(), getDefinedEncoding(response));
 			}
 		};
 	}
@@ -88,8 +96,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.doesNotExist(content);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.doesNotExist(response.getContentAsByteArray(), getDefinedEncoding(response));
 			}
 		};
 	}
@@ -102,8 +110,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertNodeCount(content, matcher);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertNodeCount(response.getContentAsByteArray(), getDefinedEncoding(response), matcher);
 			}
 		};
 	}
@@ -115,8 +123,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertNodeCount(content, expectedCount);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertNodeCount(response.getContentAsByteArray(), getDefinedEncoding(response), expectedCount);
 			}
 		};
 	}
@@ -129,8 +137,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertString(content, matcher);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertString(response.getContentAsByteArray(), getDefinedEncoding(response), matcher);
 			}
 		};
 	}
@@ -142,8 +150,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertString(content, expectedValue);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertString(response.getContentAsByteArray(), getDefinedEncoding(response), expectedValue);
 			}
 		};
 	}
@@ -156,8 +164,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertNumber(content, matcher);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertNumber(response.getContentAsByteArray(), getDefinedEncoding(response), matcher);
 			}
 		};
 	}
@@ -169,8 +177,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertNumber(content, expectedValue);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertNumber(response.getContentAsByteArray(), getDefinedEncoding(response), expectedValue);
 			}
 		};
 	}
@@ -182,8 +190,8 @@ public class XpathResultMatchers {
 		return new ResultMatcher() {
 			@Override
 			public void match(MvcResult result) throws Exception {
-				String content = result.getResponse().getContentAsString();
-				xpathHelper.assertBoolean(content, value);
+				MockHttpServletResponse response = result.getResponse();
+				xpathHelper.assertBoolean(response.getContentAsByteArray(), getDefinedEncoding(response), value);
 			}
 		};
 	}

spring-test/src/test/java/org/springframework/test/web/servlet/result/XpathResultMatchersTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 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.
@@ -15,11 +15,15 @@
  */
 package org.springframework.test.web.servlet.result;
 
+import java.nio.charset.Charset;
+
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.test.web.servlet.StubMvcResult;
+import org.springframework.util.StreamUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * Tests for {@link XpathResultMatchers}.
@@ -98,12 +102,25 @@ public class XpathResultMatchersTests {
 		new XpathResultMatchers("/foo/bar[2]", null).booleanValue(false).match(getStubMvcResult());
 	}
 
+	@Test
+	public void testStringEncodingDetection() throws Exception {
+		String content = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\n" +
+				"<person><name>Jürgen</name></person>";
+		byte[] bytes = content.getBytes(Charset.forName("UTF-8"));
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		response.addHeader("Content-Type", "application/xml");
+		StreamUtils.copy(bytes, response.getOutputStream());
+		StubMvcResult result = new StubMvcResult(null, null, null, null, null, null, response);
+
+		new XpathResultMatchers("/person/name", null).string("Jürgen").match(result);
+	}
+
 
 	private static final String RESPONSE_CONTENT = "<foo><bar>111</bar><bar>true</bar></foo>";
 
 	private StubMvcResult getStubMvcResult() throws Exception {
 		MockHttpServletResponse response = new MockHttpServletResponse();
-		response.addHeader("Content-Type", "application/json");
+		response.addHeader("Content-Type", "application/xml");
 		response.getWriter().print(new String(RESPONSE_CONTENT.getBytes("ISO-8859-1")));
 		return new StubMvcResult(null, null, null, null, null, null, response);
 	}