REST high-level client: encode path parts (#28663)

The REST high-level client supports now encoding of path parts, so that for instance documents with valid ids, but containing characters that need to be encoded as part of urls (`#` etc.), are properly supported. We also make sure that each path part can contain `/` by encoding them properly too.

Closes #28625

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();

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 = "<logstash-{now/M}>";
+        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());
+        }
+    }
 }

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("<logstash-{now/M}>", "_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"));

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<String, List<String>> 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).
      */

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<String, String> pathParts = new HashMap<>();
         Map<String, String> queryStringParams = new HashMap<>();
 
-        Set<String> apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey)
+        Set<String> apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(Entry::getValue).map(Entry::getKey)
                 .collect(Collectors.toSet());
-        Set<String> apiRequiredParameters = restApi.getParams().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey)
+        Set<String> apiRequiredParameters = restApi.getParams().entrySet().stream().filter(Entry::getValue).map(Entry::getKey)
                 .collect(Collectors.toSet());
 
         for (Map.Entry<String, String> 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"));