Merge branch '15characterlimi-pinner-equals'

* 15characterlimi-pinner-equals:
  Fix Connection sharing across equivalent OkHttpClients

okhttp-tests/src/test/java/okhttp3/CertificateChainCleanerTest.java

@@ -21,6 +21,9 @@
 import java.util.ArrayList;
 import java.util.List;
 import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSocketFactory;
+import javax.net.ssl.X509TrustManager;
+import okhttp3.internal.platform.Platform;
 import okhttp3.internal.tls.CertificateChainCleaner;
 import okhttp3.internal.tls.HeldCertificate;
 import org.junit.Test;
@@ -29,6 +32,27 @@
 import static org.junit.Assert.fail;
 
 public final class CertificateChainCleanerTest {
+  @Test public void equalsFromCertificate() throws Exception {
+    HeldCertificate rootA = new HeldCertificate.Builder()
+        .serialNumber("1")
+        .build();
+    HeldCertificate rootB = new HeldCertificate.Builder()
+        .serialNumber("2")
+        .build();
+    assertEquals(
+        CertificateChainCleaner.get(rootA.certificate, rootB.certificate),
+        CertificateChainCleaner.get(rootB.certificate, rootA.certificate));
+  }
+
+  @Test public void equalsFromTrustManager() throws Exception {
+    Platform platform = Platform.get();
+    X509TrustManager x509TrustManager = platform.trustManager(
+        (SSLSocketFactory) SSLSocketFactory.getDefault());
+    assertEquals(
+        CertificateChainCleaner.get(x509TrustManager),
+        CertificateChainCleaner.get(x509TrustManager));
+  }
+
   @Test public void normalizeSingleSelfSignedCertificate() throws Exception {
     HeldCertificate root = new HeldCertificate.Builder()
         .serialNumber("1")

okhttp-tests/src/test/java/okhttp3/OkHttpClientTest.java

@@ -120,4 +120,10 @@ public final class OkHttpClientTest {
     } catch (IllegalArgumentException expected) {
     }
   }
+
+  @Test public void certificatePinnerEquality() {
+    OkHttpClient clientA = TestUtil.defaultClient();
+    OkHttpClient clientB = TestUtil.defaultClient();
+    assertEquals(clientA.certificatePinner(), clientB.certificatePinner());
+  }
 }

okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java

@@ -20,6 +20,7 @@
 import java.io.OutputStream;
 import java.net.Authenticator;
 import java.net.ConnectException;
+import java.net.CookieManager;
 import java.net.HttpRetryException;
 import java.net.HttpURLConnection;
 import java.net.InetAddress;
@@ -593,7 +594,15 @@ private void doUpload(TransferKind uploadKind, WriteKind writeKind) throws Excep
     assertNotNull(httpsConnection.getCipherSuite());
   }
 
