Reindex from Remote allow date math (#40303)

Previously, reindexing from remote using date math in the source index
name did not work if the math contained / or ,. A workaround was to
then URL escape the index name in the request.

With this change, we now support any index name in the remote request
that the remote source supports, doing the URL escape when sending the
request.

Related to #23533

modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java

@@ -38,6 +38,10 @@ import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
 
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.util.Arrays;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.unit.TimeValue.timeValueMillis;
 
@@ -54,8 +58,8 @@ final class RemoteRequestBuilders {
     static Request initialSearch(SearchRequest searchRequest, BytesReference query, Version remoteVersion) {
         // It is nasty to build paths with StringBuilder but we'll be careful....
         StringBuilder path = new StringBuilder("/");
-        addIndexesOrTypes(path, "Index", searchRequest.indices());
+        addIndices(path, searchRequest.indices());
-        addIndexesOrTypes(path, "Type", searchRequest.types());
+        addTypes(path, searchRequest.types());
         path.append("_search");
         Request request = new Request("POST", path.toString());
 
@@ -158,14 +162,34 @@ final class RemoteRequestBuilders {
         return request;
     }
 
-    private static void addIndexesOrTypes(StringBuilder path, String name, String[] indicesOrTypes) {
+    private static void addIndices(StringBuilder path, String[] indices) {
-        if (indicesOrTypes == null || indicesOrTypes.length == 0) {
+        if (indices == null || indices.length == 0) {
             return;
         }
-        for (String indexOrType : indicesOrTypes) {
+
-            checkIndexOrType(name, indexOrType);
+        path.append(Arrays.stream(indices).map(RemoteRequestBuilders::encodeIndex).collect(Collectors.joining(","))).append('/');
     }
-        path.append(Strings.arrayToCommaDelimitedString(indicesOrTypes)).append('/');
+
+    private static String encodeIndex(String s) {
+        if (s.contains("%")) { // already encoded, pass-through to allow this in mixed version clusters
+            checkIndexOrType("Index", s);
+            return s;
+        }
+        try {
+            return URLEncoder.encode(s, "utf-8");
+        } catch (UnsupportedEncodingException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static void addTypes(StringBuilder path, String[] types) {
+        if (types == null || types.length == 0) {
+            return;
+        }
+        for (String indexOrType : types) {
+            checkIndexOrType("Type", indexOrType);
+        }
+        path.append(Strings.arrayToCommaDelimitedString(types)).append('/');
     }
 
     private static void checkIndexOrType(String name, String indexOrType) {

modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java

@@ -68,19 +68,26 @@ public class RemoteRequestBuildersTests extends ESTestCase {
         searchRequest.indices("a", "b");
         searchRequest.types("c", "d");
         assertEquals("/a,b/c,d/_search", initialSearch(searchRequest, query, remoteVersion).getEndpoint());
-
         searchRequest.indices("cat,");
-        expectBadStartRequest(searchRequest, "Index", ",", "cat,");
+        assertEquals("/cat%2C/c,d/_search", initialSearch(searchRequest, query, remoteVersion).getEndpoint());
-        searchRequest.indices("cat,", "dog");
-        expectBadStartRequest(searchRequest, "Index", ",", "cat,");
-        searchRequest.indices("dog", "cat,");
-        expectBadStartRequest(searchRequest, "Index", ",", "cat,");
         searchRequest.indices("cat/");
-        expectBadStartRequest(searchRequest, "Index", "/", "cat/");
+        assertEquals("/cat%2F/c,d/_search", initialSearch(searchRequest, query, remoteVersion).getEndpoint());
         searchRequest.indices("cat/", "dog");
-        expectBadStartRequest(searchRequest, "Index", "/", "cat/");
+        assertEquals("/cat%2F,dog/c,d/_search", initialSearch(searchRequest, query, remoteVersion).getEndpoint());
-        searchRequest.indices("dog", "cat/");
+        // test a specific date math + all characters that need escaping.
-        expectBadStartRequest(searchRequest, "Index", "/", "cat/");
+        searchRequest.indices("<cat{now/d}>", "<>/{}|+:,");
+        assertEquals("/%3Ccat%7Bnow%2Fd%7D%3E,%3C%3E%2F%7B%7D%7C%2B%3A%2C/c,d/_search",
+            initialSearch(searchRequest, query, remoteVersion).getEndpoint());
+
+        // pass-through if already escaped.
+        searchRequest.indices("%2f", "%3a");
+        assertEquals("/%2f,%3a/c,d/_search", initialSearch(searchRequest, query, remoteVersion).getEndpoint());
+
+        // do not allow , and / if already escaped.
+        searchRequest.indices("%2fcat,");
+        expectBadStartRequest(searchRequest, "Index", ",", "%2fcat,");
+        searchRequest.indices("%3ccat/");
+        expectBadStartRequest(searchRequest, "Index", "/", "%3ccat/");
 
         searchRequest.indices("ok");
         searchRequest.types("cat,");