From 8a6f900f2c00755c4920c18b7b2e5f2f3d5207f8 Mon Sep 17 00:00:00 2001
From: Thomas Farr <tsfarr@amazon.com>
Date: Tue, 18 Jun 2024 23:38:17 +1200
Subject: [PATCH] Normalize URI paths in RestClient

Signed-off-by: Thomas Farr <tsfarr@amazon.com>
---
 CHANGELOG.md                                  |  1 +
 .../org/opensearch/client/RestClient.java     | 26 +++++++++----------
 .../opensearch/client/RestClientTests.java    | 26 ++++++++++++++++---
 3 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d6943e538fdcc..15076d61d15cf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fixed array field name omission in flat_object function for nested JSON ([#13620](https://github.com/opensearch-project/OpenSearch/pull/13620))
 - Fix range aggregation optimization ignoring top level queries ([#15194](https://github.com/opensearch-project/OpenSearch/pull/15194))
 - Fix incorrect parameter names in MinHash token filter configuration handling ([#15233](https://github.com/opensearch-project/OpenSearch/pull/15233))
+- Fixed RestClient URI path handling to ensure a leading forward-slash is always present ([#14423](https://github.com/opensearch-project/OpenSearch/pull/14423))
 
 ### Security
 
diff --git a/client/rest/src/main/java/org/opensearch/client/RestClient.java b/client/rest/src/main/java/org/opensearch/client/RestClient.java
index 5c87e3fda5701..ac4582d272f5e 100644
--- a/client/rest/src/main/java/org/opensearch/client/RestClient.java
+++ b/client/rest/src/main/java/org/opensearch/client/RestClient.java
@@ -783,20 +783,10 @@ private HttpUriRequestBase addRequestBody(HttpUriRequestBase httpRequest, HttpEn
     static URI buildUri(String pathPrefix, String path, Map<String, String> params) {
         Objects.requireNonNull(path, "path must not be null");
         try {
-            String fullPath;
-            if (pathPrefix != null && pathPrefix.isEmpty() == false) {
-                if (pathPrefix.endsWith("/") && path.startsWith("/")) {
-                    fullPath = pathPrefix.substring(0, pathPrefix.length() - 1) + path;
-                } else if (pathPrefix.endsWith("/") || path.startsWith("/")) {
-                    fullPath = pathPrefix + path;
-                } else {
-                    fullPath = pathPrefix + "/" + path;
-                }
-            } else {
-                fullPath = path;
-            }
+            URIBuilder uriBuilder = new URIBuilder();
+            uriBuilder.appendPath(pathPrefix != null ? trimSlashes(pathPrefix) : "");
+            uriBuilder.appendPath(trimSlashes(path));
 
-            URIBuilder uriBuilder = new URIBuilder(fullPath);
             for (Map.Entry<String, String> param : params.entrySet()) {
                 uriBuilder.addParameter(param.getKey(), param.getValue());
             }
@@ -811,6 +801,16 @@ static URI buildUri(String pathPrefix, String path, Map<String, String> params)
         }
     }
 
+    private static String trimSlashes(String str) {
+        int start = 0;
+        int end = str.length();
+        while (start < end && str.charAt(start) == '/')
+            ++start;
+        while (end > start && str.charAt(end - 1) == '/')
+            --end;
+        return str.substring(start, end);
+    }
+
     /**
      * Listener used in any async call to wrap the provided user listener (or SyncResponseListener in sync calls).
      * Allows to track potential failures coming from the different retry attempts and returning to the original listener
diff --git a/client/rest/src/test/java/org/opensearch/client/RestClientTests.java b/client/rest/src/test/java/org/opensearch/client/RestClientTests.java
index f4f1c57cdd588..64ba2d2f871ae 100644
--- a/client/rest/src/test/java/org/opensearch/client/RestClientTests.java
+++ b/client/rest/src/test/java/org/opensearch/client/RestClientTests.java
@@ -132,7 +132,7 @@ public void onFailure(Exception exception) {
         }
     }
 
-    public void testBuildUriLeavesPathUntouched() {
+    public void testBuildUriCorrectlyNormalizesPath() {
         final Map<String, String> emptyMap = Collections.emptyMap();
         {
             URI uri = RestClient.buildUri("/foo$bar", "/index/type/id", emptyMap);
@@ -148,11 +148,11 @@ public void testBuildUriLeavesPathUntouched() {
         }
         {
             URI uri = RestClient.buildUri(null, "*", emptyMap);
-            assertEquals("*", uri.getPath());
+            assertEquals("/*", uri.getPath());
         }
         {
             URI uri = RestClient.buildUri("", "*", emptyMap);
-            assertEquals("*", uri.getPath());
+            assertEquals("/*", uri.getPath());
         }
         {
             URI uri = RestClient.buildUri(null, "/*", emptyMap);
@@ -167,6 +167,26 @@ public void testBuildUriLeavesPathUntouched() {
             assertEquals("/index/type/id", uri.getPath());
             assertEquals("foo$bar=x/y/z", uri.getQuery());
         }
+        {
+            URI uri = RestClient.buildUri("/foo/", "/bar/", emptyMap);
+            assertEquals("/foo/bar", uri.getPath());
+        }
+        {
+            URI uri = RestClient.buildUri("", "", emptyMap);
+            assertEquals("", uri.getPath());
+        }
+        {
+            URI uri = RestClient.buildUri("////", "/foobar", emptyMap);
+            assertEquals("/foobar", uri.getPath());
+        }
+        {
+            URI uri = RestClient.buildUri("///", "///", emptyMap);
+            assertEquals("", uri.getPath());
+        }
+        {
+            URI uri = RestClient.buildUri("/foo/", "/bar//baz/", emptyMap);
+            assertEquals("/foo/bar//baz", uri.getPath());
+        }
     }
 
     public void testSetNodesWrongArguments() throws IOException {