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.

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

Enable setting client.path.prefix='/' #30119

Merged
merged 4 commits into from
Jul 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,10 @@ 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) {
if (path.startsWith("/")) {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,32 +143,33 @@ public RestClientBuilder setRequestConfigCallback(RequestConfigCallback requestC
* For example, if this is set to "/my/path", then any client request will become <code>"/my/path/" + endpoint</code>.
* <p>
* In essence, every request's {@code endpoint} is prefixed by this {@code pathPrefix}. The path prefix is useful for when
* Elasticsearch is behind a proxy that provides a base path; it is not intended for other purposes and it should not be supplied in
* other scenarios.
* Elasticsearch is behind a proxy that provides a base path or a proxy that requires all paths to start with '/';
* it is not intended for other purposes and it should not be supplied in other scenarios.
*
* @throws NullPointerException if {@code pathPrefix} is {@code null}.
* @throws IllegalArgumentException if {@code pathPrefix} is empty, only '/', or ends with more than one '/'.
* @throws IllegalArgumentException if {@code pathPrefix} is empty, or ends with more than one '/'.
*/
public RestClientBuilder setPathPrefix(String pathPrefix) {
Objects.requireNonNull(pathPrefix, "pathPrefix must not be null");
String cleanPathPrefix = pathPrefix;

if (pathPrefix.isEmpty()) {
throw new IllegalArgumentException("pathPrefix must not be empty");
}

String cleanPathPrefix = pathPrefix;
if (cleanPathPrefix.startsWith("/") == false) {
cleanPathPrefix = "/" + cleanPathPrefix;
}

// best effort to ensure that it looks like "/base/path" rather than "/base/path/"
if (cleanPathPrefix.endsWith("/")) {
if (cleanPathPrefix.endsWith("/") && cleanPathPrefix.length() > 1) {
cleanPathPrefix = cleanPathPrefix.substring(0, cleanPathPrefix.length() - 1);

if (cleanPathPrefix.endsWith("/")) {
throw new IllegalArgumentException("pathPrefix is malformed. too many trailing slashes: [" + pathPrefix + "]");
}
}

if (cleanPathPrefix.isEmpty() || "/".equals(cleanPathPrefix)) {
throw new IllegalArgumentException("pathPrefix must not be empty or '/': [" + pathPrefix + "]");
}

this.pathPrefix = cleanPathPrefix;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ public void testSetPathPrefixNull() {
}

public void testSetPathPrefixEmpty() {
assertSetPathPrefixThrows("/");
assertSetPathPrefixThrows("");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,33 @@ public void onFailure(Exception exception) {
}

public void testBuildUriLeavesPathUntouched() {
final Map<String, String> emptyMap = Collections.emptyMap();
{
URI uri = RestClient.buildUri("/foo$bar", "/index/type/id", Collections.<String, String>emptyMap());
URI uri = RestClient.buildUri("/foo$bar", "/index/type/id", emptyMap);
assertEquals("/foo$bar/index/type/id", uri.getPath());
}
{
URI uri = RestClient.buildUri(null, "/foo$bar/ty/pe/i/d", Collections.<String, String>emptyMap());
URI uri = RestClient.buildUri("/", "/*", emptyMap);
assertEquals("/*", uri.getPath());
}
{
URI uri = RestClient.buildUri("/", "*", emptyMap);
assertEquals("/*", uri.getPath());
}
{
URI uri = RestClient.buildUri(null, "*", emptyMap);
assertEquals("*", uri.getPath());
}
{
URI uri = RestClient.buildUri("", "*", emptyMap);
assertEquals("*", uri.getPath());
}
{
URI uri = RestClient.buildUri(null, "/*", emptyMap);
assertEquals("/*", uri.getPath());
}
{
URI uri = RestClient.buildUri(null, "/foo$bar/ty/pe/i/d", emptyMap);
assertEquals("/foo$bar/ty/pe/i/d", uri.getPath());
}
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public abstract class ESRestTestCase extends ESTestCase {
public static final String TRUSTSTORE_PASSWORD = "truststore.password";
public static final String CLIENT_RETRY_TIMEOUT = "client.retry.timeout";
public static final String CLIENT_SOCKET_TIMEOUT = "client.socket.timeout";
public static final String CLIENT_PATH_PREFIX = "client.path.prefix";

/**
* Convert the entity from a {@link Response} into a map of maps.
Expand Down Expand Up @@ -383,7 +384,11 @@ private void waitForClusterStateUpdatesToFinish() throws Exception {
* Used to obtain settings for the REST client that is used to send REST requests.
*/
protected Settings restClientSettings() {
return Settings.EMPTY;
Settings.Builder builder = Settings.builder();
if (System.getProperty("tests.rest.client_path_prefix") != null) {
builder.put(CLIENT_PATH_PREFIX, System.getProperty("tests.rest.client_path_prefix"));
}
return builder.build();
}

/**
Expand Down Expand Up @@ -454,6 +459,9 @@ protected static void configureClient(RestClientBuilder builder, Settings settin
final TimeValue socketTimeout = TimeValue.parseTimeValue(socketTimeoutString, CLIENT_SOCKET_TIMEOUT);
builder.setRequestConfigCallback(conf -> conf.setSocketTimeout(Math.toIntExact(socketTimeout.getMillis())));
}
if (settings.hasValue(CLIENT_PATH_PREFIX)) {
builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX));
}
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public static Iterable<Object[]> parameters() throws Exception {
protected Settings restClientSettings() {
String token = basicAuthHeaderValue(USER, new SecureString(PASS.toCharArray()));
return Settings.builder()
.put(super.restClientSettings())
.put(ThreadContext.PREFIX + ".Authorization", token)
.build();
}
Expand Down