XpathResultMatchers ignores character encoding of XML document [SPR-12676]

[spring-projects-issues]

Tor-Einar Jarnbjo opened SPR-12676 and commented

When using MockMvcResultMatcher.xpath(...) to assert the result of a REST service, the XML document in the response is decoded using WebUtils.DEFAULT_CHARACTER_ENCODING (ISO-8859-1) as defined in MockHttpServletResponse instead of the character set specified in the XML header. In the implementation of XpathResultMatchers, the XML document is accessed with result.getResponse().getContentAsString(), which only considers any character set potentially specified in the content-type HTTP header.

This fails, if the XML character set is not UTF-8 and the response does not specify the character encoding in the content-type header and contains characters encoded differently in UTF-8 and the actual default character set.

E.g.for the response

Content-Type: application/xml

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>

<foo>ö</foo>

the test

andExpect(xpath("/foo").string("ö"))

will fail with

java.lang.AssertionError: XPath /foo
Expected :ö
Actual :ö

---

Affects: 4.1.2

Referenced from: commits f988151

[spring-projects-issues]

Tor-Einar Jarnbjo commented

If the service always uses the same character encoding in the XML response, a possible workaround would be to implement a ResultHandler to set a content-type with charset parameter before asserting any expectations:

private static SetContentTypeResultHandler setContentType(String contentType) {
return new SetContentTypeResultHandler(contentType);
}

private static class SetContentTypeResultHandler implements ResultHandler {
private String contentType;
private SetContentTypeResultHandler(String contentType) {
this.contentType = contentType;
}
@Override
public void handle(MvcResult result) throws Exception {
result.getResponse().setContentType(contentType);
}
}

And then apply this ResultHandler before and andExcpect assertations:

mockMvc.perform(...). ...
andDo(setContentType("text/xml;charset=utf-8")). // workaround for #17275
andExpect(xpath("/foo").string("ö"));

[spring-projects-issues]

Brian Clozel commented

Hi Tor-Einar Jarnbjo

Indeed, XML parsers are supposed to auto detect the character encoding of the document using various techniques documented in the official spec.

In an HTTP client context things can be a bit different, and I suspect some browsers would be confused in this case as well (and even throw encoding errors).

The current behavior looks fine to me:

• rely on the Encoding header sent by the response to parse it
• use the default servlet encoding if none was given

Your workaround could work but I’ve got some concerns about it:

• ResultHandlers are mostly focusing on printing the result, rather than modifying the response
• this change would break the API flow, since the order of method calls would change the outcome here
• changing this is probably hiding a bug anyway, since the HTTP response is missing critical information to parse the response

Could you elaborate a bit more on your use case, especially why fixing the response in the first place is not possible?

Thanks,

[spring-projects-issues]

Tor-Einar Jarnbjo commented

The current behaviour is unfortunately not ok. The response producer is Spring MVC and the response conforms to the specifications.

If you check the specification you link to, you will see that the XML specification define two different approaches depending on wether external meta-data is unavailable (section F.1) or available (F.2). The techniques described in section F.1 can be used if the XML document is e.g. read from a file and the file system don't provide additional information about the used encoding. When the XML document is delivered over HTTP, section F.2 apply and refer to the specification of the content types text/xml and application/xml as defined in RFC 3032.

For content type text/xml, the default character set is actually us-ascii (RFC 3032, section 3.1). The XML parser must hence either use us-ascii (if not charset parameter is provided) or the provided character set.

For content type application/xml (RFC 3032, section 3.2), if no charset is provided, the RFC points back to the XML specification and requires that the XML parser should use the character encoding specified in the XML document.

Additionally, to comply strictly with the XML specification, the parser must also verify if a different character encoding is specified in the XML document header. If there is a mismatch between the character encoding provided externally and the character encoding in the XML document, the parser must fail.

Without caring much about all this, a trivial fix would however be simply not try any character set detection at all. Instead of getting the content as a string and forward the string to the XML parser, it would be much simpler to get the content as a byte array and create the InputSource with a ByteArrayInputStream from the byte[] (instead of the current implementation, which wraps the String in a StringReader) and if available, forward the charset from the content-type header to the InputSource (XpathExpectationsHelper.parseXmlString). Please consider #17276 as well, since currently, the charset parameter is not extracted correctly from the content-type header. A bug I ran into, when trying my workaround by setting the charset manually before asserting the content of the returned XML document.

[spring-projects-issues]

Brian Clozel commented

I totally agree, the current implementation is not conforming to the XML specification regarding XML documents encoding.

I've taken a look at your proposal:

• not guessing the encoding at all
• feeding the parser with a byte[] and an optional encoding value extracted from the response headers

First, the SAX implementation is not following those specs either:

The SAX parser will use the InputSource object to determine how to read XML input. If there is a character stream available, the parser will read that stream directly, disregarding any text encoding declaration found in that stream. If there is no character stream, but there is a byte stream, the parser will use that byte stream, using the encoding specified in the InputSource or else (if no encoding is specified) autodetecting the character encoding using an algorithm such as the one in the XML specification. If neither a character stream nor a byte stream is available, the parser will attempt to open a URI connection to the resource identified by the system identifier.

