Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize URI paths in RestClient #14423

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −16
Draft

Normalize URI paths in RestClient #14423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 13 additions & 13 deletions client/rest/src/main/java/org/opensearch/client/RestClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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
Expand Down
26 changes: 23 additions & 3 deletions client/rest/src/test/java/org/opensearch/client/RestClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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 {
Expand Down
Loading