Use non-static set_key interface on DataReductionProxySettings

This removes the use of a static initializer and makes it possible
for clients to pass a key without owning the memory where the
key is stored (as would be the case if passed to a static const char*).

BUG=371626, 371204

Review URL: https://codereview.chromium.org/279633003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269578 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 64 insertions, 49 deletions

diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.cc
index 04665fb..1158748 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.cc
@@ -33,7 +33,8 @@ int64
 DataReductionProxyAuthRequestHandler::auth_token_invalidation_timestamp_ = 0;
 
 
-DataReductionProxyAuthRequestHandler::DataReductionProxyAuthRequestHandler() {
+DataReductionProxyAuthRequestHandler::DataReductionProxyAuthRequestHandler(
+    DataReductionProxySettings* settings) : settings_(settings) {
 }
 
 DataReductionProxyAuthRequestHandler::~DataReductionProxyAuthRequestHandler() {
@@ -108,7 +109,8 @@ bool DataReductionProxyAuthRequestHandler::IsAcceptableAuthChallenge(
 
 base::string16 DataReductionProxyAuthRequestHandler::GetTokenForAuthChallenge(
     net::AuthChallengeInfo* auth_info) {
-  return DataReductionProxySettings::GetTokenForAuthChallenge(auth_info);
+  DCHECK(settings_);
+  return settings_->GetTokenForAuthChallenge(auth_info);
 }
 
 base::TimeTicks DataReductionProxyAuthRequestHandler::Now() {
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h
index 4de3757..312eed2 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h
@@ -15,6 +15,8 @@ class AuthChallengeInfo;
 
 namespace data_reduction_proxy {
 
+class DataReductionProxySettings;
+
 class DataReductionProxyAuthRequestHandler {
  public:
   enum TryHandleResult {
@@ -23,7 +25,10 @@ class DataReductionProxyAuthRequestHandler {
     TRY_HANDLE_RESULT_CANCEL
   };
 
-  DataReductionProxyAuthRequestHandler();
+  // Constructs an authentication request handler and takes a pointer to a
+  // |settings| object, which must outlive the handler.
+  explicit DataReductionProxyAuthRequestHandler(
+      DataReductionProxySettings* settings);
   virtual ~DataReductionProxyAuthRequestHandler();
 
   // Returns |PROCEED| if the authentication challenge provided is one that the
@@ -65,6 +70,9 @@ class DataReductionProxyAuthRequestHandler {
   // invalidation from repeat failures due to the client not being authorized.
   static int64 auth_token_invalidation_timestamp_;
 
+  // Settings object for the data reduction proxy. Must outlive the handler.
+  DataReductionProxySettings* settings_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyAuthRequestHandler);
 };
 
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler_unittest.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler_unittest.cc
index 19c6dee..ba30403 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler_unittest.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler_unittest.cc
@@ -31,7 +31,8 @@ class TestDataReductionProxyAuthRequestHandler
  public:
   TestDataReductionProxyAuthRequestHandler(int time_step_ms,
                                            int64 initial_time_ms)
-      : time_step_ms_(time_step_ms),
+      : DataReductionProxyAuthRequestHandler(NULL),
+        time_step_ms_(time_step_ms),
         now_(base::TimeTicks() +
              base::TimeDelta::FromMilliseconds(initial_time_ms)) {}
  protected:
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
index 3683936..fe08eaa 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
@@ -67,7 +67,6 @@ int64 GetInt64PrefValue(const base::ListValue& list_value, size_t index) {
 
 namespace data_reduction_proxy {
 
-std::string DataReductionProxySettings::key_;
 bool DataReductionProxySettings::allowed_;
 bool DataReductionProxySettings::promo_allowed_;
 
@@ -93,11 +92,6 @@ bool DataReductionProxySettings::IsIncludedInFieldTrialOrFlags() {
 }
 
 // static
-void DataReductionProxySettings::SetKey(const std::string& key) {
-  key_ = key;
-}
-
-// static
 void DataReductionProxySettings::SetAllowed(bool allowed) {
   allowed_ = allowed;
 }
@@ -174,25 +168,26 @@ void DataReductionProxySettings::SetProxyConfigurator(
 
 // static
 void DataReductionProxySettings::InitDataReductionProxySession(
-    net::HttpNetworkSession* session) {
-// This is a no-op unless the authentication parameters are compiled in.
-// (even though values for them may be specified on the command line).
-// Authentication will still work if the command line parameters are used,
-// however there will be a round-trip overhead for each challenge/response
-// (typically once per session).
-// TODO(bengr):Pass a configuration struct into DataReductionProxyConfigurator's
-// constructor. The struct would carry everything in the preprocessor flags.
-  if (key_.empty())
+    net::HttpNetworkSession* session,
+    const std::string& key) {
+  // This is a no-op unless the key is set. (even though values for them may be
+  // specified on the command line). Authentication will still work if the
+  // command line parameters are used, however there will be a round-trip
+  // overhead for each challenge/response (typically once per session).
+  // TODO(bengr):Pass a configuration struct into
+  // DataReductionProxyConfigurator's constructor.
+  if (key.empty())
     return;
   DCHECK(session);
   net::HttpAuthCache* auth_cache = session->http_auth_cache();
   DCHECK(auth_cache);
-  InitDataReductionAuthentication(auth_cache);
+  InitDataReductionAuthentication(auth_cache, key);
 }
 
 // static
 void DataReductionProxySettings::InitDataReductionAuthentication(
-    net::HttpAuthCache* auth_cache) {
+    net::HttpAuthCache* auth_cache,
+    const std::string& key) {
   DCHECK(auth_cache);
   int64 timestamp =
       (base::Time::Now() - base::Time::UnixEpoch()).InMilliseconds() / 1000;
@@ -215,7 +210,7 @@ void DataReductionProxySettings::InitDataReductionAuthentication(
         rand[0],
         rand[1],
         rand[2]);
-    base::string16 password = AuthHashForSalt(timestamp);
+    base::string16 password = AuthHashForSalt(timestamp, key);
 
     DVLOG(1) << "origin: [" << auth_origin << "] realm: [" << realm
         << "] challenge: [" << challenge << "] password: [" << password << "]";
@@ -310,7 +305,6 @@ bool DataReductionProxySettings::IsAcceptableAuthChallenge(
   return false;
 }
 
-// static
 base::string16 DataReductionProxySettings::GetTokenForAuthChallenge(
     net::AuthChallengeInfo* auth_info) {
   if (auth_info->realm.length() > strlen(kAuthenticationRealmName)) {
@@ -318,7 +312,7 @@ base::string16 DataReductionProxySettings::GetTokenForAuthChallenge(
     std::string realm_suffix =
         auth_info->realm.substr(strlen(kAuthenticationRealmName));
     if (base::StringToInt64(realm_suffix, &salt)) {
-      return AuthHashForSalt(salt);
+      return AuthHashForSalt(salt, key_);
     } else {
       DVLOG(1) << "Unable to parse realm name " << auth_info->realm
                << "into an int for salting.";
@@ -652,11 +646,10 @@ std::string DataReductionProxySettings::GetProxyCheckURL() {
 }
 
 // static
-base::string16 DataReductionProxySettings::AuthHashForSalt(int64 salt) {
-  if (!IsDataReductionProxyAllowed())
-    return base::string16();
-
-  std::string key;
+base::string16 DataReductionProxySettings::AuthHashForSalt(
+    int64 salt,
+    const std::string& key) {
+  std::string active_key;
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kDataReductionProxy)) {
@@ -665,17 +658,17 @@ base::string16 DataReductionProxySettings::AuthHashForSalt(int64 salt) {
     // Don't expose |key_| to a proxy passed in via the command line.
     if (!command_line.HasSwitch(switches::kDataReductionProxyKey))
       return base::string16();
-    key = command_line.GetSwitchValueASCII(switches::kDataReductionProxyKey);
+    active_key = command_line.GetSwitchValueASCII(
+        switches::kDataReductionProxyKey);
   } else {
-    key = key_;
+    active_key = key;
   }
-
-  DCHECK(!key.empty());
+  DCHECK(!active_key.empty());
 
   std::string salted_key =
       base::StringPrintf("%lld%s%lld",
                          static_cast<long long>(salt),
-                         key.c_str(),
+                         active_key.c_str(),
                          static_cast<long long>(salt));
   return base::UTF8ToUTF16(base::MD5String(salted_key));
 }
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_settings.h b/components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
index 4fd7711..27bf158 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
@@ -101,13 +101,21 @@ class DataReductionProxySettings
   // determine if the data reduction proxy is allowed.
   static bool IsIncludedInFieldTrialOrFlags();
 
-  static void SetKey(const std::string& key);
   static void SetAllowed(bool allowed);
   static void SetPromoAllowed(bool promo_allowed);
 
   DataReductionProxySettings();
   virtual ~DataReductionProxySettings();
 
+  // Set and get the key to be used for data reduction proxy authentication.
+  void set_key(const std::string& key) {
+    key_ = key;
+  }
+
+  const std::string& key() const {
+    return key_;
+  }
+
   // Initializes the data reduction proxy with profile and local state prefs,
   // and a |UrlRequestContextGetter| for canary probes. The caller must ensure
   // that all parameters remain alive for the lifetime of the
@@ -133,9 +141,10 @@ class DataReductionProxySettings
   void SetProxyConfigurator(
       scoped_ptr<DataReductionProxyConfigurator> configurator);
 
-  // If proxy authentication is compiled in, pre-cache authentication
-  // keys for all configured proxies in |session|.
-  static void InitDataReductionProxySession(net::HttpNetworkSession* session);
+  // If proxy authentication is compiled in, pre-cache an authentication
+  // |key| for all configured proxies in |session|.
+  static void InitDataReductionProxySession(net::HttpNetworkSession* session,
+                                            const std::string& key);
 
   // Returns true if the data reduction proxy is allowed to be used. This could
   // return false, for example, if this instance is not part of the field trial,
@@ -168,8 +177,7 @@ class DataReductionProxySettings
   // Returns a UTF16 string suitable for use as an authentication token in
   // response to the challenge represented by |auth_info|. If the token can't
   // be correctly generated for |auth_info|, returns an empty UTF16 string.
-  static base::string16 GetTokenForAuthChallenge(
-      net::AuthChallengeInfo* auth_info);
+  base::string16 GetTokenForAuthChallenge(net::AuthChallengeInfo* auth_info);
 
   // Returns true if the proxy is enabled.
   bool IsDataReductionProxyEnabled();
@@ -286,7 +294,8 @@ class DataReductionProxySettings
 
   // Underlying implementation of InitDataReductionProxySession(), factored
   // out to be testable without creating a full HttpNetworkSession.
-  static void InitDataReductionAuthentication(net::HttpAuthCache* auth_cache);
+  static void InitDataReductionAuthentication(net::HttpAuthCache* auth_cache,
+                                              const std::string& key);
 
   void OnProxyEnabledPrefChange();
 
@@ -301,14 +310,14 @@ class DataReductionProxySettings
   std::string GetProxyCheckURL();
 
   // Returns a UTF16 string that's the hash of the configured authentication
-  // key and |salt|. Returns an empty UTF16 string if no key is configured or
+  // |key| and |salt|. Returns an empty UTF16 string if no key is configured or
   // the data reduction proxy feature isn't available.
-  static base::string16 AuthHashForSalt(int64 salt);
+  static base::string16 AuthHashForSalt(int64 salt, const std::string& key);
 
-  static std::string key_;
   static bool allowed_;
   static bool promo_allowed_;
 
+  std::string key_;
   bool restricted_by_carrier_;
   bool enabled_by_user_;
 
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_settings_unittest.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_settings_unittest.cc
index ddb56a9..56778a7 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_settings_unittest.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_settings_unittest.cc
@@ -41,7 +41,8 @@ class DataReductionProxySettingsTest
 TEST_F(DataReductionProxySettingsTest, TestAuthenticationInit) {
   AddProxyToCommandLine();
   net::HttpAuthCache cache;
-  DataReductionProxySettings::InitDataReductionAuthentication(&cache);
+  DataReductionProxySettings::InitDataReductionAuthentication(
+      &cache, kDataReductionProxyKey);
   DataReductionProxySettings::DataReductionProxyList proxies =
       DataReductionProxySettings::GetDataReductionProxies();
   for (DataReductionProxySettings::DataReductionProxyList::iterator it =
@@ -120,7 +121,8 @@ TEST_F(DataReductionProxySettingsTest, TestAuthHashGeneration) {
   std::string salted_key = salt + kDataReductionProxyKey + salt;
   base::string16 expected_hash = base::UTF8ToUTF16(base::MD5String(salted_key));
   EXPECT_EQ(expected_hash,
-            DataReductionProxySettings::AuthHashForSalt(8675309));
+            DataReductionProxySettings::AuthHashForSalt(
+                8675309, kDataReductionProxyKey));
 }
 
 // Test that the auth key set by preprocessor directive is not used
@@ -131,7 +133,8 @@ TEST_F(DataReductionProxySettingsTest,
   CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kDataReductionProxy, kDataReductionProxy);
   EXPECT_EQ(base::string16(),
-            DataReductionProxySettings::AuthHashForSalt(8675309));
+            DataReductionProxySettings::AuthHashForSalt(
+                8675309, kDataReductionProxyKey));
 }
 
 TEST_F(DataReductionProxySettingsTest, TestIsProxyEnabledOrManaged) {
@@ -207,8 +210,7 @@ TEST_F(DataReductionProxySettingsTest, TestChallengeTokens) {
     auth_info->challenger =
         net::HostPortPair::FromString(kDataReductionProxy);
     auth_info->realm = tests[i].realm;
-    base::string16 token =
-        DataReductionProxySettings::GetTokenForAuthChallenge(auth_info.get());
+    base::string16 token = settings_->GetTokenForAuthChallenge(auth_info.get());
     EXPECT_EQ(tests[i].expected_empty_token, token.empty());
   }
 }