From 6f2fddc5f6085ae018dd5990eebc3139f2547d62 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 8 Jan 2018 09:44:07 +0100 Subject: [PATCH] Watcher: Fix encoding of UTF8 data in HttpClient (elastic/x-pack-elasticsearch#3398) The HttpClient uses an Apache HTTP client class named StringEntity to encode a HTTP request body. This one however assumes ISO-8859-1 as its charset when encoding the string based body to bytes. This commit switches to a byte array based body, then sets the content type header and falls back to the old text/plain content type if no content type header is specified. relates elastic/x-pack-elasticsearch#3397 Original commit: elastic/x-pack-elasticsearch@d5a6e7f0c7f65e507564b9aaab91be97e0beb77c --- .../xpack/watcher/common/http/HttpClient.java | 18 +++++++++---- .../watcher/common/http/HttpClientTests.java | 25 ++++++++++++++++--- .../test/integration/SlackServiceTests.java | 2 +- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java index 43b7bd38b64..b5b44b8b84d 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java @@ -23,7 +23,8 @@ import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.conn.ssl.DefaultHostnameVerifier; import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.entity.StringEntity; +import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.entity.ContentType; import org.apache.http.impl.auth.BasicScheme; import org.apache.http.impl.client.BasicAuthCache; import org.apache.http.impl.client.BasicCredentialsProvider; @@ -38,10 +39,10 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.xpack.watcher.common.http.auth.ApplicableHttpAuth; -import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry; import org.elasticsearch.xpack.common.socket.SocketAccess; import org.elasticsearch.xpack.ssl.SSLService; +import org.elasticsearch.xpack.watcher.common.http.auth.ApplicableHttpAuth; +import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry; import javax.net.ssl.HostnameVerifier; import java.io.ByteArrayOutputStream; @@ -104,8 +105,15 @@ public class HttpClient extends AbstractComponent { internalRequest = new HttpHead(uri); } else { HttpMethodWithEntity methodWithEntity = new HttpMethodWithEntity(uri, request.method.name()); - if (request.body != null) { - methodWithEntity.setEntity(new StringEntity(request.body)); + if (request.hasBody()) { + ByteArrayEntity entity = new ByteArrayEntity(request.body.getBytes(StandardCharsets.UTF_8)); + String contentType = request.headers().get(HttpHeaders.CONTENT_TYPE); + if (Strings.hasLength(contentType)) { + entity.setContentType(contentType); + } else { + entity.setContentType(ContentType.TEXT_PLAIN.toString()); + } + methodWithEntity.setEntity(entity); } internalRequest = methodWithEntity; } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java index 942fb7a52e2..f7c2fd789c4 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.watcher.common.http; import com.carrotsearch.randomizedtesting.generators.RandomStrings; +import org.apache.http.HttpHeaders; import org.apache.http.client.ClientProtocolException; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; @@ -14,6 +15,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.mocksocket.MockServerSocket; @@ -21,12 +23,12 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.http.MockResponse; import org.elasticsearch.test.http.MockWebServer; import org.elasticsearch.test.junit.annotations.Network; -import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry; -import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuth; -import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuthFactory; import org.elasticsearch.xpack.ssl.SSLService; import org.elasticsearch.xpack.ssl.TestsSSLService; import org.elasticsearch.xpack.ssl.VerificationMode; +import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry; +import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuth; +import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuthFactory; import org.junit.After; import org.junit.Before; @@ -494,4 +496,21 @@ public class HttpClientTests extends ESTestCase { assertThat(response.body(), is(nullValue())); assertThat(webServer.requests(), hasSize(1)); } + + public void testThatBodyWithUTF8Content() throws Exception { + String body = "title あいうえお"; + webServer.enqueue(new MockResponse().setResponseCode(200).setBody(body)); + + HttpRequest request = HttpRequest.builder("localhost", webServer.getPort()) + .path("/") + .setHeader(HttpHeaders.CONTENT_TYPE, XContentType.JSON.mediaType()) + .body(body) + .build(); + HttpResponse response = httpClient.execute(request); + assertThat(response.body().utf8ToString(), is(body)); + + assertThat(webServer.requests(), hasSize(1)); + assertThat(webServer.requests().get(0).getHeader(HttpHeaders.CONTENT_TYPE), is(XContentType.JSON.mediaType())); + assertThat(webServer.requests().get(0).getBody(), is(body)); + } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/SlackServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/SlackServiceTests.java index 806b1d640ea..8f88767e76e 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/SlackServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/SlackServiceTests.java @@ -56,7 +56,7 @@ public class SlackServiceTests extends AbstractWatcherIntegrationTestCase { SlackService service = getInstanceFromMaster(SlackService.class); Attachment[] attachments = new Attachment[] { new Attachment("fallback", randomFrom("good", "warning", "danger"), "pretext `code` *bold*", "author_name", null, null, - "title", null, "_text `code` *bold*", null, null, null, new String[] { "text", "pretext" }) + "title あいうえお", null, "_text `code` *bold*", null, null, null, new String[] { "text", "pretext" }) }; SlackMessage message = new SlackMessage( "SlackServiceTests",