From f5d669cd0fc8fc183c53e25e8414655eea4230a0 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Sat, 7 Mar 2020 03:10:10 +0000
Subject: [PATCH 1/4] Allow for users who have disabled certificate checks in
 dev. (#5835)

* Allow for users who have disabled certificate checks in dev.

* Avoid repeated calls

* typo

* Lock is assumed at this point

* Stay safe

* rework

* spotless

* Fix

* Handle one more case

* Capture the exception

* Add test

* Comment
---
 okhttp/src/main/java/okhttp3/Handshake.kt     | 15 ++++----
 .../internal/connection/ExchangeFinder.kt     |  7 ++--
 .../internal/connection/RealConnection.kt     | 11 ++++--
 .../okhttp3/ConnectionCoalescingTest.java     | 34 +++++++++++++++++++
 4 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/okhttp/src/main/java/okhttp3/Handshake.kt b/okhttp/src/main/java/okhttp3/Handshake.kt
index 0dd80bdbbe9a..4bc98acc1b69 100644
--- a/okhttp/src/main/java/okhttp3/Handshake.kt
+++ b/okhttp/src/main/java/okhttp3/Handshake.kt
@@ -48,8 +48,13 @@ class Handshake internal constructor(
   peerCertificatesFn: () -> List<Certificate>
 ) {
   /** Returns a possibly-empty list of certificates that identify the remote peer. */
-  @get:JvmName("peerCertificates") val peerCertificates: List<Certificate> by lazy(
-      peerCertificatesFn)
+  @get:JvmName("peerCertificates") val peerCertificates: List<Certificate> by lazy {
+    try {
+      peerCertificatesFn()
+    } catch (spue: SSLPeerUnverifiedException) {
+      listOf<Certificate>()
+    }
+  }
 
   @JvmName("-deprecated_tlsVersion")
   @Deprecated(
@@ -121,11 +126,7 @@ class Handshake internal constructor(
   }
 
   override fun toString(): String {
-    val peerCertificatesString = try {
-      peerCertificates.map { it.name }.toString()
-    } catch (_: SSLPeerUnverifiedException) {
-      "Failed: SSLPeerUnverifiedException"
-    }
+    val peerCertificatesString = peerCertificates.map { it.name }.toString()
     return "Handshake{" +
         "tlsVersion=$tlsVersion " +
         "cipherSuite=$cipherSuite " +
diff --git a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
index bf71ac2d6eef..1bd32de417a3 100644
--- a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
+++ b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
@@ -142,9 +142,10 @@ class ExchangeFinder(
     synchronized(connectionPool) {
       if (call.isCanceled()) throw IOException("Canceled")
 
-      releasedConnection = call.connection
-      toClose = if (call.connection != null &&
-          (call.connection!!.noNewExchanges || !call.connection!!.supportsUrl(address.url))) {
+      val callConnection = call.connection // changes within this overall method
+      releasedConnection = callConnection
+      toClose = if (callConnection != null && (callConnection.noNewExchanges ||
+              !callConnection.supportsUrl(address.url))) {
         call.releaseConnectionNoEvents()
       } else {
         null
diff --git a/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt b/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt
index 8ceb9ec9ac44..4dd219bbeeb9 100644
--- a/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt
+++ b/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt
@@ -576,9 +576,14 @@ class RealConnection(
     }
 
     // We have a host mismatch. But if the certificate matches, we're still good.
-    return !noCoalescedConnections &&
-        handshake != null &&
-        OkHostnameVerifier.verify(url.host, handshake!!.peerCertificates[0] as X509Certificate)
+    return !noCoalescedConnections && handshake != null && certificateSupportHost(url, handshake!!)
+  }
+
+  private fun certificateSupportHost(url: HttpUrl, handshake: Handshake): Boolean {
+    val peerCertificates = handshake.peerCertificates
+
+    return peerCertificates.isNotEmpty() && OkHostnameVerifier.verify(url.host,
+        peerCertificates[0] as X509Certificate)
   }
 
   @Throws(SocketException::class)
diff --git a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
index e0bdfd52371c..288ebe7666a5 100644
--- a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
+++ b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
@@ -19,12 +19,14 @@
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
+import java.security.cert.X509Certificate;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.X509TrustManager;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import okhttp3.testing.PlatformRule;
@@ -432,6 +434,38 @@ public final class ConnectionCoalescingTest {
     assertThat(client.connectionPool().connectionCount()).isEqualTo(2);
   }
 
+  /**
+   * Won't coalesce if we can't clean certs e.g. a dev setup.
+   */
+  @Test public void redirectWithDevSetup() throws Exception {
+    X509TrustManager TRUST_MANAGER = new X509TrustManager() {
+      @Override
+      public void checkClientTrusted(X509Certificate[] x509Certificates, String s) {
+      }
+
+      @Override
+      public void checkServerTrusted(X509Certificate[] x509Certificates, String s) {
+      }
+
+      @Override
+      public X509Certificate[] getAcceptedIssuers() {
+        return new X509Certificate[0];
+      }
+    };
+
+    client = client.newBuilder().sslSocketFactory(client.sslSocketFactory(), TRUST_MANAGER).build();
+
+    server.enqueue(new MockResponse());
+    server.enqueue(new MockResponse());
+
+    assert200Http2Response(execute(url), server.getHostName());
+
+    HttpUrl sanUrl = url.newBuilder().host("san.com").build();
+    assert200Http2Response(execute(sanUrl), "san.com");
+
+    assertThat(client.connectionPool().connectionCount()).isEqualTo(2);
+  }
+
   private Response execute(HttpUrl url) throws IOException {
     return client.newCall(new Request.Builder().url(url).build()).execute();
   }

From 71046d45cc7d9262b9f5ccd5c55aeb225ccf6819 Mon Sep 17 00:00:00 2001
From: Jesse Wilson <jwilson@squareup.com>
Date: Sun, 8 Mar 2020 09:09:14 -0400
Subject: [PATCH 2/4] Don't reuse a connection on redirect if certs match but
 DNS does not (4.4.x)

We attempt to minimize connection and reconnection work, but in this case we
were overly aggressive about retaining the same connection. In some deployments
services will share certificates but not DNS addresses; when redirecting
between such services we were incorrectly attempting to reuse the connection.

This would have resulted in 404s and other misdirected requests.

Closes: https://github.com/square/okhttp/issues/5859
---
 .../okhttp3/internal/connection/Exchange.kt   |  2 +-
 .../internal/connection/ExchangeFinder.kt     | 13 +++-
 .../internal/http/RealInterceptorChain.kt     |  2 +-
 .../okhttp3/ConnectionCoalescingTest.java     | 68 ++++++++++++++++++-
 4 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/okhttp/src/main/java/okhttp3/internal/connection/Exchange.kt b/okhttp/src/main/java/okhttp3/internal/connection/Exchange.kt
index 546738082669..fc44f72be339 100644
--- a/okhttp/src/main/java/okhttp3/internal/connection/Exchange.kt
+++ b/okhttp/src/main/java/okhttp3/internal/connection/Exchange.kt
@@ -40,7 +40,7 @@ import okio.buffer
 class Exchange(
   internal val call: RealCall,
   internal val eventListener: EventListener,
-  private val finder: ExchangeFinder,
+  internal val finder: ExchangeFinder,
   private val codec: ExchangeCodec
 ) {
   /** Returns true if the request body need not complete before the response body starts. */
diff --git a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
index 1bd32de417a3..2a7bec00a6d9 100644
--- a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
+++ b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt
@@ -19,6 +19,7 @@ import java.io.IOException
 import java.net.Socket
 import okhttp3.Address
 import okhttp3.EventListener
+import okhttp3.HttpUrl
 import okhttp3.OkHttpClient
 import okhttp3.Route
 import okhttp3.internal.assertThreadDoesntHoldLock
@@ -145,7 +146,7 @@ class ExchangeFinder(
       val callConnection = call.connection // changes within this overall method
       releasedConnection = callConnection
       toClose = if (callConnection != null && (callConnection.noNewExchanges ||
-              !callConnection.supportsUrl(address.url))) {
+              !sameHostAndPort(callConnection.route().address.url))) {
         call.releaseConnectionNoEvents()
       } else {
         null
@@ -333,4 +334,14 @@ class ExchangeFinder(
         connection.routeFailureCount == 0 &&
         connection.route().address.url.canReuseConnectionFor(address.url)
   }
+
+  /**
+   * Returns true if the host and port are unchanged from when this was created. This is used to
+   * detect if followups need to do a full connection-finding process including DNS resolution, and
+   * certificate pin checks.
+   */
+  fun sameHostAndPort(url: HttpUrl): Boolean {
+    val routeUrl = address.url
+    return url.port == routeUrl.port && url.host == routeUrl.host
+  }
 }
diff --git a/okhttp/src/main/java/okhttp3/internal/http/RealInterceptorChain.kt b/okhttp/src/main/java/okhttp3/internal/http/RealInterceptorChain.kt
index 5062ac820fd8..2cb09a499256 100644
--- a/okhttp/src/main/java/okhttp3/internal/http/RealInterceptorChain.kt
+++ b/okhttp/src/main/java/okhttp3/internal/http/RealInterceptorChain.kt
@@ -84,7 +84,7 @@ class RealInterceptorChain(
     calls++
 
     if (exchange != null) {
-      check(exchange.connection.supportsUrl(request.url)) {
+      check(exchange.finder.sameHostAndPort(request.url)) {
         "network interceptor ${interceptors[index - 1]} must retain the same host and port"
       }
       check(calls == 1) {
diff --git a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
index 288ebe7666a5..1c4e263680cb 100644
--- a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
+++ b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java
@@ -26,6 +26,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.X509TrustManager;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
@@ -269,9 +270,25 @@ public final class ConnectionCoalescingTest {
     }
   }
 
+  @Test public void skipsOnRedirectWhenDnsDontMatch() throws Exception {
+    server.enqueue(new MockResponse()
+        .setResponseCode(301)
+        .addHeader("Location", url.newBuilder().host("differentdns.com").build()));
+    server.enqueue(new MockResponse()
+        .setBody("unexpected call"));
+
+    try {
+      Response response = execute(url);
+      response.close();
+      fail("expected a failed attempt to connect");
+    } catch (IOException expected) {
+    }
+  }
+
   /** Not in the certificate SAN. */
   @Test public void skipsWhenNotSubjectAltName() throws Exception {
     server.enqueue(new MockResponse());
+    server.enqueue(new MockResponse());
 
     assert200Http2Response(execute(url), server.getHostName());
 
@@ -280,7 +297,21 @@ public final class ConnectionCoalescingTest {
     try {
       execute(nonsanUrl);
       fail("expected a failed attempt to connect");
-    } catch (IOException expected) {
+    } catch (SSLPeerUnverifiedException expected) {
+    }
+  }
+
+  @Test public void skipsOnRedirectWhenNotSubjectAltName() throws Exception {
+    server.enqueue(new MockResponse()
+        .setResponseCode(301)
+        .addHeader("Location", url.newBuilder().host("nonsan.com").build()));
+    server.enqueue(new MockResponse());
+
+    try {
+      Response response = execute(url);
+      response.close();
+      fail("expected a failed attempt to connect");
+    } catch (SSLPeerUnverifiedException expected) {
     }
   }
 
@@ -323,6 +354,24 @@ public final class ConnectionCoalescingTest {
     }
   }
 
+  @Test public void skipsOnRedirectWhenCertificatePinningFails() throws Exception {
+    CertificatePinner pinner = new CertificatePinner.Builder()
+        .add("san.com", "sha1/afwiKY3RxoMmLkuRW1l7QsPZTJPwDS2pdDROQjXw8ig=")
+        .build();
+    client = client.newBuilder().certificatePinner(pinner).build();
+
+    server.enqueue(new MockResponse()
+        .setResponseCode(301)
+        .addHeader("Location", url.newBuilder().host("san.com").build()));
+    server.enqueue(new MockResponse());
+
+    try {
+      execute(url);
+      fail("expected a failed attempt to connect");
+    } catch (SSLPeerUnverifiedException expected) {
+    }
+  }
+
   /**
    * Skips coalescing when hostname verifier is overridden since the intention of the hostname
    * verification is a black box.
@@ -343,6 +392,22 @@ public final class ConnectionCoalescingTest {
     assertThat(client.connectionPool().connectionCount()).isEqualTo(2);
   }
 
+  @Test public void skipsOnRedirectWhenHostnameVerifierUsed() throws Exception {
+    HostnameVerifier verifier = (name, session) -> true;
+    client = client.newBuilder().hostnameVerifier(verifier).build();
+
+    server.enqueue(new MockResponse()
+        .setResponseCode(301)
+        .addHeader("Location", url.newBuilder().host("san.com").build()));
+    server.enqueue(new MockResponse());
+
+    assert200Http2Response(execute(url), "san.com");
+
+    assertThat(client.connectionPool().connectionCount()).isEqualTo(2);
+    assertThat(server.takeRequest().getSequenceNumber()).isEqualTo(0); // Fresh connection.
+    assertThat(server.takeRequest().getSequenceNumber()).isEqualTo(0); // Fresh connection.
+  }
+
   /**
    * Check we would use an existing connection to a later DNS result instead of connecting to the
    * first DNS result for the first time.
@@ -376,7 +441,6 @@ public final class ConnectionCoalescingTest {
 
   /** Check that wildcard SANs are supported. */
   @Test public void commonThenWildcard() throws Exception {
-
     server.enqueue(new MockResponse());
     server.enqueue(new MockResponse());
 

From 046f7f2450f9c913d759e7c8bdb52fda528fb631 Mon Sep 17 00:00:00 2001
From: Jesse Wilson <jwilson@squareup.com>
Date: Sun, 8 Mar 2020 09:36:32 -0400
Subject: [PATCH 3/4] Prepare for release 4.4.1.

---
 README.md                            | 8 ++++----
 build.gradle                         | 2 +-
 mockwebserver/README.md              | 2 +-
 okhttp-brotli/README.md              | 2 +-
 okhttp-dnsoverhttps/README.md        | 2 +-
 okhttp-logging-interceptor/README.md | 2 +-
 okhttp-sse/README.md                 | 2 +-
 okhttp-tls/README.md                 | 2 +-
 okhttp-urlconnection/README.md       | 2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index 105ecbc96e96..22955bffdeab 100644
--- a/README.md
+++ b/README.md
@@ -99,10 +99,10 @@ Releases
 
 Our [change log][changelog] has release history.
 
-The latest release is available on [Maven Central](https://search.maven.org/artifact/com.squareup.okhttp3/okhttp/4.4.0/jar).
+The latest release is available on [Maven Central](https://search.maven.org/artifact/com.squareup.okhttp3/okhttp/4.4.1/jar).
 
 ```kotlin
-implementation("com.squareup.okhttp3:okhttp:4.4.0")
+implementation("com.squareup.okhttp3:okhttp:4.4.1")
 ```
 
 Snapshot builds are [available][snap]. [R8 and ProGuard][r8_proguard] rules are available.
@@ -113,10 +113,10 @@ MockWebServer
 
 OkHttp includes a library for testing HTTP, HTTPS, and HTTP/2 clients.
 
-The latest release is available on [Maven Central](https://search.maven.org/artifact/com.squareup.okhttp3/mockwebserver/4.4.0/jar).
+The latest release is available on [Maven Central](https://search.maven.org/artifact/com.squareup.okhttp3/mockwebserver/4.4.1/jar).
 
 ```kotlin
-testImplementation("com.squareup.okhttp3:mockwebserver:4.4.0")
+testImplementation("com.squareup.okhttp3:mockwebserver:4.4.1")
 ```
 
 License
diff --git a/build.gradle b/build.gradle
index 1b7ff7cfdaf1..611fb8caf2b3 100644
--- a/build.gradle
+++ b/build.gradle
@@ -90,7 +90,7 @@ ext.publishedArtifactId = { project ->
 allprojects {
   group = 'com.squareup.okhttp3'
   project.ext.artifactId = rootProject.ext.publishedArtifactId(project)
-  version = '4.4.0'
+  version = '4.4.1'
 
   repositories {
     mavenCentral()
diff --git a/mockwebserver/README.md b/mockwebserver/README.md
index 4cc56a7e8e79..0367e501d45e 100644
--- a/mockwebserver/README.md
+++ b/mockwebserver/README.md
@@ -142,7 +142,7 @@ server.setDispatcher(dispatcher);
 ### Download
 
 ```kotlin
-testImplementation("com.squareup.okhttp3:mockwebserver:4.4.0")
+testImplementation("com.squareup.okhttp3:mockwebserver:4.4.1")
 ```
 
 ### License
diff --git a/okhttp-brotli/README.md b/okhttp-brotli/README.md
index e45a6670afd0..17d3b7f90c26 100644
--- a/okhttp-brotli/README.md
+++ b/okhttp-brotli/README.md
@@ -14,7 +14,7 @@ OkHttpClient client = new OkHttpClient.Builder()
 ```
 
 ```kotlin
-implementation("com.squareup.okhttp3:okhttp-brotli:4.4.0")
+implementation("com.squareup.okhttp3:okhttp-brotli:4.4.1")
 ```
 
  [1]: https://github.com/google/brotli
diff --git a/okhttp-dnsoverhttps/README.md b/okhttp-dnsoverhttps/README.md
index 04365c015938..0fddbd1a7d15 100644
--- a/okhttp-dnsoverhttps/README.md
+++ b/okhttp-dnsoverhttps/README.md
@@ -7,5 +7,5 @@ API is not considered stable and may change at any time.
 ### Download
 
 ```kotlin
-testImplementation("com.squareup.okhttp3:okhttp-dnsoverhttps:4.4.0")
+testImplementation("com.squareup.okhttp3:okhttp-dnsoverhttps:4.4.1")
 ```
diff --git a/okhttp-logging-interceptor/README.md b/okhttp-logging-interceptor/README.md
index 8d234ec89c1f..bff4e200cd1c 100644
--- a/okhttp-logging-interceptor/README.md
+++ b/okhttp-logging-interceptor/README.md
@@ -37,7 +37,7 @@ Download
 --------
 
 ```kotlin
-implementation("com.squareup.okhttp3:logging-interceptor:4.4.0")
+implementation("com.squareup.okhttp3:logging-interceptor:4.4.1")
 ```
 
 
diff --git a/okhttp-sse/README.md b/okhttp-sse/README.md
index 8d5c0f45527c..fc746b74f931 100644
--- a/okhttp-sse/README.md
+++ b/okhttp-sse/README.md
@@ -7,5 +7,5 @@ API is not considered stable and may change at any time.
 ### Download
 
 ```kotlin
-testImplementation("com.squareup.okhttp3:okhttp-sse:4.4.0")
+testImplementation("com.squareup.okhttp3:okhttp-sse:4.4.1")
 ```
diff --git a/okhttp-tls/README.md b/okhttp-tls/README.md
index 7e806f85f431..add074220a78 100644
--- a/okhttp-tls/README.md
+++ b/okhttp-tls/README.md
@@ -227,7 +227,7 @@ Download
 --------
 
 ```kotlin
-implementation("com.squareup.okhttp3:okhttp-tls:4.4.0")
+implementation("com.squareup.okhttp3:okhttp-tls:4.4.1")
 ```
 
  [held_certificate]: http://square.github.io/okhttp/4.x/okhttp-tls/okhttp3.tls/-held-certificate/
diff --git a/okhttp-urlconnection/README.md b/okhttp-urlconnection/README.md
index 50eeb0c44de9..175c5deb13fa 100644
--- a/okhttp-urlconnection/README.md
+++ b/okhttp-urlconnection/README.md
@@ -6,5 +6,5 @@ This module integrates OkHttp with `Authenticator` and `CookieHandler` from `jav
 ### Download
 
 ```kotlin
-testImplementation("com.squareup.okhttp3:okhttp-urlconnection:4.4.0")
+testImplementation("com.squareup.okhttp3:okhttp-urlconnection:4.4.1")
 ```

From eedeef9687ff1267973a59ccb4394e033f05d782 Mon Sep 17 00:00:00 2001
From: Jesse Wilson <jwilson@squareup.com>
Date: Sun, 8 Mar 2020 09:36:52 -0400
Subject: [PATCH 4/4] Prepare next development version.

---
 build.gradle | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build.gradle b/build.gradle
index 611fb8caf2b3..b541cf5ab417 100644
--- a/build.gradle
+++ b/build.gradle
@@ -90,7 +90,7 @@ ext.publishedArtifactId = { project ->
 allprojects {
   group = 'com.squareup.okhttp3'
   project.ext.artifactId = rootProject.ext.publishedArtifactId(project)
-  version = '4.4.1'
+  version = '4.4.2-SNAPSHOT'
 
   repositories {
     mavenCentral()