Skip to content

Commit

Permalink
Enable setting client path prefix to / (elastic#30119)
Browse files Browse the repository at this point in the history
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 /.
  • Loading branch information
beiske authored and jasontedor committed Jul 1, 2018
1 parent bd6653f commit e302d80
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -794,8 +794,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 @@ -223,12 +223,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

0 comments on commit e302d80

Please sign in to comment.