Split SdchManager::Dictionary out into separate class/file.

net/base/sdch_manager.cc

Index: net/base/sdch_manager.cc
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index 34fc76af6ecd818a23340eaa90376892a310e8e5..0f9b96dc626c3bb6fbb501fc031de703e531bd14 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -12,7 +12,6 @@
 #include "base/time/default_clock.h"
 #include "base/values.h"
 #include "crypto/sha2.h"
-#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/base/sdch_observer.h"
 #include "net/url_request/url_request_http_job.h"
 
@@ -45,165 +44,6 @@ bool SdchManager::g_sdch_enabled_ = true;
 // static
 bool SdchManager::g_secure_scheme_supported_ = true;
 
-SdchManager::Dictionary::Dictionary(const std::string& dictionary_text,
-                                    size_t offset,
-                                    const std::string& client_hash,
-                                    const std::string& server_hash,
-                                    const GURL& gurl,
-                                    const std::string& domain,
-                                    const std::string& path,
-                                    const base::Time& expiration,
-                                    const std::set<int>& ports)
-    : text_(dictionary_text, offset),
-      client_hash_(client_hash),
-      server_hash_(server_hash),
-      url_(gurl),
-      domain_(domain),
-      path_(path),
-      expiration_(expiration),
-      ports_(ports),
-      clock_(new base::DefaultClock) {
-}
-
-SdchManager::Dictionary::Dictionary(const SdchManager::Dictionary& rhs)
-    : text_(rhs.text_),
-      client_hash_(rhs.client_hash_),
-      server_hash_(rhs.server_hash_),
-      url_(rhs.url_),
-      domain_(rhs.domain_),
-      path_(rhs.path_),
-      expiration_(rhs.expiration_),
-      ports_(rhs.ports_),
-      clock_(new base::DefaultClock) {
-}
-
-SdchManager::Dictionary::~Dictionary() {}
-
-// Security functions restricting loads and use of dictionaries.
-
-// static
-SdchProblemCode SdchManager::Dictionary::CanSet(const std::string& domain,
-                                                const std::string& path,
-                                                const std::set<int>& ports,
-                                                const GURL& dictionary_url) {
-  /*
-  A dictionary is invalid and must not be stored if any of the following are
-  true:
-    1. The dictionary has no Domain attribute.
-    2. The effective host name that derives from the referer URL host name does
-      not domain-match the Domain attribute.
-    3. The Domain attribute is a top level domain.
-    4. The referer URL host is a host domain name (not IP address) and has the
-      form HD, where D is the value of the Domain attribute, and H is a string
-      that contains one or more dots.
-    5. If the dictionary has a Port attribute and the referer URL's port was not
-      in the list.
-  */
-
-  // TODO(jar): Redirects in dictionary fetches might plausibly be problematic,
-  // and hence the conservative approach is to not allow any redirects (if there
-  // were any... then don't allow the dictionary to be set).
-
-  if (domain.empty())
-    return SDCH_DICTIONARY_MISSING_DOMAIN_SPECIFIER;  // Domain is required.
-
-  if (registry_controlled_domains::GetDomainAndRegistry(
-          domain, registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)
-          .empty()) {
-    return SDCH_DICTIONARY_SPECIFIES_TOP_LEVEL_DOMAIN;  // domain was a TLD.
-  }
-
-  if (!Dictionary::DomainMatch(dictionary_url, domain))
-    return SDCH_DICTIONARY_DOMAIN_NOT_MATCHING_SOURCE_URL;
-
-  std::string referrer_url_host = dictionary_url.host();
-  size_t postfix_domain_index = referrer_url_host.rfind(domain);
-  // See if it is indeed a postfix, or just an internal string.
-  if (referrer_url_host.size() == postfix_domain_index + domain.size()) {
-    // It is a postfix... so check to see if there's a dot in the prefix.
-    size_t end_of_host_index = referrer_url_host.find_first_of('.');
-    if (referrer_url_host.npos != end_of_host_index &&
-        end_of_host_index < postfix_domain_index) {
-      return SDCH_DICTIONARY_REFERER_URL_HAS_DOT_IN_PREFIX;
-    }
-  }
-
-  if (!ports.empty() && 0 == ports.count(dictionary_url.EffectiveIntPort()))
-    return SDCH_DICTIONARY_PORT_NOT_MATCHING_SOURCE_URL;
-
-  return SDCH_OK;
-}
-
-SdchProblemCode SdchManager::Dictionary::CanUse(
-    const GURL& target_url) const {
-  /*
-    1. The request URL's host name domain-matches the Domain attribute of the
-      dictionary.
-    2. If the dictionary has a Port attribute, the request port is one of the
-      ports listed in the Port attribute.
-    3. The request URL path-matches the path attribute of the dictionary.
-    4. The request is not an HTTPS request.
-    We can override (ignore) item (4) only when we have explicitly enabled
-    HTTPS support AND the dictionary acquisition scheme matches the target
-     url scheme.
-  */
-  if (!DomainMatch(target_url, domain_))
-    return SDCH_DICTIONARY_FOUND_HAS_WRONG_DOMAIN;
-
-  if (!ports_.empty() && 0 == ports_.count(target_url.EffectiveIntPort()))
-    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PORT_LIST;
-
-  if (path_.size() && !PathMatch(target_url.path(), path_))
-    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PATH;
-
-  if (!SdchManager::secure_scheme_supported() && target_url.SchemeIsSecure())
-    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
-
-  if (target_url.SchemeIsSecure() != url_.SchemeIsSecure())
-    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
-
-  // TODO(jar): Remove overly restrictive failsafe test (added per security
-  // review) when we have a need to be more general.
-  if (!target_url.SchemeIsHTTPOrHTTPS())
-    return SDCH_ATTEMPT_TO_DECODE_NON_HTTP_DATA;
-
-  return SDCH_OK;
-}
-
-// static
-bool SdchManager::Dictionary::PathMatch(const std::string& path,
-                                        const std::string& restriction) {
-  /*  Must be either:
-  1. P2 is equal to P1
-  2. P2 is a prefix of P1 and either the final character in P2 is "/" or the
-      character following P2 in P1 is "/".
-      */
-  if (path == restriction)
-    return true;
-  size_t prefix_length = restriction.size();
-  if (prefix_length > path.size())
-    return false;  // Can't be a prefix.
-  if (0 != path.compare(0, prefix_length, restriction))
-    return false;
-  return restriction[prefix_length - 1] == '/' || path[prefix_length] == '/';
-}
-
-// static
-bool SdchManager::Dictionary::DomainMatch(const GURL& gurl,
-                                          const std::string& restriction) {
-  // TODO(jar): This is not precisely a domain match definition.
-  return gurl.DomainIs(restriction.data(), restriction.size());
-}
-
-bool SdchManager::Dictionary::Expired() const {
-  return clock_->Now() > expiration_;
-}
-
-void SdchManager::Dictionary::SetClockForTesting(
-    scoped_ptr<base::Clock> clock) {
-  clock_ = clock.Pass();
-}
-
 SdchManager::DictionarySet::DictionarySet() {}
 
 SdchManager::DictionarySet::~DictionarySet() {}
