Update SSL error handling code to account for Subject CN deprecation

components/ssl_errors/error_info.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/ssl_errors/error_info.h"
 
 #include <stddef.h>
 
 #include "base/i18n/message_formatter.h"
 #include "base/macros.h"
@@ skipping 14 matching lines @@
                      const base::string16& short_description)
     : details_(details), short_description_(short_description) {}
 
 // static
 ErrorInfo ErrorInfo::CreateError(ErrorType error_type,
                                  net::X509Certificate* cert,
                                  const GURL& request_url) {
   base::string16 details, short_description;
   switch (error_type) {
     case CERT_COMMON_NAME_INVALID: {
-      // If the certificate contains multiple DNS names, we choose the most
-      // representative one -- either the DNS name that's also in the subject
-      // field, or the first one.  If this heuristic turns out to be
-      // inadequate, we can consider choosing the DNS name that is the
-      // "closest match" to the host name in the request URL, or listing all
-      // the DNS names with an HTML <ul>.
       std::vector<std::string> dns_names;
-      cert->GetDNSNames(&dns_names);
-      DCHECK(!dns_names.empty());
+      cert->GetSubjectAltName(&dns_names, NULL);

[Ryan Sleevi, 2017/03/31 14:51:34]

nullptr ;)

[elawrence, 2017/03/31 16:09:41]

Done.

+
       size_t i = 0;
-      for (; i < dns_names.size(); ++i) {
-        if (dns_names[i] == cert->subject().common_name)
-          break;
+      if (!dns_names.empty()) {
+        // If the certificate contains multiple DNS names, we choose the most
+        // representative one -- either the DNS name that's also in the subject
+        // field, or the first one. If this heuristic turns out to be
+        // inadequate, we can consider choosing the DNS name that is the
+        // "closest match" to the host name in the request URL, or listing all
+        // the DNS names with an HTML <ul>.
+        for (; i < dns_names.size(); ++i) {
+          if (dns_names[i] == cert->subject().common_name)
+            break;
+        }
+        if (i == dns_names.size())
+          i = 0;
+      } else {
+        // The certificate had no DNS names, use an empty string for display.
+        dns_names.push_back("\"missing_subjectAltName\"");

[Ryan Sleevi, 2017/03/31 14:51:34]

readability suggestion: It's usually easier to put the 'error' handling first
int he condition, and then jump to the bulk of the logic, as it helps group
together the flow

That is

if (dns_names.empty()) {
  dns_names.push_back(...)
} else {
  ...

[elawrence, 2017/03/31 16:09:41]

Swapped. (Heh. That's how I had it originally, then I flipped it because I felt
like it was better to be optimistic.)

       }
-      if (i == dns_names.size())
-        i = 0;
+
       details = l10n_util::GetStringFUTF16(
           IDS_CERT_ERROR_COMMON_NAME_INVALID_DETAILS,
           UTF8ToUTF16(request_url.host()),
           net::EscapeForHTML(UTF8ToUTF16(dns_names[i])));
       short_description = l10n_util::GetStringUTF16(
           IDS_CERT_ERROR_COMMON_NAME_INVALID_DESCRIPTION);
       break;
     }
     case CERT_DATE_INVALID:
       if (cert->HasExpired()) {
@@ skipping 191 matching lines @@
 
   for (size_t i = 0; i < arraysize(kErrorFlags); ++i) {
     if ((cert_status & kErrorFlags[i]) && errors) {
       errors->push_back(
           ErrorInfo::CreateError(kErrorTypes[i], cert.get(), url));
     }
   }
 }
 
 }  // namespace ssl_errors