diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fcf6dfac..fd6cf8725 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +### Version 13.18 + +* `Response.charset()` reads the `Content-Type` `charset` parameter wherever it appears, not only as + the first parameter, so `text/plain; format=flowed; charset=ISO-8859-1` decodes as ISO-8859-1 + instead of falling back to UTF-8. Parameters are scanned quote-aware, so a `;` or a fake + `charset=` inside a quoted parameter value stays data. Quoted charset values still work, and a + missing, illegal or unsupported charset still falls back to UTF-8 (#3626). + ### Version 13.17 * `Request` now accepts a `null` `Request.Body` and treats it as "no body", instead of throwing a diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index b09a3fd52..d745d0bc6 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -216,17 +216,9 @@ public Charset charset() { if (contentTypeHeaders != null) { for (String contentTypeHeader : contentTypeHeaders) { - String[] contentTypeParmeters = contentTypeHeader.split(";"); - if (contentTypeParmeters.length > 1) { - String[] charsetParts = contentTypeParmeters[1].split("="); - if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { - String charsetString = charsetParts[1].replaceAll("\"", ""); - try { - return Charset.forName(charsetString); - } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { - return Util.UTF_8; - } - } + Charset charset = charsetOf(contentTypeHeader); + if (charset != null) { + return charset; } } } @@ -234,6 +226,84 @@ public Charset charset() { return Util.UTF_8; } + /** + * Reads the charset parameter of a single Content-Type header value, in whatever position it + * appears. Parameters are scanned quote-aware, so a {@code ;} inside a quoted-string parameter + * value is data rather than a parameter separator (rfc9110 - 5.6.4). Returns null when the header + * carries no charset parameter, and UTF-8 when it carries an illegal or unsupported one. + */ + private static Charset charsetOf(String contentTypeHeader) { + int length = contentTypeHeader.length(); + int start = 0; + while (start <= length) { + int end = nextUnquoted(contentTypeHeader, start, length, ';'); + Charset charset = charsetParameterOf(contentTypeHeader, start, end); + if (charset != null) { + return charset; + } + if (end >= length) { + return null; + } + start = end + 1; + } + return null; + } + + /** Reads {@code name=value} between the given bounds, or null if the name is not charset. */ + private static Charset charsetParameterOf(String contentTypeHeader, int start, int end) { + int equals = nextUnquoted(contentTypeHeader, start, end, '='); + if (equals >= end) { + return null; + } + if (!"charset".equalsIgnoreCase(contentTypeHeader.substring(start, equals).trim())) { + return null; + } + try { + return Charset.forName(unquote(contentTypeHeader.substring(equals + 1, end).trim())); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + return Util.UTF_8; + } + } + + /** Index of the first {@code separator} between the given bounds outside a quoted-string. */ + private static int nextUnquoted(String value, int start, int end, char separator) { + boolean quoted = false; + for (int i = start; i < end; i++) { + char c = value.charAt(i); + if (quoted) { + if (c == '\\' && i + 1 < end) { + i++; // a backslash escapes the next character inside a quoted-string, e.g. \" or \\ + } else if (c == '"') { + quoted = false; + } + } else if (c == '"') { + quoted = true; + } else if (c == separator) { + return i; + } + } + return end; + } + + /** Removes the surrounding quotes of a quoted-string value and resolves its escapes. */ + private static String unquote(String value) { + if (value.isEmpty() || value.charAt(0) != '"') { + return value; + } + StringBuilder unquoted = new StringBuilder(value.length()); + for (int i = 1; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '"') { + return unquoted.toString(); // anything after the closing quote is not part of the value + } + if (c == '\\' && i + 1 < value.length()) { + c = value.charAt(++i); + } + unquoted.append(c); + } + return unquoted.toString(); // unterminated quoted-string, tolerate it + } + @Override public String toString() { StringBuilder builder = diff --git a/core/src/test/java/feign/ResponseTest.java b/core/src/test/java/feign/ResponseTest.java index e781bce66..9d32b0ed8 100644 --- a/core/src/test/java/feign/ResponseTest.java +++ b/core/src/test/java/feign/ResponseTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import feign.Request.HttpMethod; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -117,6 +118,62 @@ void charsetDefaultsToUtf8ForUnsupportedCharset() { assertThat(response.charset()).isEqualTo(Util.UTF_8); } + @Test + void charsetSupportsCharsetAsSecondParameter() { + // A charset may follow other media type parameters (RFC 9110 8.3.1). + Response response = responseWithContentType("text/plain;format=flowed;charset=ISO-8859-1"); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void charsetSupportsCharsetAsLaterParameter() { + Response response = + responseWithContentType("text/plain; format=flowed; boundary=xyz; charset=ISO-8859-1"); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void charsetParameterNameIsCaseInsensitiveWhenNotFirstParameter() { + Response response = responseWithContentType("text/plain; Format=Flowed; Charset=ISO-8859-1"); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void charsetSupportsQuotedCharsetValueWhenNotFirstParameter() { + Response response = responseWithContentType("text/plain;format=flowed;charset=\"ISO-8859-1\""); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void quotedParameterValueContainingCharsetTextDoesNotAffectCharset() { + // RFC 9110 5.6.6 allows a quoted-string parameter value to contain ";", so the fake + // "charset=UTF-16" below is data and must not be read as the charset parameter + Response response = + responseWithContentType("text/plain; note=\"x;charset=UTF-16\"; charset=ISO-8859-1"); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void escapedQuoteInQuotedParameterValueDoesNotHideCharsetParameters() { + // the \" below is escaped, so the quoted value runs to the next unescaped quote and keeps the + // ";charset=UTF-16" text as data + Response response = + responseWithContentType("text/plain; note=\"a\\\";charset=UTF-16\"; charset=ISO-8859-1"); + assertThat(response.charset()).isEqualTo(StandardCharsets.ISO_8859_1); + } + + @Test + void charsetDefaultsToUtf8WhenNoCharsetParameterIsPresent() { + Response response = responseWithContentType("text/plain; format=flowed"); + assertThat(response.charset()).isEqualTo(Util.UTF_8); + } + + @Test + void charsetDefaultsToUtf8ForUnsupportedCharsetInLaterParameter() { + Response response = responseWithContentType("text/plain; format=flowed; charset=made-up-99"); + assertThat(response.charset()).isEqualTo(Util.UTF_8); + } + @Test void headerValuesWithSameNameOnlyVaryingInCaseAreMerged() { Map> headersMap = new LinkedHashMap<>(); @@ -197,4 +254,15 @@ void protocolVersionDefaultsToHttp1_1() { assertThat(response.protocolVersion()).isEqualTo(Request.ProtocolVersion.HTTP_1_1); assertThat(response.toString()).startsWith("HTTP/1.1 200"); } + + private Response responseWithContentType(String contentType) { + Map> headersMap = new LinkedHashMap<>(); + headersMap.put("Content-Type", Collections.singletonList(contentType)); + return Response.builder() + .status(200) + .headers(headersMap) + .request(Request.create(HttpMethod.GET, "/api", Collections.emptyMap(), null, Util.UTF_8)) + .body(new byte[0]) + .build(); + } } diff --git a/core/src/test/java/feign/codec/DefaultDecoderTest.java b/core/src/test/java/feign/codec/DefaultDecoderTest.java index d2bc51568..48fcae961 100644 --- a/core/src/test/java/feign/codec/DefaultDecoderTest.java +++ b/core/src/test/java/feign/codec/DefaultDecoderTest.java @@ -72,6 +72,30 @@ void decodesToStringHonouringContentTypeCharset() throws Exception { assertThat(decodedObject).isEqualTo(content); } + @Test + void decodesToStringHonouringCharsetAfterOtherContentTypeParameters() throws Exception { + // "café" as ISO-8859-1 bytes; decoding them as UTF-8 yields a replacement character instead + byte[] body = new byte[] {'c', 'a', 'f', (byte) 0xE9}; + Response response = + responseWith("text/plain;format=flowed;charset=ISO-8859-1", body, body.length); + assertThat(decoder.decode(response, String.class)).isEqualTo("café"); + } + + @Test + void decodesToStringHonouringQuotedCharsetAfterOtherContentTypeParameters() throws Exception { + byte[] body = new byte[] {'c', 'a', 'f', (byte) 0xE9}; + Response response = + responseWith("text/plain; format=flowed; charset=\"ISO-8859-1\"", body, body.length); + assertThat(decoder.decode(response, String.class)).isEqualTo("café"); + } + + @Test + void decodesToStringUsingUtf8WhenCharsetParameterIsMissing() throws Exception { + byte[] body = "café".getBytes(UTF_8); + Response response = responseWith("text/plain; format=flowed", body, body.length); + assertThat(decoder.decode(response, String.class)).isEqualTo("café"); + } + @Test void decodesToByteArray() throws Exception { Response response = knownResponse(); @@ -106,6 +130,18 @@ private Response knownResponse() { .build(); } + private Response responseWith(String contentType, byte[] content, int length) { + Map> headers = new HashMap<>(); + headers.put("Content-Type", Collections.singleton(contentType)); + return Response.builder() + .status(200) + .reason("OK") + .headers(headers) + .request(Request.create(HttpMethod.GET, "/api", Collections.emptyMap(), null, UTF_8)) + .body(new ByteArrayInputStream(content), length) + .build(); + } + private Response nullBodyResponse() { return Response.builder() .status(200)