Fix watcher HttpClient URL creation (elastic#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 elastic#44970
Pandinosaurus · Aug 13, 2019 · b2ae6d6 · b2ae6d6
1 parent 5436581
commit b2ae6d6
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 3 deletions.
diff --git a/.../plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java b/.../plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java
@@ -316,7 +316,8 @@ private HttpProxy getProxyFromSettings(Settings settings) {
         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 @@ private Tuple<HttpHost, URI> createURI(HttpRequest request) {
                 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()));
+                    }
                 }
             }
 

diff --git a/...in/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java b/...in/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java
@@ -8,6 +8,7 @@
 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.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.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 void testWhitelistEverythingByDefault() {
         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()));