-  @Test public void connectViaHttpsReusingConnections() throws IOException, InterruptedException {
+  @Test public void connectViaHttpsReusingConnections() throws Exception {
+    connectViaHttpsReusingConnections(false);
+  }
+
+  @Test public void connectViaHttpsReusingConnectionsAfterRebuildingClient() throws Exception {
+    connectViaHttpsReusingConnections(true);
+  }
+
+  private void connectViaHttpsReusingConnections(boolean rebuildClient) throws Exception {
     server.useHttps(sslClient.socketFactory, false);
     server.enqueue(new MockResponse().setBody("this response comes via HTTPS"));
     server.enqueue(new MockResponse().setBody("another response via HTTPS"));
@@ -602,13 +611,29 @@ private void doUpload(TransferKind uploadKind, WriteKind writeKind) throws Excep
     SSLSocketFactory clientSocketFactory = sslClient.socketFactory;
     RecordingHostnameVerifier hostnameVerifier = new RecordingHostnameVerifier();
 
-    urlFactory.setClient(urlFactory.client().newBuilder()
+    CookieJar cookieJar = new JavaNetCookieJar(new CookieManager());
+    ConnectionPool connectionPool = new ConnectionPool();
+
+    urlFactory.setClient(new OkHttpClient.Builder()
+        .cache(cache)
+        .connectionPool(connectionPool)
+        .cookieJar(cookieJar)
         .sslSocketFactory(clientSocketFactory, sslClient.trustManager)
         .hostnameVerifier(hostnameVerifier)
         .build());
     connection = urlFactory.open(server.url("/").url());
     assertContent("this response comes via HTTPS", connection);
 
+    if (rebuildClient) {
+      urlFactory.setClient(new OkHttpClient.Builder()
+          .cache(cache)
+          .connectionPool(connectionPool)
+          .cookieJar(cookieJar)
+          .sslSocketFactory(clientSocketFactory, sslClient.trustManager)
+          .hostnameVerifier(hostnameVerifier)
+          .build());
+    }
+
     connection = urlFactory.open(server.url("/").url());
     assertContent("another response via HTTPS", connection);
 

okhttp/src/main/java/okhttp3/CertificatePinner.java

@@ -20,12 +20,16 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 import javax.net.ssl.SSLPeerUnverifiedException;
 import okhttp3.internal.Util;
 import okhttp3.internal.tls.CertificateChainCleaner;
 import okio.ByteString;
 
+import static okhttp3.internal.Util.equal;
+
 /**
  * Constrains which certificates are trusted. Pinning certificates defends against attacks on
  * certificate authorities. It also prevents connections through man-in-the-middle certificate
@@ -124,14 +128,27 @@
 public final class CertificatePinner {
   public static final CertificatePinner DEFAULT = new Builder().build();
 
-  private final List<Pin> pins;
+  private final Set<Pin> pins;
   private final CertificateChainCleaner certificateChainCleaner;
 
-  private CertificatePinner(List<Pin> pins, CertificateChainCleaner certificateChainCleaner) {
+  private CertificatePinner(Set<Pin> pins, CertificateChainCleaner certificateChainCleaner) {
     this.pins = pins;
     this.certificateChainCleaner = certificateChainCleaner;
   }
 
+  @Override public boolean equals(Object other) {
+    if (other == this) return true;
+    return other instanceof CertificatePinner
+        && (equal(certificateChainCleaner, ((CertificatePinner) other).certificateChainCleaner)
+        && pins.equals(((CertificatePinner) other).pins));
+  }
+
+  @Override public int hashCode() {
+    int result = certificateChainCleaner != null ? certificateChainCleaner.hashCode() : 0;
+    result = 31 * result + pins.hashCode();
+    return result;
+  }
+
   /**
    * Confirms that at least one of the certificates pinned for {@code hostname} is in {@code
    * peerCertificates}. Does nothing if there are no certificates pinned for {@code hostname}.
@@ -210,9 +227,9 @@ List<Pin> findMatchingPins(String hostname) {
 
   /** Returns a certificate pinner that uses {@code certificateChainCleaner}. */
   CertificatePinner withCertificateChainCleaner(CertificateChainCleaner certificateChainCleaner) {
-    return this.certificateChainCleaner != certificateChainCleaner
-        ? new CertificatePinner(pins, certificateChainCleaner)
-        : this;
+    return equal(this.certificateChainCleaner, certificateChainCleaner)
+        ? this
+        : new CertificatePinner(pins, certificateChainCleaner);
   }
 
   /**
@@ -319,7 +336,7 @@ public Builder add(String pattern, String... pins) {
     }
 
     public CertificatePinner build() {
-      return new CertificatePinner(Util.immutableList(pins), null);
+      return new CertificatePinner(new LinkedHashSet<>(pins), null);
     }
   }
 }

okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java

@@ -239,6 +239,14 @@ static final class AndroidCertificateChainCleaner extends CertificateChainCleane
         throw new AssertionError(e);
       }
     }
+
+    @Override public boolean equals(Object other) {
+      return other instanceof AndroidCertificateChainCleaner; // All instances are equivalent.
+    }
+
+    @Override public int hashCode() {
+      return 0;
+    }
   }
 
   /**

okhttp/src/main/java/okhttp3/internal/tls/BasicCertificateChainCleaner.java

@@ -111,4 +111,14 @@ private boolean verifySignature(X509Certificate toVerify, X509Certificate signin
       return false;
     }
   }
+
+  @Override public int hashCode() {
+    return trustRootIndex.hashCode();
+  }
+
+  @Override public boolean equals(Object other) {
+    if (other == this) return true;
+    return other instanceof BasicCertificateChainCleaner
+        && ((BasicCertificateChainCleaner) other).trustRootIndex.equals(trustRootIndex);
+  }
 }

okhttp/src/main/java/okhttp3/internal/tls/TrustRootIndex.java

@@ -20,10 +20,10 @@
 import java.security.PublicKey;
 import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
-import java.util.ArrayList;
 import java.util.LinkedHashMap;
-import java.util.List;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
 import javax.net.ssl.X509TrustManager;
 import javax.security.auth.x500.X500Principal;
 
@@ -79,19 +79,37 @@ static final class AndroidTrustRootIndex extends TrustRootIndex {
         return null;
       }
     }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == this) {
+        return true;
+      }
+      if (!(obj instanceof AndroidTrustRootIndex)) {
+        return false;
+      }
+      AndroidTrustRootIndex that = (AndroidTrustRootIndex) obj;
+      return trustManager.equals(that.trustManager)
+              && findByIssuerAndSignatureMethod.equals(that.findByIssuerAndSignatureMethod);
+    }
+
+    @Override
+    public int hashCode() {
+      return trustManager.hashCode() + 31 * findByIssuerAndSignatureMethod.hashCode();
+    }
   }
 
   /** A simple index that of trusted root certificates that have been loaded into memory. */
   static final class BasicTrustRootIndex extends TrustRootIndex {
-    private final Map<X500Principal, List<X509Certificate>> subjectToCaCerts;
+    private final Map<X500Principal, Set<X509Certificate>> subjectToCaCerts;
 
     public BasicTrustRootIndex(X509Certificate... caCerts) {
       subjectToCaCerts = new LinkedHashMap<>();
       for (X509Certificate caCert : caCerts) {
         X500Principal subject = caCert.getSubjectX500Principal();
-        List<X509Certificate> subjectCaCerts = subjectToCaCerts.get(subject);
+        Set<X509Certificate> subjectCaCerts = subjectToCaCerts.get(subject);
         if (subjectCaCerts == null) {
-          subjectCaCerts = new ArrayList<>(1);
+          subjectCaCerts = new LinkedHashSet<>(1);
           subjectToCaCerts.put(subject, subjectCaCerts);
         }
         subjectCaCerts.add(caCert);
@@ -100,7 +118,7 @@ public BasicTrustRootIndex(X509Certificate... caCerts) {
 
     @Override public X509Certificate findByIssuerAndSignature(X509Certificate cert) {
       X500Principal issuer = cert.getIssuerX500Principal();
-      List<X509Certificate> subjectCaCerts = subjectToCaCerts.get(issuer);
+      Set<X509Certificate> subjectCaCerts = subjectToCaCerts.get(issuer);
       if (subjectCaCerts == null) return null;
 
       for (X509Certificate caCert : subjectCaCerts) {
@@ -114,5 +132,15 @@ public BasicTrustRootIndex(X509Certificate... caCerts) {
 
       return null;
     }
+
+    @Override public boolean equals(Object other) {
+      if (other == this) return true;
+      return other instanceof BasicTrustRootIndex
+          && ((BasicTrustRootIndex) other).subjectToCaCerts.equals(subjectToCaCerts);
+    }
+
+    @Override public int hashCode() {
+      return subjectToCaCerts.hashCode();
+    }
   }
 }