From e302d803c86ce9c38d1dc524a6e5868d54b5b698 Mon Sep 17 00:00:00 2001 From: Konrad Beiske Date: Sun, 1 Jul 2018 19:42:03 +0200 Subject: [PATCH] Enable setting client path prefix to / (#30119) Some proxies require all requests to have paths starting with / since there are no relative paths at the HTTP connection level. Elasticsearch assumes paths are absolute. In order to run rest tests against a cluster behind such a proxy, set the system property tests.rest.client_path_prefix to /. --- .../org/elasticsearch/client/RestClient.java | 6 +++-- .../client/RestClientBuilder.java | 17 +++++++------ .../client/RestClientBuilderTests.java | 1 - .../elasticsearch/client/RestClientTests.java | 25 +++++++++++++++++-- .../test/rest/ESRestTestCase.java | 10 +++++++- ...CoreWithSecurityClientYamlTestSuiteIT.java | 1 + 6 files changed, 46 insertions(+), 14 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index 00b275f701c6d..934b952608674 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -794,8 +794,10 @@ static URI buildUri(String pathPrefix, String path, Map 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; diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java index fb61f4f17c483..dd3f5ad5a7274 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -143,22 +143,26 @@ public RestClientBuilder setRequestConfigCallback(RequestConfigCallback requestC * For example, if this is set to "/my/path", then any client request will become "/my/path/" + endpoint. *

* 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("/")) { @@ -166,9 +170,6 @@ public RestClientBuilder setPathPrefix(String pathPrefix) { } } - if (cleanPathPrefix.isEmpty() || "/".equals(cleanPathPrefix)) { - throw new IllegalArgumentException("pathPrefix must not be empty or '/': [" + pathPrefix + "]"); - } this.pathPrefix = cleanPathPrefix; return this; diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java index 9fcb4978e28a7..834748d65de34 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java @@ -180,7 +180,6 @@ public void testSetPathPrefixNull() { } public void testSetPathPrefixEmpty() { - assertSetPathPrefixThrows("/"); assertSetPathPrefixThrows(""); } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java index 271fc51ef8835..ef94b70542f6c 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java @@ -223,12 +223,33 @@ public void onFailure(Exception exception) { } public void testBuildUriLeavesPathUntouched() { + final Map emptyMap = Collections.emptyMap(); { - URI uri = RestClient.buildUri("/foo$bar", "/index/type/id", Collections.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.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()); } { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index 495df4aa461a9..8737378dbd715 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -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. @@ -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(); } /** @@ -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") diff --git a/x-pack/qa/core-rest-tests-with-security/src/test/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java b/x-pack/qa/core-rest-tests-with-security/src/test/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java index e5c82a23a59a5..212a342479d3f 100644 --- a/x-pack/qa/core-rest-tests-with-security/src/test/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java +++ b/x-pack/qa/core-rest-tests-with-security/src/test/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java @@ -39,6 +39,7 @@ public static Iterable 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(); }