From e66d0bbe188548b8dbe4fa596ac13668cbdae5a0 Mon Sep 17 00:00:00 2001 From: "andrey.stolyarchuk" Date: Fri, 2 Sep 2016 20:57:28 +0300 Subject: [PATCH 1/4] Support request content-type with charset --- .../java/feign/client/AbstractClientTest.java | 17 +++++++++++++++++ .../feign/httpclient/ApacheHttpClient.java | 18 ++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java index a2421a3461..d478942149 100644 --- a/core/src/test/java/feign/client/AbstractClientTest.java +++ b/core/src/test/java/feign/client/AbstractClientTest.java @@ -15,6 +15,7 @@ import feign.Param; import feign.RequestLine; import feign.Response; +import feign.Util; import feign.assertj.MockWebServerAssertions; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; @@ -218,6 +219,18 @@ public void testResponseLength() throws Exception { assertEquals(expected, actual); } + @Test + public void testContentTypeWithCharset() throws Exception { + server.enqueue(new MockResponse() + .setBody("AAAAAAAA")); + TestInterface api = newBuilder() + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + Response response = api.postContentTypeWithCharset("foo"); + // Response length should not be null + assertEquals("AAAAAAAA", Util.toString(response.body().asReader())); + } + public interface TestInterface { @RequestLine("POST /?foo=bar&foo=baz&qux=") @@ -241,6 +254,10 @@ public interface TestInterface { @RequestLine("PUT") String noPutBody(); + + @RequestLine("POST /?foo=bar&foo=baz&qux=") + @Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain;charset=utf-8"}) + Response postContentTypeWithCharset(String body); } } diff --git a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java index 8cfebf0a63..12ea22ba1f 100644 --- a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java +++ b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java @@ -153,13 +153,19 @@ HttpUriRequest toHttpUriRequest(Request request, Request.Options options) throws private ContentType getContentType(Request request) { ContentType contentType = ContentType.DEFAULT_TEXT; for (Map.Entry> entry : request.headers().entrySet()) - if (entry.getKey().equalsIgnoreCase("Content-Type")) { - Collection values = entry.getValue(); - if (values != null && !values.isEmpty()) { - contentType = ContentType.create(entry.getValue().iterator().next(), request.charset()); - break; + if (entry.getKey().equalsIgnoreCase("Content-Type")) { + Collection values = entry.getValue(); + if (values != null && !values.isEmpty()) { + String mimeType = values.iterator().next(); + try { + contentType = ContentType.create(mimeType, request.charset()); + } catch (IllegalArgumentException e) { + // MIME type may not contain reserved characters, so try to parse content type + contentType = ContentType.parse(mimeType); + } + break; + } } - } return contentType; } From e4cd7a34ee6143b6b50adf85e7273119a8120ccc Mon Sep 17 00:00:00 2001 From: "andrey.stolyarchuk" Date: Mon, 5 Sep 2016 22:02:14 +0300 Subject: [PATCH 2/4] Support request content-type with charset. Improve unit test --- .../java/feign/client/AbstractClientTest.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java index d478942149..704d88b425 100644 --- a/core/src/test/java/feign/client/AbstractClientTest.java +++ b/core/src/test/java/feign/client/AbstractClientTest.java @@ -226,7 +226,19 @@ public void testContentTypeWithCharset() throws Exception { TestInterface api = newBuilder() .target(TestInterface.class, "http://localhost:" + server.getPort()); - Response response = api.postContentTypeWithCharset("foo"); + Response response = api.postWithContentType("foo", "text/plain;charset=utf-8"); + // Response length should not be null + assertEquals("AAAAAAAA", Util.toString(response.body().asReader())); + } + + @Test + public void testContentTypeWithoutCharset() throws Exception { + server.enqueue(new MockResponse() + .setBody("AAAAAAAA")); + TestInterface api = newBuilder() + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + Response response = api.postWithContentType("foo", "text/plain"); // Response length should not be null assertEquals("AAAAAAAA", Util.toString(response.body().asReader())); } @@ -256,8 +268,8 @@ public interface TestInterface { String noPutBody(); @RequestLine("POST /?foo=bar&foo=baz&qux=") - @Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain;charset=utf-8"}) - Response postContentTypeWithCharset(String body); + @Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: {contentType}"}) + Response postWithContentType(String body, @Param("contentType") String contentType); } } From 855d974fcd2f6a653e4387223306f4db36ebdc22 Mon Sep 17 00:00:00 2001 From: "andrey.stolyarchuk" Date: Wed, 7 Sep 2016 18:33:27 +0300 Subject: [PATCH 3/4] Support request content-type with charset. Avoid catching --- .../src/main/java/feign/httpclient/ApacheHttpClient.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java index 12ea22ba1f..c06068ae47 100644 --- a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java +++ b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java @@ -157,11 +157,9 @@ private ContentType getContentType(Request request) { Collection values = entry.getValue(); if (values != null && !values.isEmpty()) { String mimeType = values.iterator().next(); - try { - contentType = ContentType.create(mimeType, request.charset()); - } catch (IllegalArgumentException e) { - // MIME type may not contain reserved characters, so try to parse content type - contentType = ContentType.parse(mimeType); + contentType = ContentType.parse(mimeType); + if (contentType.getCharset() == null) { + contentType = ContentType.create(contentType.getMimeType(), request.charset()); } break; } From f8da3a7ca55bb9e4ae1aaee5f04f9628843f9926 Mon Sep 17 00:00:00 2001 From: "andrey.stolyarchuk" Date: Thu, 8 Sep 2016 08:16:29 +0300 Subject: [PATCH 4/4] Support request content-type with charset. Request charset should not be considered in response handling --- .../src/main/java/feign/httpclient/ApacheHttpClient.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java index c06068ae47..18fbc7512e 100644 --- a/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java +++ b/httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java @@ -156,11 +156,7 @@ private ContentType getContentType(Request request) { if (entry.getKey().equalsIgnoreCase("Content-Type")) { Collection values = entry.getValue(); if (values != null && !values.isEmpty()) { - String mimeType = values.iterator().next(); - contentType = ContentType.parse(mimeType); - if (contentType.getCharset() == null) { - contentType = ContentType.create(contentType.getMimeType(), request.charset()); - } + contentType = ContentType.parse(values.iterator().next()); break; } }