Fix watcher HttpClient URL creation (#45207)

The http client could end up creating URLs, that did not resemble the
original one, when encoding. This fixes a couple of corner cases, where
too much or too few slashes were added to an URI.

Closes #44970
This commit is contained in:
Alexander Reelsen 2019-08-13 12:15:23 +02:00
parent 00e4fba2fb
commit dd527b4e91
2 changed files with 30 additions and 3 deletions

View File

@ -316,7 +316,8 @@ public class HttpClient implements Closeable {
return HttpProxy.NO_PROXY;
}
private Tuple<HttpHost, URI> createURI(HttpRequest request) {
// for testing
static Tuple<HttpHost, URI> createURI(HttpRequest request) {
try {
List<NameValuePair> qparams = new ArrayList<>(request.params.size());
request.params.forEach((k, v) -> qparams.add(new BasicNameValuePair(k, v)));
@ -327,9 +328,19 @@ public class HttpClient implements Closeable {
unescapedPathParts = Collections.emptyList();
} else {
final String[] pathParts = request.path.split("/");
final boolean isPathEndsWithSlash = request.path.endsWith("/");
unescapedPathParts = new ArrayList<>(pathParts.length);
for (String part : pathParts) {
unescapedPathParts.add(URLDecoder.decode(part, StandardCharsets.UTF_8.name()));
for (int i = 0; i < pathParts.length; i++) {
String part = pathParts[i];
boolean isLast = i == pathParts.length - 1;
if (Strings.isEmpty(part) == false) {
String appendPart = part;
boolean appendSlash = isPathEndsWithSlash && isLast;
if (appendSlash) {
appendPart += "/";
}
unescapedPathParts.add(URLDecoder.decode(appendPart, StandardCharsets.UTF_8.name()));
}
}
}

View File

@ -8,6 +8,7 @@ package org.elasticsearch.xpack.watcher.common.http;
import com.carrotsearch.randomizedtesting.generators.RandomStrings;
import com.sun.net.httpserver.HttpsServer;
import org.apache.http.HttpHeaders;
import org.apache.http.HttpHost;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.config.RequestConfig;
import org.apache.logging.log4j.message.ParameterizedMessage;
@ -16,6 +17,7 @@ import org.apache.lucene.util.automaton.CharacterRunAutomaton;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.bootstrap.JavaVersion;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
@ -45,6 +47,7 @@ import java.net.InetAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketTimeoutException;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.security.AccessController;
@ -738,6 +741,19 @@ public class HttpClientTests extends ESTestCase {
assertThat(automaton.run(randomAlphaOfLength(10)), is(true));
}
public void testCreateUri() throws Exception {
assertCreateUri("https://example.org/foo/", "/foo/");
assertCreateUri("https://example.org/foo", "/foo");
assertCreateUri("https://example.org/", "");
assertCreateUri("https://example.org", "");
}
private void assertCreateUri(String uri, String expectedPath) {
final HttpRequest request = HttpRequest.builder().fromUrl(uri).build();
final Tuple<HttpHost, URI> tuple = HttpClient.createURI(request);
assertThat(tuple.v2().getPath(), is(expectedPath));
}
public static ClusterService mockClusterService() {
ClusterService clusterService = mock(ClusterService.class);
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, new HashSet<>(HttpSettings.getSettings()));