G-services settings v3 implementation

google_apis/gcm/engine/gservices_settings.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 "google_apis/gcm/engine/gservices_settings.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
+#include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 
 namespace {
 // The expected time in seconds between periodic checkins.
 const char kCheckinIntervalKey[] = "checkin_interval";
 // The override URL to the checkin server.
 const char kCheckinURLKey[] = "checkin_url";
 // The MCS machine name to connect to.
 const char kMCSHostnameKey[] = "gcm_hostname";
 // The MCS port to connect to.
 const char kMCSSecurePortKey[] = "gcm_secure_port";
 // The URL to get MCS registration IDs.
 const char kRegistrationURLKey[] = "gcm_registration_url";
 
 const int64 kDefaultCheckinInterval = 2 * 24 * 60 * 60;  // seconds = 2 days.
 const int64 kMinimumCheckinInterval = 12 * 60 * 60;      // seconds = 12 hours.
 const char kDefaultCheckinURL[] = "https://android.clients.google.com/checkin";
 const char kDefaultMCSHostname[] = "mtalk.google.com";
 const int kDefaultMCSMainSecurePort = 5228;
 const int kDefaultMCSFallbackSecurePort = 443;
 const char kDefaultRegistrationURL[] =
     "https://android.clients.google.com/c2dm/register3";
+// Settings that are to be deleted are marked with this prefix in checkin
+// response.
+const char kDeleteSettingPrefix[] = "delete_";
+// Settings digest starts with verison number followed by '-'.
+const char kDigestVersionPrefix[] = "1-";
 const char kMCSEnpointTemplate[] = "https://%s:%d";
+const int kMaxSecurePort = 65535;
 
 std::string MakeMCSEndpoint(const std::string& mcs_hostname, int port) {
   return base::StringPrintf(kMCSEnpointTemplate, mcs_hostname.c_str(), port);
 }
 
+bool VerifyCheckinInterval(
+    const gcm::GServicesSettings::SettingsMap& settings) {
+  gcm::GServicesSettings::SettingsMap::const_iterator iter =
+      settings.find(kCheckinIntervalKey);
+  // Default settings are only present in the map when overwritten.

[jianli, 2014/05/19 17:52:27]

Comment is a bit hard to understand. Do you mean: No need to verify if the
setting is not present.

[fgorski, 2014/05/19 20:25:39]

Done.

+  if (iter == settings.end())

[Nicolas Zea, 2014/05/19 17:49:48]

It would be useful to have some helper method that can easily tell someone
reading this code whether a particular setting is a default setting or not.

Then, maybe your code would be more like:
cur_Setting = kCheckinIntervalKey.
if (iter == settings.end())
  return IsDefaultSetting(cur_setting)

possibly even in a loop? (define an array of settings keys to iterate over)

[fgorski, 2014/05/19 20:25:39]

I added CanBeOmitted method with a documentation. I agree that having the method
adds to the readability and lists all of the settings that can be omitted in a
single place, but I given that verification goes deeper than that, I don't think
we can put it in a loop right away.

Please check it out and let me know what you think.

+    return true;
+
+  int64 checkin_interval = kMinimumCheckinInterval;
+  if (!base::StringToInt64(iter->second, &checkin_interval)) {
+    DVLOG(1) << "Failed to parse checkin interval: " << iter->second;
+    return false;
+  }
+  if (checkin_interval == std::numeric_limits<int64>::max()) {
+    DVLOG(1) << "Checkin interval is too big: " << checkin_interval;
+    return false;
+  }
+  if (checkin_interval < kMinimumCheckinInterval) {
+    DVLOG(1) << "Checkin interval: " << checkin_interval
+             << " is less than allowed minimum: " << kMinimumCheckinInterval;
+  }
+
+  return true;
+}
+
+bool VerifyMCSEndpoint(const gcm::GServicesSettings::SettingsMap& settings) {
+  // Default settings are only present in the map when overwritten.
+  std::string mcs_hostname = kDefaultMCSHostname;
+  gcm::GServicesSettings::SettingsMap::const_iterator iter =
+      settings.find(kMCSHostnameKey);
+  if (iter != settings.end())

[jianli, 2014/05/19 17:52:27]

This seems to be consistent with other Verify methods.

[fgorski, 2014/05/19 20:25:39]

I've updated it to be more consistent, but it is a bit more tricky to verify mcs
endpoints, as they come as a hostname + port couple, where any one could be
missing.

If you provide a hostname, you are affecting 2 settings: main and fallback mcs
endpoint.

Hence the settings related to mcs connection have a bit different structure.

+    mcs_hostname = iter->second;
+
+  if (mcs_hostname.empty()) {
+    DVLOG(1) << "Empty MCS hostname provided.";
+    return false;
+  }
+
+  int mcs_secure_port = 0;
+  iter = settings.find(kMCSSecurePortKey);
+  if (iter == settings.end()) {
+    mcs_secure_port = kDefaultMCSMainSecurePort;
+  } else if (!base::StringToInt(iter->second, &mcs_secure_port)) {
+    DVLOG(1) << "Failed to parse MCS secure port: " << iter->second;
+    return false;
+  }
+
+  if (mcs_secure_port < 0 || mcs_secure_port > kMaxSecurePort) {
+    DVLOG(1) << "Incorrect port value: " << mcs_secure_port;
+    return false;
+  }
+
+  GURL mcs_main_endpoint(MakeMCSEndpoint(mcs_hostname, mcs_secure_port));
+  if (!mcs_main_endpoint.is_valid()) {
+    DVLOG(1) << "Invalid main MCS endpoint: "
+             << mcs_main_endpoint.possibly_invalid_spec();
+    return false;
+  }
+  GURL mcs_fallback_endpoint(
+      MakeMCSEndpoint(mcs_hostname, kDefaultMCSFallbackSecurePort));
+  if (!mcs_fallback_endpoint.is_valid()) {
+    DVLOG(1) << "Invalid fallback MCS endpoint: "
+             << mcs_fallback_endpoint.possibly_invalid_spec();
+    return false;
+  }
+
+  return true;
+}
+
+bool VerifyCheckinURL(const gcm::GServicesSettings::SettingsMap& settings) {
+  gcm::GServicesSettings::SettingsMap::const_iterator iter =
+      settings.find(kCheckinURLKey);
+  // Default settings are only present in the map when overwritten.
+  if (iter == settings.end())
+    return true;
+
+  GURL checkin_url(iter->second);
+  if (!checkin_url.is_valid()) {
+    DVLOG(1) << "Invalid checkin URL provided: " << iter->second;
+    return false;
+  }
+
+  return true;
+}
+
+bool VerifyRegistrationURL(
+    const gcm::GServicesSettings::SettingsMap& settings) {
+  gcm::GServicesSettings::SettingsMap::const_iterator iter =
+      settings.find(kRegistrationURLKey);
+  // Default settings are only present in the map when overwritten.
+  if (iter == settings.end())
+    return true;
+
+  GURL registration_url(iter->second);
+  if (!registration_url.is_valid()) {
+    DVLOG(1) << "Invalid registration URL provided: " << iter->second;
+    return false;
+  }
+
+  return true;
+}
+
+bool VerifySettings(const gcm::GServicesSettings::SettingsMap& settings) {
+  return VerifyCheckinInterval(settings) && VerifyMCSEndpoint(settings) &&
+         VerifyCheckinURL(settings) && VerifyRegistrationURL(settings);
+}
+
 }  // namespace
 
 namespace gcm {
 
 // static
 const base::TimeDelta GServicesSettings::MinimumCheckinInterval() {
   return base::TimeDelta::FromSeconds(kMinimumCheckinInterval);
 }
 
 // static
 const GURL GServicesSettings::DefaultCheckinURL() {
   return GURL(kDefaultCheckinURL);
 }
 
-GServicesSettings::GServicesSettings()
-    : checkin_interval_(base::TimeDelta::FromSeconds(kDefaultCheckinInterval)),
-      checkin_url_(kDefaultCheckinURL),
-      mcs_main_endpoint_(MakeMCSEndpoint(kDefaultMCSHostname,
-                                         kDefaultMCSMainSecurePort)),
-      mcs_fallback_endpoint_(MakeMCSEndpoint(kDefaultMCSHostname,
-                                             kDefaultMCSFallbackSecurePort)),
-      registration_url_(kDefaultRegistrationURL),
-      weak_ptr_factory_(this) {
+// static
+std::string GServicesSettings::CalculateDigest(const SettingsMap& settings) {
+  unsigned char hash[base::kSHA1Length];
+  std::string data;
+  for (SettingsMap::const_iterator iter = settings.begin();
+       iter != settings.end();
+       ++iter) {
+    data += iter->first;
+    data += '\0';
+    data += iter->second;
+    data += '\0';
+  }
+  base::SHA1HashBytes(
+      reinterpret_cast<const unsigned char*>(&data[0]), data.size(), hash);
+  std::string digest =
+      kDigestVersionPrefix + base::HexEncode(hash, base::kSHA1Length);
+  std::transform(digest.begin(), digest.end(), digest.begin(), ::tolower);

[jianli, 2014/05/19 17:52:27]

nit: use StringToLowerASCII instead.

[fgorski, 2014/05/19 20:25:39]

Done.

+  return digest;
 }
 
-GServicesSettings::~GServicesSettings() {}
+GServicesSettings::GServicesSettings() : weak_ptr_factory_(this) {
+  digest_ = CalculateDigest(settings_);
+}
+
+GServicesSettings::~GServicesSettings() {
+}
 
 bool GServicesSettings::UpdateFromCheckinResponse(
-  const checkin_proto::AndroidCheckinResponse& checkin_response) {
-  if (!checkin_response.has_digest() ||
-      checkin_response.digest() == digest_) {
-    // There are no changes as digest is the same or no settings provided.
+    const checkin_proto::AndroidCheckinResponse& checkin_response) {
+  if (!checkin_response.has_settings_diff()) {
+    DVLOG(1) << "Field settings_diff not set in response.";
     return false;
   }
 
-  std::map<std::string, std::string> settings;
+  bool settings_diff = checkin_response.settings_diff();
+  SettingsMap new_settings;
+  // Only reuse the existing settings, if we are given a settings difference.
+  if (settings_diff)
+    new_settings = settings_map();
+
   for (int i = 0; i < checkin_response.setting_size(); ++i) {
     std::string name = checkin_response.setting(i).name();
-    std::string value = checkin_response.setting(i).value();
-    settings[name] = value;
+    if (name.empty()) {
+      DVLOG(1) << "Setting name is empty";
+      return false;
+    }
+
+    if (settings_diff && name.find(kDeleteSettingPrefix) == 0) {
+      std::string setting_to_delete =
+          name.substr(arraysize(kDeleteSettingPrefix) - 1);
+      new_settings.erase(setting_to_delete);
+      DVLOG(1) << "Setting deleted: " << setting_to_delete;
+    } else {
+      std::string value = checkin_response.setting(i).value();
+      new_settings[name] = value;
+      DVLOG(1) << "New setting: '" << name << "' : '" << value << "'";
+    }
   }
 
-  // Only update the settings in store and digest, if the settings actually
-  // passed the verificaiton in update settings.
-  if (UpdateSettings(settings)) {
-    digest_ = checkin_response.digest();
-    return true;
-  }
+  if (!VerifySettings(new_settings))
+    return false;
 
-  return false;
+  settings_.swap(new_settings);
+  digest_ = CalculateDigest(settings_);
+  return true;
 }
 
 void GServicesSettings::UpdateFromLoadResult(
     const GCMStore::LoadResult& load_result) {
-  if (UpdateSettings(load_result.gservices_settings))
-    digest_ = load_result.gservices_digest;
+  // No need to try to update settings when load_result is empty.
+  if (load_result.gservices_settings.empty())
+    return;
+  if (!VerifySettings(load_result.gservices_settings))
+    return;
+  std::string digest = CalculateDigest(load_result.gservices_settings);
+  if (digest != load_result.gservices_digest) {
+    DVLOG(1) << "G-services settings digest mismatch. "
+             << "Expected digest: " << load_result.gservices_digest
+             << ". Calculated digest is: " << digest;
+    return;
+  }
+
+  settings_ = load_result.gservices_settings;
+  digest_ = load_result.gservices_digest;
 }
 
-std::map<std::string, std::string> GServicesSettings::GetSettingsMap() const {
-  std::map<std::string, std::string> settings;
-  settings[kCheckinIntervalKey] =
-      base::Int64ToString(checkin_interval_.InSeconds());
-  settings[kCheckinURLKey] = checkin_url_.spec();
-  settings[kMCSHostnameKey] = mcs_main_endpoint_.host();
-  settings[kMCSSecurePortKey] = mcs_main_endpoint_.port();
-  settings[kRegistrationURLKey] = registration_url_.spec();
-  return settings;
+base::TimeDelta GServicesSettings::GetCheckinInterval() const {
+  int64 checkin_interval = kMinimumCheckinInterval;
+  SettingsMap::const_iterator iter = settings_.find(kCheckinIntervalKey);
+  if (iter == settings_.end() ||
+      !base::StringToInt64(iter->second, &checkin_interval)) {
+    checkin_interval = kDefaultCheckinInterval;
+  }
+
+  if (checkin_interval < kMinimumCheckinInterval)
+    checkin_interval = kMinimumCheckinInterval;
+
+  return base::TimeDelta::FromSeconds(checkin_interval);
 }
 
-bool GServicesSettings::UpdateSettings(
-    const std::map<std::string, std::string>& settings) {
-  int64 new_checkin_interval = kMinimumCheckinInterval;
-  std::map<std::string, std::string>::const_iterator iter =
-      settings.find(kCheckinIntervalKey);
-  if (iter == settings.end()) {
-    LOG(ERROR) << "Setting not found: " << kCheckinIntervalKey;
-    return false;
-  }
-  if (!base::StringToInt64(iter->second, &new_checkin_interval)) {
-    LOG(ERROR) << "Failed to parse checkin interval: " << iter->second;
-    return false;
-  }
-  if (new_checkin_interval < kMinimumCheckinInterval) {
-    LOG(ERROR) << "Checkin interval: " << new_checkin_interval
-               << " is less than allowed minimum: " << kMinimumCheckinInterval;
-    new_checkin_interval = kMinimumCheckinInterval;
-  }
-  if (new_checkin_interval == std::numeric_limits<int64>::max()) {
-    LOG(ERROR) << "Checkin interval is too big: " << new_checkin_interval;
-    return false;
+GURL GServicesSettings::GetCheckinURL() const {
+  SettingsMap::const_iterator iter = settings_.find(kCheckinURLKey);
+  if (iter == settings_.end() || iter->second.empty())
+    return GURL(kDefaultCheckinURL);
+  return GURL(iter->second);
+}
+
+GURL GServicesSettings::GetMCSMainEndpoint() const {
+  // Get alternative hostname or use default.
+  std::string mcs_hostname(kDefaultMCSHostname);

[jianli, 2014/05/19 17:52:27]

I think it will be more efficient not to set the default value. You could either
set it in the else part of next if statement, or simply skip it since the last
return statement covers it.

[fgorski, 2014/05/19 20:25:39]

Done.

+  SettingsMap::const_iterator iter = settings_.find(kMCSHostnameKey);
+  if (iter != settings_.end() && !iter->second.empty())
+    mcs_hostname = iter->second;
+
+  // Get alternative secure port or use defualt.
+  int mcs_secure_port = 0;
+  iter = settings_.find(kMCSSecurePortKey);
+  if (iter == settings_.end() || iter->second.empty() ||
+      !base::StringToInt(iter->second, &mcs_secure_port)) {
+    mcs_secure_port = kDefaultMCSMainSecurePort;
   }
 
-  std::string new_mcs_hostname;
-  iter = settings.find(kMCSHostnameKey);
-  if (iter == settings.end()) {
-    LOG(ERROR) << "Setting not found: " << kMCSHostnameKey;
-    return false;
-  }
-  new_mcs_hostname = iter->second;
-  if (new_mcs_hostname.empty()) {
-    LOG(ERROR) << "Empty MCS hostname provided.";
-    return false;
-  }
+  // If constructed address makes sense use it.
+  GURL mcs_endpoint(MakeMCSEndpoint(mcs_hostname, mcs_secure_port));
+  if (mcs_endpoint.is_valid())
+    return mcs_endpoint;
 
-  int new_mcs_secure_port = -1;
-  iter = settings.find(kMCSSecurePortKey);
-  if (iter == settings.end()) {
-    LOG(ERROR) << "Setting not found: " << kMCSSecurePortKey;
-    return false;
-  }
-  if (!base::StringToInt(iter->second, &new_mcs_secure_port)) {
-    LOG(ERROR) << "Failed to parse MCS secure port: " << iter->second;
-    return false;
-  }
-  if (new_mcs_secure_port < 0 || 65535 < new_mcs_secure_port) {
-    LOG(ERROR) << "Incorrect port value: " << new_mcs_secure_port;
-    return false;
-  }
+  // Otherwise use default settings.
+  return GURL(MakeMCSEndpoint(kDefaultMCSHostname, kDefaultMCSMainSecurePort));
+}
 
-  GURL new_mcs_main_endpoint =
-      GURL(MakeMCSEndpoint(new_mcs_hostname, new_mcs_secure_port));
-  GURL new_mcs_fallback_endpoint =
-      GURL(MakeMCSEndpoint(new_mcs_hostname, kDefaultMCSFallbackSecurePort));
-  if (!new_mcs_main_endpoint.is_valid() ||
-      !new_mcs_fallback_endpoint.is_valid())
-    return false;
+GURL GServicesSettings::GetMCSFallbackEndpoint() const {
+  // Get alternative hostname or use default.
+  std::string mcs_hostname(kDefaultMCSHostname);

[jianli, 2014/05/19 17:52:27]

ditto

[fgorski, 2014/05/19 20:25:39]

Done.

+  SettingsMap::const_iterator iter = settings_.find(kMCSHostnameKey);
+  if (iter != settings_.end() && !iter->second.empty())
+    mcs_hostname = iter->second;
 
-  GURL new_checkin_url;
-  iter = settings.find(kCheckinURLKey);
-  if (iter == settings.end()) {
-    LOG(ERROR) << "Setting not found: " << kCheckinURLKey;
-    return false;
-  }
-  new_checkin_url = GURL(iter->second);
-  if (!new_checkin_url.is_valid()) {
-    LOG(ERROR) << "Invalid checkin URL provided: "
-               << new_checkin_url.possibly_invalid_spec();
-    return false;
-  }
+  // If constructed address makes sense use it.
+  GURL mcs_endpoint(
+      MakeMCSEndpoint(mcs_hostname, kDefaultMCSFallbackSecurePort));
+  if (mcs_endpoint.is_valid())
+    return mcs_endpoint;
 
-  GURL new_registration_url;
-  iter = settings.find(kRegistrationURLKey);
-  if (iter == settings.end()) {
-    LOG(ERROR) << "Setting not found: " << kRegistrationURLKey;
-    return false;
-  }
-  new_registration_url = GURL(iter->second);
-  if (!new_registration_url.is_valid()) {
-    LOG(ERROR) << "Invalid registration URL provided: "
-               << new_registration_url.possibly_invalid_spec();
-    return false;
-  }
+  return GURL(
+      MakeMCSEndpoint(kDefaultMCSHostname, kDefaultMCSFallbackSecurePort));
+}
 
-  // We only update the settings once all of them are correct.
-  checkin_interval_ = base::TimeDelta::FromSeconds(new_checkin_interval);
-  mcs_main_endpoint_ = new_mcs_main_endpoint;
-  mcs_fallback_endpoint_ = new_mcs_fallback_endpoint;
-  checkin_url_ = new_checkin_url;
-  registration_url_ = new_registration_url;
-  return true;
+GURL GServicesSettings::GetRegistrationURL() const {
+  SettingsMap::const_iterator iter = settings_.find(kRegistrationURLKey);
+  if (iter == settings_.end() || iter->second.empty())
+    return GURL(kDefaultRegistrationURL);
+  return GURL(iter->second);
 }
 
 }  // namespace gcm