Merge pull request #5788 from square/jwilson.0216.null_host

Don't crash on a null host
square · Feb 16, 2020 · 4bb66bc · 4bb66bc
2 parents 2479bd0 + f00566f
commit 4bb66bc
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 10 deletions.
diff --git a/okhttp/src/main/java/okhttp3/internal/connection/RouteSelector.kt b/okhttp/src/main/java/okhttp3/internal/connection/RouteSelector.kt
@@ -94,19 +94,23 @@ class RouteSelector(
 
   /** Prepares the proxy servers to try. */
   private fun resetNextProxy(url: HttpUrl, proxy: Proxy?) {
-    eventListener.proxySelectStart(call, url)
-    proxies = if (proxy != null) {
+    fun selectProxies(): List<Proxy> {
       // If the user specifies a proxy, try that and only that.
-      listOf(proxy)
-    } else {
+      if (proxy != null) return listOf(proxy)
+
+      // If the URI lacks a host (as in "http://</"), don't call the ProxySelector.
+      val uri = url.toUri()
+      if (uri.host == null) return immutableListOf(Proxy.NO_PROXY)
+
       // Try each of the ProxySelector choices until one connection succeeds.
-      val proxiesOrNull = address.proxySelector.select(url.toUri())
-      if (proxiesOrNull != null && proxiesOrNull.isNotEmpty()) {
-        proxiesOrNull.toImmutableList()
-      } else {
-        immutableListOf(Proxy.NO_PROXY)
-      }
+      val proxiesOrNull = address.proxySelector.select(uri)
+      if (proxiesOrNull.isNullOrEmpty()) return immutableListOf(Proxy.NO_PROXY)
+
+      return proxiesOrNull.toImmutableList()
     }
+
+    eventListener.proxySelectStart(call, url)
+    proxies = selectProxies()
     nextProxyIndex = 0
     eventListener.proxySelectEnd(call, url, proxies)
   }

diff --git a/okhttp/src/test/java/okhttp3/HttpUrlTest.java b/okhttp/src/test/java/okhttp3/HttpUrlTest.java
@@ -440,6 +440,13 @@ HttpUrl parse(String url) {
         .test(Component.HOST);
   }
 
+  /** This one's ugly: the HttpUrl's host is non-empty, but the URI's host is null. */
+  @Test public void hostContainsOnlyStrippedCharacters() throws Exception {
+    HttpUrl url = parse("http://>/");
+    assertThat(url.host()).isEqualTo(">");
+    assertThat(url.uri().getHost()).isNull();
+  }
+
   @Test public void hostIpv6() throws Exception {
     // Square braces are absent from host()...
     assertThat(parse("http://[::1]/").host()).isEqualTo("::1");

diff --git a/okhttp/src/test/java/okhttp3/internal/connection/RouteSelectorTest.java b/okhttp/src/test/java/okhttp3/internal/connection/RouteSelectorTest.java
@@ -181,6 +181,30 @@ public final class RouteSelectorTest {
     proxySelector.assertRequests(); // No proxy selector requests!
   }
 
+  /**
+   * Don't call through to the proxy selector if we don't have a host name.
+   * https://github.com/square/okhttp/issues/5770
+   */
+  @Test public void proxySelectorNotCalledForNullHost() throws Exception {
+    // The string '>' is okay in a hostname in HttpUrl, which does very light hostname validation.
+    // It is not okay in URI, and so it's stripped and we get a URI with a null host.
+    String bogusHostname = ">";
+    Address address = new Address(bogusHostname, uriPort, dns, socketFactory, null, null, null,
+        authenticator, null, protocols, connectionSpecs, proxySelector);
+    RouteSelector routeSelector = new RouteSelector(address, routeDatabase, call,
+        EventListener.NONE);
+
+    assertThat(routeSelector.hasNext()).isTrue();
+    dns.set(bogusHostname, dns.allocate(1));
+    RouteSelector.Selection selection = routeSelector.next();
+    assertRoute(selection.next(), address, NO_PROXY, dns.lookup(bogusHostname, 0), uriPort);
+
+    assertThat(selection.hasNext()).isFalse();
+    assertThat(routeSelector.hasNext()).isFalse();
+    dns.assertRequests(bogusHostname);
+    proxySelector.assertRequests(); // No proxy selector requests!
+  }
+
   @Test public void proxySelectorReturnsNull() throws Exception {
     ProxySelector nullProxySelector = new ProxySelector() {
       @Override public List<Proxy> select(URI uri) {