Update SSL error handling code to account for Subject CN deprecation

chrome/browser/ssl/ssl_error_handler_unittest.cc

Index: chrome/browser/ssl/ssl_error_handler_unittest.cc
diff --git a/chrome/browser/ssl/ssl_error_handler_unittest.cc b/chrome/browser/ssl/ssl_error_handler_unittest.cc
index 19471ae78d789477045d3911fed819458300186c..986fe4d57b8fb4d74f6ccc2624e0f63a9cbeec0f 100644
--- a/chrome/browser/ssl/ssl_error_handler_unittest.cc
+++ b/chrome/browser/ssl/ssl_error_handler_unittest.cc
@@ -201,8 +201,7 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::SetUp();
     SSLErrorHandler::ResetConfigForTesting();
     SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
-    ssl_info_.cert =
-        net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+    ssl_info_.cert = GetCertificate();
     ssl_info_.cert_status = net::CERT_STATUS_COMMON_NAME_INVALID;
     ssl_info_.public_key_hashes.push_back(
         net::HashValue(kCertPublicKeyHashValue));
@@ -217,6 +216,13 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
         base::Callback<void(content::CertificateRequestResultType)>()));
   }
 
+  // Returns a certificate for the test. Virtual to allow derived fixtures to
+  // use a certificate with different characteristics.
+  virtual scoped_refptr<net::X509Certificate> GetCertificate() {

[estark, 2017/04/04 17:22:07]

nit: make protected

[elawrence, 2017/04/04 19:53:24]

I made it private based on usage.

+    return net::ImportCertFromFile(net::GetTestCertsDirectory(),
+                                   "subjectAltName_www_example_com.pem");
+  }
+
   void TearDown() override {
     EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
     error_handler_.reset(nullptr);
@@ -224,6 +230,8 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
+  ~SSLErrorHandlerNameMismatchTest() override {}

[estark, 2017/04/04 17:22:07]

nit: should go at line 199

[elawrence, 2017/04/04 19:53:24]

Done.

+
   TestSSLErrorHandler* error_handler() { return error_handler_.get(); }
   TestSSLErrorHandlerDelegate* delegate() { return delegate_; }
 
@@ -238,6 +246,21 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
   DISALLOW_COPY_AND_ASSIGN(SSLErrorHandlerNameMismatchTest);
 };
 
+// A class to test name mismatch errors, where the certificate lacks a
+// SubjectAltName. Creates an error handler with a name mismatch error.
+class SSLErrorHandlerNameMismatchNoSANTest
+    : public SSLErrorHandlerNameMismatchTest {
+ public:
+  SSLErrorHandlerNameMismatchNoSANTest() {}
+
+  // Return a certificate that contains no SubjectAltName field.
+  scoped_refptr<net::X509Certificate> GetCertificate() override {
+    return net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(SSLErrorHandlerNameMismatchNoSANTest);
+};
+
 // A class to test the captive portal certificate list feature. Creates an error
 // handler with a name mismatch error by default. The error handler can be
 // recreated by calling ResetErrorHandler() with an appropriate cert status.
@@ -575,7 +598,7 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
-                               SSLErrorHandler::WWW_MISMATCH_FOUND, 1);
+                               SSLErrorHandler::WWW_MISMATCH_FOUND_IN_SAN, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
@@ -649,7 +672,7 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
-                               SSLErrorHandler::WWW_MISMATCH_FOUND, 1);
+                               SSLErrorHandler::WWW_MISMATCH_FOUND_IN_SAN, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
@@ -681,11 +704,35 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
-                               SSLErrorHandler::WWW_MISMATCH_FOUND, 1);
+                               SSLErrorHandler::WWW_MISMATCH_FOUND_IN_SAN, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::WWW_MISMATCH_URL_AVAILABLE, 1);
 }
 
+// No suggestions should be requested if certificate lacks a SubjectAltName.
+TEST_F(SSLErrorHandlerNameMismatchNoSANTest,
+       SSLCommonNameMismatchHandlingRequiresSubjectAltName) {
+  base::HistogramTester histograms;
+  EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
+  delegate()->set_suggested_url_exists();
+  error_handler()->StartHandlingError();
+
+  EXPECT_FALSE(delegate()->suggested_url_checked());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(delegate()->ssl_interstitial_shown());
+  EXPECT_FALSE(delegate()->redirected_to_suggested_url());
+
+  histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
+  histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                               SSLErrorHandler::HANDLE_ALL, 1);
+  histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                               SSLErrorHandler::WWW_MISMATCH_FOUND_IN_SAN, 0);
+  histograms.ExpectBucketCount(
+      SSLErrorHandler::GetHistogramNameForTesting(),
+      SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
+}
+
 TEST_F(SSLErrorHandlerNameMismatchTest,
        ShouldShowSSLInterstitialOnInvalidUrlCheckResult) {
   base::HistogramTester histograms;
@@ -710,7 +757,7 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
-                               SSLErrorHandler::WWW_MISMATCH_FOUND, 1);
+                               SSLErrorHandler::WWW_MISMATCH_FOUND_IN_SAN, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::WWW_MISMATCH_URL_NOT_AVAILABLE,
                                1);