@@ -221,7 +61,7 @@ std::string SdchManager::DictionarySet::GetDictionaryClientHashList() const {
   return result;
 }
 
-const SdchManager::Dictionary* SdchManager::DictionarySet::GetDictionary(
+const SdchDictionary* SdchManager::DictionarySet::GetDictionary(
     const std::string& hash) const {
   auto it = dictionaries_.find(hash);
   if (it == dictionaries_.end())
@@ -236,8 +76,7 @@ bool SdchManager::DictionarySet::Empty() const {
 
 void SdchManager::DictionarySet::AddDictionary(
     const std::string& server_hash,
-    const scoped_refptr<base::RefCountedData<SdchManager::Dictionary>>&
-    dictionary) {
+    const scoped_refptr<base::RefCountedData<SdchDictionary>>& dictionary) {
   DCHECK(dictionaries_.end() == dictionaries_.find(server_hash));
 
   dictionaries_[server_hash] = dictionary;
@@ -430,6 +269,8 @@ SdchManager::GetDictionarySet(const GURL& target_url) {
   int count = 0;
   scoped_ptr<SdchManager::DictionarySet> result(new DictionarySet);
   for (const auto& entry: dictionaries_) {
+    if (!secure_scheme_supported() && target_url.SchemeIsSecure())

[Elly Fong-Jones, 2015/04/08 16:25:33]

Is this an unrelated bugfix?

[Randy Smith (Not in Mondays), 2015/04/08 21:06:05]

Not really.  CanUse() used to use the above test, but copying that directly into
SdchDictionary mean that SdchManager depends on SdchDictionary depends on
SdchManager.  Pulling the check up into GetDictionarySet() allows breaking that
dependency loop.

+      continue;
     if (entry.second->data.CanUse(target_url) != SDCH_OK)
       continue;
     if (entry.second->data.Expired())
@@ -458,6 +299,11 @@ SdchManager::GetDictionarySetByHash(
   if (it == dictionaries_.end())
     return result.Pass();
 
+  if (!SdchManager::secure_scheme_supported() && target_url.SchemeIsSecure()) {
+    *problem_code = SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
+    return result.Pass();
+  }
+
   *problem_code = it->second->data.CanUse(target_url);
   if (*problem_code != SDCH_OK)
     return result.Pass();
@@ -587,18 +433,18 @@ SdchProblemCode SdchManager::AddSdchDictionary(
   if (rv != SDCH_OK)
     return rv;
 
-  rv = Dictionary::CanSet(domain, path, ports, dictionary_url_normalized);
+  rv = SdchDictionary::CanSet(domain, path, ports, dictionary_url_normalized);
   if (rv != SDCH_OK)
     return rv;
 
   UMA_HISTOGRAM_COUNTS("Sdch3.Dictionary size loaded", dictionary_text.size());
   DVLOG(1) << "Loaded dictionary with client hash " << client_hash
            << " and server hash " << server_hash;
-  Dictionary dictionary(dictionary_text, header_end + 2, client_hash,
-                        server_hash, dictionary_url_normalized, domain, path,
-                        expiration, ports);
+  SdchDictionary dictionary(dictionary_text, header_end + 2, client_hash,
+                            server_hash, dictionary_url_normalized, domain,
+                            path, expiration, ports);
   dictionaries_[server_hash] =
-      new base::RefCountedData<Dictionary>(dictionary);
+      new base::RefCountedData<SdchDictionary>(dictionary);
   if (server_hash_p)
     *server_hash_p = server_hash;