Simple: Move PVer4 related code from util.* to v4_protocol_manager_util.*

components/safe_browsing_db/v4_protocol_manager_util.h

 // Copyright 2016 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.
 
 #ifndef COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_
 #define COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_
 
 // A class that implements the stateless methods used by the GetHashUpdate and
 // GetFullHash stubby calls made by Chrome using the SafeBrowsing V4 protocol.
 
 #include <ostream>
 #include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/hash.h"
+#include "base/strings/string_piece.h"
 #include "components/safe_browsing_db/safebrowsing.pb.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 namespace net {
 class HttpRequestHeaders;
 }  // namespace net
 
 namespace safe_browsing {
 
+// The size of the hash prefix, in bytes. It should be between 4 to 32 (full
+// hash).
+typedef size_t PrefixSize;
+
+// A hash prefix sent by the SafeBrowsing PVer4 service.
+typedef std::string HashPrefix;
+
+// The sorted list of hash prefixes.

[Nathan Parker, 2016/08/05 18:07:39]

Is this needed by anything other than the v4LocalDBManager?  It could go there
instead, since the big-string-of-concatenated-hashes is an implementation
detail.

That might apply to PrefixSize as well, not sure.  HashPrefix has a built-in
size.

[vakh (use Gerrit instead), 2016/08/05 18:25:48]

Currently it is being used by the Store only.
I moved them all here so that they can be defined together. My assumption is
that anyone looking into this code would need to understand why we need
HashPrefix, HashPrefixes, and PrefixSize.

But I also think it is just my assumption and you have a good argument about why
HashPrefixes and PrefixSize don't belong here. Moving them to V4Store.

+typedef std::string HashPrefixes;
+
+// A full SHA256 hash.
+typedef HashPrefix FullHash;
+
 typedef FetchThreatListUpdatesRequest::ListUpdateRequest ListUpdateRequest;
 typedef FetchThreatListUpdatesResponse::ListUpdateResponse ListUpdateResponse;
 
 // Config passed to the constructor of a V4 protocol manager.
 struct V4ProtocolConfig {
   // The safe browsing client name sent in each request.
   std::string client_name;
 
   // Current product version sent in each request.
   std::string version;
@@ skipping 27 matching lines @@
   bool operator==(const UpdateListIdentifier& other) const;
   bool operator!=(const UpdateListIdentifier& other) const;
   size_t hash() const;
 
  private:
   UpdateListIdentifier();
 };
 
 std::ostream& operator<<(std::ostream& os, const UpdateListIdentifier& id);
 
+#if defined(OS_WIN)
+#define PLATFORM_TYPE WINDOWS_PLATFORM
+#elif defined(OS_LINUX)
+#define PLATFORM_TYPE LINUX_PLATFORM
+#elif defined(OS_MACOSX)
+#define PLATFORM_TYPE OSX_PLATFORM
+#else
+// This should ideally never compile but it is getting compiled on Android.
+// See: https://bugs.chromium.org/p/chromium/issues/detail?id=621647
+// TODO(vakh): Once that bug is fixed, this should be removed. If we leave
+// the platform_type empty, the server won't recognize the request and
+// return an error response which will pollute our UMA metrics.
+#define PLATFORM_TYPE LINUX_PLATFORM
+#endif
+
+const UpdateListIdentifier kUrlMalware(PLATFORM_TYPE, URL, MALWARE_THREAT);

[Nathan Parker, 2016/08/05 18:07:39]

nit: The names could include "Id" or "ListId", so it doesn't look like it's a
URL.

Will we need one for API blacklist as well (for hash-checking)?

[vakh (use Gerrit instead), 2016/08/05 18:25:48]

Done.
Yes. Will add it later.

+const UpdateListIdentifier kUrlSoceng(PLATFORM_TYPE,
+                                      URL,
+                                      SOCIAL_ENGINEERING_PUBLIC);
+
 // The set of interesting lists and ASCII filenames for their hash prefix
 // stores. The stores are created inside the user-data directory.
 // For instance, the UpdateListIdentifier could be for URL expressions for UwS
 // on Windows platform, and the corresponding file on disk could be named:
 // "uws_win_url.store"
 // TODO(vakh): Find the canonical place where these are defined and update the
 // comment to point to that place.
 typedef base::hash_map<UpdateListIdentifier, std::string> StoreFileNameMap;
 
 // Represents the state of each store.
@@ skipping 27 matching lines @@
   ALREADY_PENDING_ERROR = 6,
 
   // Memory space for histograms is determined by the max.  ALWAYS
   // ADD NEW VALUES BEFORE THIS ONE.
   OPERATION_RESULT_MAX = 7
 };
 
 // A class that provides static methods related to the Pver4 protocol.
 class V4ProtocolManagerUtil {
  public:
-  // Record HTTP response code when there's no error in fetching an HTTP
-  // request, and the error code, when there is.
-  // |metric_name| is the name of the UMA metric to record the response code or
-  // error code against, |status| represents the status of the HTTP request, and
-  // |response code| represents the HTTP response code received from the server.
-  static void RecordHttpResponseOrErrorCode(const char* metric_name,
-                                            const net::URLRequestStatus& status,
-                                            int response_code);
+  // Canonicalizes url as per Google Safe Browsing Specification.
+  // See: https://developers.google.com/safe-browsing/v4/urls-hashing
+  static void CanonicalizeUrl(const GURL& url,
+                              std::string* canonicalized_hostname,
+                              std::string* canonicalized_path,
+                              std::string* canonicalized_query);
+
+  // This method returns the host suffix combinations from the hostname in the
+  // URL, as described here:
+  // https://developers.google.com/safe-browsing/v4/urls-hashing
+  static void GenerateHostVariantsToCheck(const std::string& host,
+                                          std::vector<std::string>* hosts);
+
+  // This method returns the path prefix combinations from the path in the
+  // URL, as described here:
+  // https://developers.google.com/safe-browsing/v4/urls-hashing
+  static void GeneratePathVariantsToCheck(const std::string& path,
+                                          const std::string& query,
+                                          std::vector<std::string>* paths);
 
   // Generates a Pver4 request URL and sets the appropriate header values.
   // |request_base64| is the serialized request protocol buffer encoded in
   // base 64.
   // |method_name| is the name of the method to call, as specified in the proto,
   // |config| is an instance of V4ProtocolConfig that stores the client config,
   // |gurl| is set to the value of the PVer4 request URL,
   // |headers| is populated with the appropriate header values.
   static void GetRequestUrlAndHeaders(const std::string& request_base64,
                                       const std::string& method_name,
                                       const V4ProtocolConfig& config,
                                       GURL* gurl,
                                       net::HttpRequestHeaders* headers);
 
   // Worker function for calculating the backoff times.
   // |multiplier| is doubled for each consecutive error after the
   // first, and |error_count| is incremented with each call.
   static base::TimeDelta GetNextBackOffInterval(size_t* error_count,
                                                 size_t* multiplier);
 
+  // Record HTTP response code when there's no error in fetching an HTTP
+  // request, and the error code, when there is.
+  // |metric_name| is the name of the UMA metric to record the response code or
+  // error code against, |status| represents the status of the HTTP request, and
+  // |response code| represents the HTTP response code received from the server.
+  static void RecordHttpResponseOrErrorCode(const char* metric_name,

[Nathan Parker, 2016/08/05 18:07:39]

Is this specific to the V4ProtocolManager? Maybe there's not a better place for
it.  I just want to make sure we're not over-namespacing these things.

[vakh (use Gerrit instead), 2016/08/05 18:25:48]

It records the information about the response received from the server and is
only used in both protocol manager classes (update/get_hash) so I think this is
the right place for this method.

+                                            const net::URLRequestStatus& status,
+                                            int response_code);
+
+  // Generate the set of FullHashes to check for |url|.
+  static void UrlToFullHashes(const GURL& url,
+                              base::hash_set<FullHash>* full_hashes);
+
  private:
   V4ProtocolManagerUtil(){};
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4ProtocolManagerUtilTest,
-                           TestBackOffLogic);
-  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4ProtocolManagerUtilTest,
+  FRIEND_TEST_ALL_PREFIXES(V4ProtocolManagerUtilTest, TestBackOffLogic);
+  FRIEND_TEST_ALL_PREFIXES(V4ProtocolManagerUtilTest,
                            TestGetRequestUrlAndUpdateHeaders);