I've reproduced this - if given any encoding value with the byte[], then that byte array is read with that encoding and overrides the one declared in the document. Also, the Mock MVC response's encoding is ISO-8859-1 by default anyway, so you'd have to go response.setCharacterEncoding(null) to override that. This, is the servlet spec.

Now, because this testing infrastructure's main goal is to test HTTP endpoints that will be consumed by HTTP clients, I've tested some, including curl and Firefox.

Whenever the response encoding doesn't match the actual body encoding, wrong characters are displayed every time. With or without encoding declaration in the XML body. So while I'd really like to get every spec right, it seems that trying to do so will involve a lot of API breakage and will mislead developers into thinking that they don't have an encoding problem when they do.

One more thing, we're relying on those HTTP headers for the encoding value in our HttpMessageConverters; so changing this would also create new unintended consequences...

As always, we're open to discussion!

[spring-projects-issues]

Tor-Einar Jarnbjo commented

Is there any reason why this issue is pending closure and won't be fixed?

[spring-projects-issues]

Tor-Einar Jarnbjo commented

Is there any reason why this issue is pending closure and won't be fixed?

[spring-projects-issues]

Brian Clozel commented

This issue is now pending closure for the reasons I listed in my last comment.
Enforcing the requested behavior would conflict with HTTP spec and/or browser implementation.

I agree that the XML parsing spec is quite clear about that, but I think that implementing this behavior would create problems - basically your tests being green while HTTP clients have encoding issues.

[spring-projects-issues]

Tor-Einar Jarnbjo commented

I can't see anything in your last comment, which justifies the current behaviour of the test framework.

Are you honestly trying to explain that it is ok for the Spring MVC test framework to not be able to test a service implemented with Spring MVC just because you in some other situation (mismatching character encodings) see a behaviour in the browsers, which deviates from the specification?

Just to repeat myself: The Spring MVC service produces an HTTP response with no explicit character set declaration in the application/xml content type header, declares UTF-8 in the document header and uses (as expected) UTF-8 to encode the XML document. There is nothing strange about this, the behaviour is spec conform, commonly used and would be understood by any sane XML consumer. The declaration of the character set in the header is even redundant, since the XML specification declares UTF-8 as the default character set and the RFC for mime type "application/xml" delegates the character set determination to the XML specification in this case.

Now, the Spring MVC test framework gets this response, sees no character encoding in the content type header and hence uses ISO-8859-1, which is simply and blatantly wrong, violates both the MIME type and the XML specifications and is a behaviour you would not find in any other XML parser.

I am also not sure at all why you are bringing in the Servlet specification here, which defines a programming framework from Oracle to serve HTTP requests. I am not able to follow any connections you have made between the Servlet spec and how an HTTP consumer should parse XML documents.

[spring-projects-issues]

Brian Clozel commented

Hi

I've taken some time to take a fresh start with this issue.
I'm sorry about all this back and forth and I agree that the behavior you're describing is problematic -
the core issue here is to fix this and not break expectations (other specs, such as HTTP and Servlet) nor existing
behaviors.

I'm referring to the Servlet spec, because Spring MVC is tied to the Servlet spec through several interfaces, including MockHttpServletResponse, which implements javax.servlet.ServletResponse and is used here to get the response content and feed it to the parser.

I've decided not to work with default encodings depending on the mime types since it can be confusing. For example, you mentioned that XML specs define "us/ascii" as the default encoding for "text/xml" mime types, whereas the HTTP spec says that "text/*" mime types should use "ISO-8859-1" by default.

Now I've got something ready in a PR that should fix this:

• the MockHttpServletResponse has now a new method that says whether the character encoding has been set or not; the getCharacterEncoding method still returns "ISO-8859-1" by default, just as getPrintWriter will use that same encoding by default for writing content.
• XPathHelpers are now given byte arrays and an optional encoding, so it is now up to the parser to guess the encoding if nothing has been specified in the response headers

Could you take a look at it and tell me if there are other parts that need improvements?

Thanks!

[spring-projects-issues]

Tor-Einar Jarnbjo commented

Since the original HTTP specification (RFC 2616) was in conflict with the media type specification for "text/*" (RFC 2046), the original HTTP specification has been replaced with newer RFCs (RFC 7230 through 7235), in which the conflict has been resolved by removing "ISO-8859-1" as default character encoding for text/ responses.

The servlet specification and the javax.servlet.ServletResponse API only specify the producer side of an HTTP response. Spring's MockHttpServletResponse extend the functionality with a consumer side as well, but this is of course not covered by the Servlet specification. It might have been more obvious to implement the consumer side of the mock response as an extension of java.net.HttpURLConnection.

Passing the content as a byte array with an optional character encoding (as you've done in the commit) should solve the problem.

[spring-projects-issues]

Brian Clozel commented

Thanks for this report!
This is now in master and will be available shortly in the latest 4.2.0.BUILD-SNAPSHOT version.