Normalize URI paths in RestClient

Signed-off-by: Thomas Farr <tsfarr@amazon.com>
opensearch-project · Jun 18, 2024 · 2b8571b · 2b8571b
1 parent 3a0c0c0
commit 2b8571b
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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 {