v4_update_protocol_manager: Basic implementation with TODOs

components/safe_browsing_db/v4_update_protocol_manager.cc

+// 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.
+
+#include "components/safe_browsing_db/v4_update_protocol_manager.h"
+
+#include <utility>
+
+#include "base/base64.h"
+#include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/timer/timer.h"
+#include "components/safe_browsing_db/safebrowsing.pb.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context_getter.h"
+
+using base::Time;
+using base::TimeDelta;
+
+namespace {
+
+// Enumerate parsing failures for histogramming purposes.  DO NOT CHANGE
+// THE ORDERING OF THESE VALUES.
+enum ParseResultType {
+  // Error parsing the protocol buffer from a string.
+  PARSE_FROM_STRING_ERROR = 0,
+
+  // No platform_type set in the response.
+  NO_PLATFORM_TYPE_ERROR = 1,
+
+  // No threat_entry_type set in the response.
+  NO_THREAT_ENTRY_TYPE_ERROR = 2,
+
+  // No threat_type set in the response.
+  NO_THREAT_TYPE_ERROR = 3,
+
+  // No state set in the response for one or more lists.
+  NO_STATE_ERROR = 4,
+
+  // Memory space for histograms is determined by the max.  ALWAYS
+  // ADD NEW VALUES BEFORE THIS ONE.
+  PARSE_RESULT_TYPE_MAX = 5
+};
+
+// Record parsing errors of an update result.
+void RecordParseUpdateResult(ParseResultType result_type) {
+  UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.ParseV4UpdateResult", result_type,
+                            PARSE_RESULT_TYPE_MAX);
+}
+
+}  // namespace
+
+namespace safe_browsing {
+
+const char kUmaV4UpdateResponseMetricName[] =

[Nathan Parker, 2016/03/22 23:57:55]

It's pretty standard to just put the metric name's string literal directly in
the call, if it's used in only one place.

[vakh (use Gerrit instead), 2016/03/24 22:34:24]

Done.

+    "SafeBrowsing.V4UpdateHttpResponseOrErrorCode";
+
+// The default V4UpdateProtocolManagerFactory.
+class V4UpdateProtocolManagerFactoryImpl
+    : public V4UpdateProtocolManagerFactory {
+ public:
+  V4UpdateProtocolManagerFactoryImpl() {}
+  ~V4UpdateProtocolManagerFactoryImpl() override {}
+  V4UpdateProtocolManager* CreateProtocolManager(
+      net::URLRequestContextGetter* request_context_getter,
+      const V4ProtocolConfig& config) override {
+    return new V4UpdateProtocolManager(request_context_getter, config);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(V4UpdateProtocolManagerFactoryImpl);
+};
+
+// V4UpdateProtocolManager implementation --------------------------------
+
+// static
+V4UpdateProtocolManagerFactory* V4UpdateProtocolManager::factory_ = NULL;
+
+// static
+V4UpdateProtocolManager* V4UpdateProtocolManager::Create(
+    net::URLRequestContextGetter* request_context_getter,
+    const V4ProtocolConfig& config) {
+  if (!factory_)
+    factory_ = new V4UpdateProtocolManagerFactoryImpl();

[Nathan Parker, 2016/03/22 23:57:55]

Do you need this Impl class at all?  Another way would be to return a new V4UPM
right here if !factory_

[vakh (use Gerrit instead), 2016/03/24 22:34:24]

This would be useful for testing using DI. The old implementation uses it that
way.
Leaving it as-is for now but I'd not mind removing the factory if you feel
strongly.

+  return factory_->CreateProtocolManager(request_context_getter, config);
+}
+
+void V4UpdateProtocolManager::ResetUpdateErrors() {
+  update_error_count_ = 0;
+  update_back_off_mult_ = 1;
+}
+
+V4UpdateProtocolManager::V4UpdateProtocolManager(
+    net::URLRequestContextGetter* request_context_getter,
+    const V4ProtocolConfig& config)
+    : update_error_count_(0),
+      update_back_off_mult_(1),
+      next_update_time_(Time::FromDoubleT(0)),
+      config_(config),
+      request_context_getter_(request_context_getter),
+      url_fetcher_id_(0),
+      update_request_pending_(false) {}
+
+// static
+void V4UpdateProtocolManager::RecordUpdateResult(
+    OperationResultType result_type) {
+  UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.V4UpdateResult", result_type,
+                            V4ProtocolManagerUtil::OPERATION_RESULT_TYPE_MAX);
+}
+
+V4UpdateProtocolManager::~V4UpdateProtocolManager() {
+}
+
+std::string V4UpdateProtocolManager::GetUpdateRequest(
+    const base::hash_set<const UpdateListIdentifier*>& lists_to_update,
+    const base::hash_map<const UpdateListIdentifier*, const std::string&>&
+        current_list_states) {
+  // Build the request. Client info and client states are not added to the
+  // request protocol buffer. Client info is passed as params in the url.
+  FetchThreatListUpdatesRequest request;
+  for (const UpdateListIdentifier* list_to_update : lists_to_update) {
+    ListUpdateRequest* list_update_request = request.add_list_update_requests();
+    list_update_request->set_platform_type(list_to_update->platform_type);
+    list_update_request->set_threat_entry_type(
+        list_to_update->threat_entry_type);
+    list_update_request->set_threat_type(list_to_update->threat_type);
+
+    // If the current state of the list is available, add that to the proto.
+    base::hash_map<const UpdateListIdentifier*,
+                   const std::string&>::const_iterator list_iter =
+        current_list_states.find(list_to_update);
+    if (list_iter != current_list_states.end()) {
+      list_update_request->set_state(list_iter->second);
+    }
+  }
+
+  // Serialize and Base64 encode.
+  std::string req_data, req_base64;
+  request.SerializeToString(&req_data);
+  base::Base64Encode(req_data, &req_base64);
+
+  return req_base64;
+}
+
+bool V4UpdateProtocolManager::ParseUpdateResponse(
+    const std::string& data,
+    std::vector<ListUpdateResponse>* list_update_responses) {
+  FetchThreatListUpdatesResponse response;
+
+  if (!response.ParseFromString(data)) {
+    RecordParseUpdateResult(PARSE_FROM_STRING_ERROR);
+    return false;
+  }
+
+  if (response.has_minimum_wait_duration()) {
+    // Seconds resolution is good enough so we ignore the nanos field.
+    next_update_time_ =

[Nathan Parker, 2016/03/22 23:57:55]

If it doesn't have a min wait duration, does next_update_time_ remain as ~now?

[vakh (use Gerrit instead), 2016/03/24 22:34:24]

Yes, we don't update it if the response doesn't contain min_wait_duration.

[Nathan Parker, 2016/03/24 22:48:05]

ok, so then assumption is that the caller to GetUpdate() would only invoke it at
a reasonable rate.

[vakh (use Gerrit instead), 2016/03/25 02:42:37]

Not really. The class stores next_update_time_ which tells it until when not to
send another update request.

+        Time::Now() + base::TimeDelta::FromSeconds(
+                          response.minimum_wait_duration().seconds());
+  }
+
+  // TODO(vakh): Do something useful with this response.
+  for (const ListUpdateResponse& list_update_response :
+       response.list_update_responses()) {
+    if (!list_update_response.has_platform_type()) {
+      RecordParseUpdateResult(NO_PLATFORM_TYPE_ERROR);
+    } else if (!list_update_response.has_threat_entry_type()) {
+      RecordParseUpdateResult(NO_THREAT_ENTRY_TYPE_ERROR);
+    } else if (!list_update_response.has_threat_type()) {
+      RecordParseUpdateResult(NO_THREAT_TYPE_ERROR);
+    } else if (!list_update_response.has_new_client_state()) {
+      RecordParseUpdateResult(NO_STATE_ERROR);
+    } else {
+      list_update_responses->push_back(list_update_response);
+    }
+  }
+  return true;
+}
+
+void V4UpdateProtocolManager::GetUpdates(
+    const base::hash_set<const UpdateListIdentifier*>& lists_to_update,
+    const base::hash_map<const UpdateListIdentifier*, const std::string&>&
+        current_list_states,
+    UpdateCallback callback) {
+  DCHECK(CalledOnValidThread());
+
+  // If an update request is already pending, return an empty result.
+  if (update_request_pending_) {
+    RecordUpdateResult(V4ProtocolManagerUtil::ALREADY_PENDING_ERROR);
+    std::vector<ListUpdateResponse> list_update_responses;
+    callback.Run(list_update_responses);
+    return;
+  }
+
+  // We need to wait the minimum waiting duration, and if we are in backoff,
+  // we need to check if we're past the next allowed time. If we are, we can
+  // proceed with the request. If not, we are required to return empty results.
+  if (Time::Now() <= next_update_time_) {
+    if (update_error_count_) {
+      RecordUpdateResult(V4ProtocolManagerUtil::BACKOFF_ERROR);
+    } else {
+      RecordUpdateResult(V4ProtocolManagerUtil::MIN_WAIT_DURATION_ERROR);
+    }
+    std::vector<ListUpdateResponse> list_update_responses;
+    callback.Run(list_update_responses);
+    return;
+  }
+
+  std::string req_base64 =
+      GetUpdateRequest(lists_to_update, current_list_states);
+  GURL update_url = GetUpdateUrl(req_base64);
+
+  net::URLFetcher* fetcher =

[Nathan Parker, 2016/03/22 23:57:55]

nit: You could put this directly into fetcher_

[vakh (use Gerrit instead), 2016/03/24 22:34:24]

Done.

+      net::URLFetcher::Create(url_fetcher_id_++, update_url,
+                              net::URLFetcher::GET, this)
+          .release();
+  update_request_pending_ = true;
+  callback_ = callback;
+  request_.reset(fetcher);
+
+  fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
+  fetcher->SetRequestContext(request_context_getter_.get());
+  fetcher->Start();
+}
+
+// net::URLFetcherDelegate implementation ----------------------------------
+
+// SafeBrowsing request responses are handled here.
+void V4UpdateProtocolManager::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  DCHECK(CalledOnValidThread());
+
+  int response_code = source->GetResponseCode();
+  net::URLRequestStatus status = source->GetStatus();
+  V4ProtocolManagerUtil::RecordHttpResponseOrErrorCode(
+      kUmaV4UpdateResponseMetricName, status, response_code);
+
+  std::vector<ListUpdateResponse> list_update_responses;
+  if (status.is_success() && response_code == net::HTTP_OK) {
+    RecordUpdateResult(V4ProtocolManagerUtil::STATUS_200);
+    ResetUpdateErrors();
+    std::string data;
+    source->GetResponseAsString(&data);
+    if (!ParseUpdateResponse(data, &list_update_responses)) {
+      list_update_responses.clear();
+      RecordUpdateResult(V4ProtocolManagerUtil::PARSE_ERROR);
+    }
+  } else {
+    HandleUpdateError(Time::Now());
+
+    DVLOG(1) << "SafeBrowsing GetEncodedUpdates request for: "
+             << source->GetURL() << " failed with error: " << status.error()
+             << " and response code: " << response_code;
+
+    if (status.status() == net::URLRequestStatus::FAILED) {
+      RecordUpdateResult(V4ProtocolManagerUtil::NETWORK_ERROR);
+    } else {
+      RecordUpdateResult(V4ProtocolManagerUtil::HTTP_ERROR);
+    }
+  }
+
+  // Invoke the callback with list_update_responses, even if there was a parse
+  // error or an error response code (in which case list_update_responses will
+  // be empty). The caller can't be blocked indefinitely.
+  callback_.Run(list_update_responses);
+
+  update_request_pending_ = false;
+}
+
+void V4UpdateProtocolManager::HandleUpdateError(const Time& now) {
+  DCHECK(CalledOnValidThread());
+  base::TimeDelta next = V4ProtocolManagerUtil::GetNextBackOffInterval(
+      &update_error_count_, &update_back_off_mult_);
+  next_update_time_ = now + next;
+}
+
+GURL V4UpdateProtocolManager::GetUpdateUrl(
+    const std::string& req_base64) const {
+  return V4ProtocolManagerUtil::GetRequestUrl(req_base64, "encodedUpdates",
+                                              config_);
+}
+
+}  // namespace safe_browsing