Merge pull request searchbox-io#592 from graylog-labs/scrub-server-uris

Scrub server URIs before logging them in AbstractJestClient
chuanheng · Jun 19, 2018 · fe19dc4 · fe19dc4
2 parents 65139a5 + 2355a8c
commit fe19dc4
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 2 deletions.
diff --git a/jest-common/src/main/java/io/searchbox/client/AbstractJestClient.java b/jest-common/src/main/java/io/searchbox/client/AbstractJestClient.java
@@ -1,6 +1,7 @@
 package io.searchbox.client;
 
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -13,6 +14,8 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -49,13 +52,13 @@ public void setServers(Set<String> servers) {
         if (servers.equals(serverPoolReference.get().getServers())) {
             if (log.isDebugEnabled()) {
                 log.debug("Server pool already contains same list of servers: {}",
-                        Joiner.on(',').join(servers));
+                        Joiner.on(',').join(scrubServerURIs(servers)));
             }
             return;
         }
         if (log.isInfoEnabled()) {
             log.info("Setting server pool to a list of {} servers: [{}]",
-                      servers.size(), Joiner.on(',').join(servers));
+                      servers.size(), Joiner.on(',').join(scrubServerURIs(servers)));
         }
         serverPoolReference.set(new ServerPool(servers));
 
@@ -64,6 +67,27 @@ public void setServers(Set<String> servers) {
         }
     }
 
+    @VisibleForTesting
+    Set<String> scrubServerURIs(Set<String> servers) {
+        final ImmutableSet.Builder<String> scrubbedServers = ImmutableSet.builder();
+        for (String server : servers) {
+            final URI originalURI = URI.create(server);
+            try {
+                final URI scrubbedURI = new URI(originalURI.getScheme(),
+                        null, // Remove user info
+                        originalURI.getHost(),
+                        originalURI.getPort(),
+                        originalURI.getPath(),
+                        originalURI.getQuery(),
+                        originalURI.getFragment());
+                scrubbedServers.add(scrubbedURI.toString());
+            } catch (URISyntaxException e) {
+                log.debug("Couldn't scrub server URI " + originalURI, e);
+            }
+        }
+        return scrubbedServers.build();
+    }
+
     /**
      * {@inheritDoc}
      */

diff --git a/jest-common/src/test/java/io/searchbox/client/AbstractJestClientTest.java b/jest-common/src/test/java/io/searchbox/client/AbstractJestClientTest.java
@@ -1,5 +1,6 @@
 package io.searchbox.client;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import io.searchbox.action.Action;
 import org.junit.Test;
@@ -63,6 +64,39 @@ public void testGetElasticSearchServer() throws Exception {
         assertTrue(set.contains("http://localhost:9400"));
     }
 
+    @Test
+    public void testGetElasticSearchServerWithCredentials() {
+        final Set<String> set = ImmutableSet.of(
+                "http://user:pass@localhost:9200",
+                "http://user:pass@localhost:9300",
+                "http://user:pass@localhost:9400");
+        client.setServers(set);
+
+        Set<String> serverList = new HashSet<>();
+
+        for (int i = 0; i < set.size(); i++) {
+            serverList.add(client.getNextServer());
+        }
+
+        assertEquals("round robin does not work", 3, serverList.size());
+
+        assertTrue(set.contains("http://user:pass@localhost:9200"));
+        assertTrue(set.contains("http://user:pass@localhost:9300"));
+        assertTrue(set.contains("http://user:pass@localhost:9400"));
+    }
+
+    @Test
+    public void testScrubServerURIs() {
+        final Set<String> set = ImmutableSet.of(
+                "http://user:pass@localhost:9200",
+                "http://localhost:9300/path?query#fragment");
+        final Set<String> scrubbedURIs = client.scrubServerURIs(set);
+
+        assertEquals(2, scrubbedURIs.size());
+        assertTrue(scrubbedURIs.contains("http://localhost:9200"));
+        assertTrue(scrubbedURIs.contains("http://localhost:9300/path?query#fragment"));
+    }
+
     @Test
     public void testGetElasticSearchServerIsThreadsafe() throws Exception {
         final int NUM_THREADS = 12;