Reduce buggy usage of the registry controlled domain service.

extensions/common/manifest_handlers/externally_connectable.cc

 // Copyright 2014 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 "extensions/common/manifest_handlers/externally_connectable.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
 
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/crx_file/id_util.h"
 #include "extensions/common/api/extensions_manifest_types.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/permissions_parser.h"
 #include "extensions/common/permissions/api_permission_set.h"
 #include "extensions/common/url_pattern.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/base/url_util.h"
 #include "url/gurl.h"
 
 namespace rcd = net::registry_controlled_domains;
 
 namespace extensions {
 
 namespace externally_connectable_errors {
 const char kErrorInvalidMatchPattern[] = "Invalid match pattern '*'";
 const char kErrorInvalidId[] = "Invalid ID '*'";
 const char kErrorNothingSpecified[] =
@@ skipping 99 matching lines @@
       if (pattern.host().empty()) {
         // Warning not error for forwards compatibility.
         install_warnings->push_back(
             InstallWarning(ErrorUtils::FormatErrorMessage(
                                errors::kErrorWildcardHostsNotAllowed, *it),
                            keys::kExternallyConnectable,
                            *it));
         continue;
       }
 
+      url::CanonHostInfo host_info;

[brettw, 2016/10/19 20:16:36]

This is changed around a bit because it wanted to check for the behavior of
"invalid host" and I didn't want to change the semantics. So it explicitly
canonicalizes first, checks for success, then uses the bool version later for
clarity. This could have used the Buggy* version without being buggy since it
only cares about 0, npos, and >0, but that seemed potentially confusing.

+      std::string canonical_host =
+          net::CanonicalizeHost(pattern.host(), &host_info);
+      if (canonical_host.empty()) {
+        // CanonicalizeHost returns empty string on error. The URL parsing
+        // combined with host().empty() should have caught this above.
+        NOTREACHED() << *it;
+        *error = ErrorUtils::FormatErrorMessageUTF16(
+            errors::kErrorInvalidMatchPattern, *it);
+        return std::unique_ptr<ExternallyConnectableInfo>();
+      }
+
       // Wildcards on subdomains of a TLD are not allowed.
-      size_t registry_length = rcd::GetRegistryLength(
-          pattern.host(),
+      bool has_registry = rcd::HostHasRegistryControlledDomain(
+          canonical_host,
           // This means that things that look like TLDs - the foobar in
           // http://google.foobar - count as TLDs.
           rcd::INCLUDE_UNKNOWN_REGISTRIES,
           // This means that effective TLDs like appspot.com count as TLDs;
           // codereview.appspot.com and evil.appspot.com are different.
           rcd::INCLUDE_PRIVATE_REGISTRIES);
 
-      if (registry_length == std::string::npos) {
-        // The URL parsing combined with host().empty() should have caught this.
-        NOTREACHED() << *it;
-        *error = ErrorUtils::FormatErrorMessageUTF16(
-            errors::kErrorInvalidMatchPattern, *it);
-        return std::unique_ptr<ExternallyConnectableInfo>();
-      }
-
       // Broad match patterns like "*.com", "*.co.uk", and even "*.appspot.com"
       // are not allowed. However just "appspot.com" is ok.
-      if (registry_length == 0 && pattern.match_subdomains()) {
+      if (!has_registry && pattern.match_subdomains()) {
         // Warning not error for forwards compatibility.
         install_warnings->push_back(
             InstallWarning(ErrorUtils::FormatErrorMessage(
                                errors::kErrorTopLevelDomainsNotAllowed,
                                pattern.host().c_str(),
                                *it),
                            keys::kExternallyConnectable,
                            *it));
         continue;
       }
@@ skipping 49 matching lines @@
 }
 
 bool ExternallyConnectableInfo::IdCanConnect(const std::string& id) {
   if (all_ids)
     return true;
   DCHECK(base::STLIsSorted(ids));
   return std::binary_search(ids.begin(), ids.end(), id);
 }
 
 }  // namespace extensions