diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index f46248c2f5b..9f8fcbb7ab2 100755 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -74,6 +74,8 @@ import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.charset.Charset; import java.util.Collections; import java.util.HashMap; @@ -580,7 +582,16 @@ public final class Request { StringJoiner joiner = new StringJoiner("/", "/", ""); for (String part : parts) { if (Strings.hasLength(part)) { - joiner.add(part); + try { + //encode each part (e.g. index, type and id) separately before merging them into the path + //we prepend "/" to the path part to make this pate absolute, otherwise there can be issues with + //paths that start with `-` or contain `:` + URI uri = new URI(null, null, null, -1, "/" + part, null, null); + //manually encode any slash that each part may contain + joiner.add(uri.getRawPath().substring(1).replaceAll("/", "%2F")); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Path part [" + part + "] couldn't be encoded", e); + } } } return joiner.toString(); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 7d3f67bfa83..559dded4f4d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -26,6 +26,7 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.admin.indices.get.GetIndexRequest; import org.elasticsearch.action.bulk.BulkItemResponse; import org.elasticsearch.action.bulk.BulkProcessor; import org.elasticsearch.action.bulk.BulkRequest; @@ -52,6 +53,9 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.format.DateTimeFormat; import java.io.IOException; import java.util.Collections; @@ -648,7 +652,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase { validateBulkResponses(nbItems, errors, bulkResponse, bulkRequest); } - public void testBulkProcessorIntegration() throws IOException, InterruptedException { + public void testBulkProcessorIntegration() throws IOException { int nbItems = randomIntBetween(10, 100); boolean[] errors = new boolean[nbItems]; @@ -762,4 +766,69 @@ public class CrudIT extends ESRestHighLevelClientTestCase { } } } + + public void testUrlEncode() throws IOException { + String indexPattern = ""; + String expectedIndex = "logstash-" + + DateTimeFormat.forPattern("YYYY.MM.dd").print(new DateTime(DateTimeZone.UTC).monthOfYear().roundFloorCopy()); + { + IndexRequest indexRequest = new IndexRequest(indexPattern, "type", "id#1"); + indexRequest.source("field", "value"); + IndexResponse indexResponse = highLevelClient().index(indexRequest); + assertEquals(expectedIndex, indexResponse.getIndex()); + assertEquals("type", indexResponse.getType()); + assertEquals("id#1", indexResponse.getId()); + } + { + GetRequest getRequest = new GetRequest(indexPattern, "type", "id#1"); + GetResponse getResponse = highLevelClient().get(getRequest); + assertTrue(getResponse.isExists()); + assertEquals(expectedIndex, getResponse.getIndex()); + assertEquals("type", getResponse.getType()); + assertEquals("id#1", getResponse.getId()); + } + + String docId = "this/is/the/id/中文"; + { + IndexRequest indexRequest = new IndexRequest("index", "type", docId); + indexRequest.source("field", "value"); + IndexResponse indexResponse = highLevelClient().index(indexRequest); + assertEquals("index", indexResponse.getIndex()); + assertEquals("type", indexResponse.getType()); + assertEquals(docId, indexResponse.getId()); + } + { + GetRequest getRequest = new GetRequest("index", "type", docId); + GetResponse getResponse = highLevelClient().get(getRequest); + assertTrue(getResponse.isExists()); + assertEquals("index", getResponse.getIndex()); + assertEquals("type", getResponse.getType()); + assertEquals(docId, getResponse.getId()); + } + + assertTrue(highLevelClient().indices().exists(new GetIndexRequest().indices(indexPattern, "index"))); + } + + public void testParamsEncode() throws IOException { + //parameters are encoded by the low-level client but let's test that everything works the same when we use the high-level one + String routing = "routing/中文value#1?"; + { + IndexRequest indexRequest = new IndexRequest("index", "type", "id"); + indexRequest.source("field", "value"); + indexRequest.routing(routing); + IndexResponse indexResponse = highLevelClient().index(indexRequest); + assertEquals("index", indexResponse.getIndex()); + assertEquals("type", indexResponse.getType()); + assertEquals("id", indexResponse.getId()); + } + { + GetRequest getRequest = new GetRequest("index", "type", "id").routing(routing); + GetResponse getResponse = highLevelClient().get(getRequest); + assertTrue(getResponse.isExists()); + assertEquals("index", getResponse.getIndex()); + assertEquals("type", getResponse.getType()); + assertEquals("id", getResponse.getId()); + assertEquals(routing, getResponse.getField("_routing").getValue()); + } + } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 400a47ca47a..970eac83726 100755 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -1188,6 +1188,22 @@ public class RequestTests extends ESTestCase { assertEquals("/a/_create", Request.buildEndpoint("a", null, null, "_create")); } + public void testBuildEndPointEncodeParts() { + assertEquals("/-%23index1,index%232/type/id", Request.buildEndpoint("-#index1,index#2", "type", "id")); + assertEquals("/index/type%232/id", Request.buildEndpoint("index", "type#2", "id")); + assertEquals("/index/type/this%2Fis%2Fthe%2Fid", Request.buildEndpoint("index", "type", "this/is/the/id")); + assertEquals("/index/type/this%7Cis%7Cthe%7Cid", Request.buildEndpoint("index", "type", "this|is|the|id")); + assertEquals("/index/type/id%231", Request.buildEndpoint("index", "type", "id#1")); + assertEquals("/%3Clogstash-%7Bnow%2FM%7D%3E/_search", Request.buildEndpoint("", "_search")); + assertEquals("/中文", Request.buildEndpoint("中文")); + assertEquals("/foo%20bar", Request.buildEndpoint("foo bar")); + assertEquals("/foo+bar", Request.buildEndpoint("foo+bar")); + assertEquals("/foo%2Fbar", Request.buildEndpoint("foo/bar")); + assertEquals("/foo%5Ebar", Request.buildEndpoint("foo^bar")); + assertEquals("/cluster1:index1,index2/_search", Request.buildEndpoint("cluster1:index1,index2", "_search")); + assertEquals("/*", Request.buildEndpoint("*")); + } + public void testEndpoint() { assertEquals("/index/type/id", Request.endpoint("index", "type", "id")); assertEquals("/index/type/id/_endpoint", Request.endpoint("index", "type", "id", "_endpoint")); diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java index 6d4e3ba4bc8..fafe745d934 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java @@ -74,7 +74,7 @@ public class RestClientSingleHostIntegTests extends RestClientTestCase { @BeforeClass public static void startHttpServer() throws Exception { - pathPrefix = randomBoolean() ? "/testPathPrefix/" + randomAsciiOfLengthBetween(1, 5) : ""; + pathPrefix = randomBoolean() ? "/testPathPrefix/" + randomAsciiAlphanumOfLengthBetween(1, 5) : ""; httpServer = createHttpServer(); defaultHeaders = RestClientTestUtil.randomHeaders(getRandom(), "Header-default"); restClient = createRestClient(false, true); @@ -101,6 +101,7 @@ public class RestClientSingleHostIntegTests extends RestClientTestCase { @Override public void handle(HttpExchange httpExchange) throws IOException { + //copy request body to response body so we can verify it was sent StringBuilder body = new StringBuilder(); try (InputStreamReader reader = new InputStreamReader(httpExchange.getRequestBody(), Consts.UTF_8)) { char[] buffer = new char[256]; @@ -109,6 +110,7 @@ public class RestClientSingleHostIntegTests extends RestClientTestCase { body.append(buffer, 0, read); } } + //copy request headers to response headers so we can verify they were sent Headers requestHeaders = httpExchange.getRequestHeaders(); Headers responseHeaders = httpExchange.getResponseHeaders(); for (Map.Entry> header : requestHeaders.entrySet()) { @@ -214,6 +216,41 @@ public class RestClientSingleHostIntegTests extends RestClientTestCase { bodyTest("GET"); } + public void testEncodeParams() throws IOException { + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "this/is/the/routing")); + assertEquals(pathPrefix + "/200?routing=this%2Fis%2Fthe%2Frouting", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "this|is|the|routing")); + assertEquals(pathPrefix + "/200?routing=this%7Cis%7Cthe%7Crouting", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "routing#1")); + assertEquals(pathPrefix + "/200?routing=routing%231", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "中文")); + assertEquals(pathPrefix + "/200?routing=%E4%B8%AD%E6%96%87", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "foo bar")); + assertEquals(pathPrefix + "/200?routing=foo+bar", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "foo+bar")); + assertEquals(pathPrefix + "/200?routing=foo%2Bbar", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "foo/bar")); + assertEquals(pathPrefix + "/200?routing=foo%2Fbar", response.getRequestLine().getUri()); + } + { + Response response = restClient.performRequest("PUT", "/200", Collections.singletonMap("routing", "foo^bar")); + assertEquals(pathPrefix + "/200?routing=foo%5Ebar", response.getRequestLine().getUri()); + } + } + /** * Verify that credentials are sent on the first request with preemptive auth enabled (default when provided with credentials). */ diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 01fd3bad0f3..f5e834aa90c 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -19,7 +19,6 @@ package org.elasticsearch.test.rest.yaml; import com.carrotsearch.randomizedtesting.RandomizedTest; - import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; @@ -85,9 +84,9 @@ public class ClientYamlTestClient { Map pathParts = new HashMap<>(); Map queryStringParams = new HashMap<>(); - Set apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey) + Set apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(Entry::getValue).map(Entry::getKey) .collect(Collectors.toSet()); - Set apiRequiredParameters = restApi.getParams().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey) + Set apiRequiredParameters = restApi.getParams().entrySet().stream().filter(Entry::getValue).map(Entry::getKey) .collect(Collectors.toSet()); for (Map.Entry entry : params.entrySet()) { @@ -151,7 +150,7 @@ public class ClientYamlTestClient { for (String pathPart : restPath.getPathParts()) { try { finalPath.append('/'); - // We append "/" to the path part to handle parts that start with - or other invalid characters + // We prepend "/" to the path part to handle parts that start with - or other invalid characters URI uri = new URI(null, null, null, -1, "/" + pathPart, null, null); //manually escape any slash that each part may contain finalPath.append(uri.getRawPath().substring(1).replaceAll("/", "%2F"));