+  FRIEND_TEST_ALL_PREFIXES(V4ProtocolManagerUtilTest, UrlParsing);
+  FRIEND_TEST_ALL_PREFIXES(V4ProtocolManagerUtilTest, CanonicalizeUrl);
 
   // Composes a URL using |prefix|, |method| (e.g.: encodedFullHashes).
   // |request_base64|, |client_id|, |version| and |key_param|. |prefix|
   // should contain the entire url prefix including scheme, host and path.
   static std::string ComposeUrl(const std::string& prefix,
                                 const std::string& method,
                                 const std::string& request_base64,
                                 const std::string& key_param);
 
   // Sets the HTTP headers expected by a standard PVer4 request.
   static void UpdateHeaders(net::HttpRequestHeaders* headers);
 
+  // Given a URL, returns all the hosts we need to check.  They are returned
+  // in order of size (i.e. b.c is first, then a.b.c).
+  static void GenerateHostsToCheck(const GURL& url,
+                                   std::vector<std::string>* hosts);
+
+  // Given a URL, returns all the paths we need to check.
+  static void GeneratePathsToCheck(const GURL& url,
+                                   std::vector<std::string>* paths);
+
+  // Given a URL, returns all the patterns we need to check.
+  static void GeneratePatternsToCheck(const GURL& url,
+                                      std::vector<std::string>* urls);
+
+  static std::string RemoveConsecutiveChars(base::StringPiece str,
+                                            const char c);
+
   DISALLOW_COPY_AND_ASSIGN(V4ProtocolManagerUtil);
 };
 
 }  // namespace safe_browsing
 
 namespace std {
 template <>
 struct hash<safe_browsing::UpdateListIdentifier> {
   std::size_t operator()(const safe_browsing::UpdateListIdentifier& s) const {
     return s.hash();
   }
 };
 }
 
 #endif  // COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_