From 2b8571b28bbcad1672ef7501e43713fcb2e2e54b 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     | 24 ++++++++---------
 .../opensearch/client/RestClientTests.java    | 26 ++++++++++++++++---
 3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6654b478c74f8..1394d30c75269 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 - Fix handling of Short and Byte data types in ScriptProcessor ingest pipeline ([#14379](https://github.com/opensearch-project/OpenSearch/issues/14379))
+- 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 15905add76c4f..9527a5ba83cab 100644
--- a/client/rest/src/main/java/org/opensearch/client/RestClient.java
+++ b/client/rest/src/main/java/org/opensearch/client/RestClient.java
@@ -674,20 +674,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());
             }
@@ -702,6 +692,14 @@ 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 dd51da3a30d8c..d98ba07818cce 100644
--- a/client/rest/src/test/java/org/opensearch/client/RestClientTests.java
+++ b/client/rest/src/test/java/org/opensearch/client/RestClientTests.java
@@ -129,7 +129,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);
@@ -145,11 +145,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);
@@ -164,6 +164,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 {