DRP: Add GetPageIdFromRequestHeaders to DRP Request Options

Also refactors GetSessionKeyFromRequestHeaders to be static and return a
bool to indicate success, setting a passed pointer to the parsed session
key.

This is needed to support Lite Page Redirect pingbacks, in order to
get the DRP session id and page id that were used on a navigation.

Bug: 864665
Change-Id: Iefa1d6e39b58a0b71dbc1976ef1a0a2ba4712e73
Reviewed-on: https://chromium-review.googlesource.com/c/1345424
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609973}

components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc

@@ -311,7 +311,9 @@ bool DataReductionProxyConfigServiceClient::ShouldRetryDueToAuthFailure(
   // If the session key used in the request is different from the current
   // session key, then the current session key does not need to be
   // invalidated.
-  if (request_options_->GetSessionKeyFromRequestHeaders(request_headers) !=
+  base::Optional<std::string> session_key =
+      request_options_->GetSessionKeyFromRequestHeaders(request_headers);
+  if ((session_key.has_value() ? session_key.value() : std::string()) !=
       request_options_->GetSecureSession()) {
     RecordAuthExpiredSessionKey(false);
     return true;

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/optional.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/safe_sprintf.h"
@@ -38,6 +39,17 @@ std::string FormatOption(const std::string& name, const std::string& value) {
   return name + "=" + value;
 }
 
+bool ParseChromeProxyHeader(const net::HttpRequestHeaders& request_headers,
+                            base::StringPairs* kv_pairs) {
+  std::string chrome_proxy_header_value;
+  return request_headers.GetHeader(chrome_proxy_header(),
+                                   &chrome_proxy_header_value) &&
+         base::SplitStringIntoKeyValuePairs(chrome_proxy_header_value,
+                                            '=',  // Key-value delimiter
+                                            ',',  // Key-value pair delimiter
+                                            kv_pairs);
+}
+
 }  // namespace
 
 const char kSecureSessionHeaderOption[] = "s";
@@ -248,20 +260,13 @@ void DataReductionProxyRequestOptions::RegenerateRequestHeaderValue() {
   }
 }
 
-std::string DataReductionProxyRequestOptions::GetSessionKeyFromRequestHeaders(
-    const net::HttpRequestHeaders& request_headers) const {
-  std::string chrome_proxy_header_value;
+// static
+base::Optional<std::string>
+DataReductionProxyRequestOptions::GetSessionKeyFromRequestHeaders(
+    const net::HttpRequestHeaders& request_headers) {
   base::StringPairs kv_pairs;
-  // Return if the request does not have request headers or if they can't be
-  // parsed into key-value pairs.
-  if (!request_headers.GetHeader(chrome_proxy_header(),
-                                 &chrome_proxy_header_value) ||
-      !base::SplitStringIntoKeyValuePairs(chrome_proxy_header_value,
-                                          '=',  // Key-value delimiter
-                                          ',',  // Key-value pair delimiter
-                                          &kv_pairs)) {
-    return "";
-  }
+  if (!ParseChromeProxyHeader(request_headers, &kv_pairs))
+    return base::nullopt;
 
   for (const auto& kv_pair : kv_pairs) {
     // Delete leading and trailing white space characters from the key before
@@ -272,7 +277,32 @@ std::string DataReductionProxyRequestOptions::GetSessionKeyFromRequestHeaders(
           .as_string();
     }
   }
-  return "";
+  return base::nullopt;
+}
+
+// static
+base::Optional<uint64_t>
+DataReductionProxyRequestOptions::GetPageIdFromRequestHeaders(
+    const net::HttpRequestHeaders& request_headers) {
+  base::StringPairs kv_pairs;
+  if (!ParseChromeProxyHeader(request_headers, &kv_pairs))
+    return base::nullopt;
+
+  for (const auto& kv_pair : kv_pairs) {
+    // Delete leading and trailing white space characters from the key before
+    // comparing.
+    if (base::TrimWhitespaceASCII(kv_pair.first, base::TRIM_ALL) ==
+        kPageIdOption) {
+      uint64_t page_id;
+      if (base::StringToUint64(
+              base::TrimWhitespaceASCII(kv_pair.second, base::TRIM_ALL)
+                  .as_string(),
+              &page_id)) {
+        return page_id;
+      }
+    }
+  }
+  return base::nullopt;
 }
 
 uint64_t DataReductionProxyRequestOptions::GeneratePageId() {

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h

@@ -94,8 +94,12 @@ class DataReductionProxyRequestOptions {
   void Invalidate();
 
   // Parses |request_headers| and returns the value of the session key.
-  std::string GetSessionKeyFromRequestHeaders(
-      const net::HttpRequestHeaders& request_headers) const;
+  static base::Optional<std::string> GetSessionKeyFromRequestHeaders(
+      const net::HttpRequestHeaders& request_headers);
+
+  // Parses |request_headers| and returns the value of the page id.
+  static base::Optional<uint64_t> GetPageIdFromRequestHeaders(
+      const net::HttpRequestHeaders& request_headers);
 
   // Creates and returns a new unique page ID (unique per session).
   uint64_t GeneratePageId();

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc

@@ -367,20 +367,22 @@ TEST_F(DataReductionProxyRequestOptionsTest, GetSessionKeyFromRequestHeaders) {
     std::string chrome_proxy_header_key;
     std::string chrome_proxy_header_value;
     std::string expected_session_key;
+    bool expect_result;
   } tests[] = {
-      {"chrome-proxy", "something=something_else, s=123, key=value", "123"},
+      {"chrome-proxy", "something=something_else, s=123, key=value", "123",
+       true},
       {"chrome-proxy", "something=something_else, s= 123  456 , key=value",
-       "123  456"},
+       "123  456", true},
       {"chrome-proxy", "something=something_else, s=123456,    key=value",
-       "123456"},
+       "123456", true},
       {"chrome-proxy", "something=something else, s=123456,    key=value",
-       "123456"},
-      {"chrome-proxy", "something=something else, s=123456  ", "123456"},
-      {"chrome-proxy", "something=something_else, s=, key=value", ""},
-      {"chrome-proxy", "something=something_else, key=value", ""},
-      {"chrome-proxy", "s=123", "123"},
-      {"chrome-proxy", " s = 123 ", "123"},
-      {"some_other_header", "s=123", ""},
+       "123456", true},
+      {"chrome-proxy", "something=something else, s=123456  ", "123456", true},
+      {"chrome-proxy", "something=something_else, s=, key=value", "", false},
+      {"chrome-proxy", "something=something_else, key=value", "", false},
+      {"chrome-proxy", "s=123", "123", true},
+      {"chrome-proxy", " s = 123 ", "123", true},
+      {"some_other_header", "s=123", "", false},
   };
 
   for (const auto& test : tests) {
@@ -390,11 +392,55 @@ TEST_F(DataReductionProxyRequestOptionsTest, GetSessionKeyFromRequestHeaders) {
                               test.chrome_proxy_header_value);
     request_headers.SetHeader("some_random_header_after", "some_random_key");
 
-    std::string session_key =
+    base::Optional<std::string> session_key =
         request_options()->GetSessionKeyFromRequestHeaders(request_headers);
-    EXPECT_EQ(test.expected_session_key, session_key)
-        << test.chrome_proxy_header_key << ":"
-        << test.chrome_proxy_header_value;
+    EXPECT_EQ(test.expect_result, session_key.has_value());
+    if (test.expect_result) {
+      EXPECT_EQ(test.expected_session_key, session_key)
+          << test.chrome_proxy_header_key << ":"
+          << test.chrome_proxy_header_value;
+    }
+  }
+}
+
+TEST_F(DataReductionProxyRequestOptionsTest, GetPageIdFromRequestHeaders) {
+  const struct {
+    std::string chrome_proxy_header_key;
+    std::string chrome_proxy_header_value;
+    uint64_t expected_page_id;
+    bool expect_result;
+  } tests[] = {
+      {"chrome-proxy", "something=something_else, pid=123, key=value", 123,
+       true},
+      {"chrome-proxy", "something=something_else, pid= 123 , key=value", 123,
+       true},
+      {"chrome-proxy", "something=something_else, pid=123456,    key=value",
+       123456, true},
+      {"chrome-proxy", "something=something else, pid=123456,    key=value",
+       123456, true},
+      {"chrome-proxy", "something=something else, pid=123456  ", 123456, true},
+      {"chrome-proxy", "something=something_else, pid=, key=value", 0, false},
+      {"chrome-proxy", "something=something_else, key=value", 0, false},
+      {"chrome-proxy", "pid=123", 123, true},
+      {"chrome-proxy", " pid = 123 ", 123, true},
+      {"some_other_header", "pid=123", 0, false},
+  };
+
+  for (const auto& test : tests) {
+    net::HttpRequestHeaders request_headers;
+    request_headers.SetHeader("some_random_header_before", "some_random_key");
+    request_headers.SetHeader(test.chrome_proxy_header_key,
+                              test.chrome_proxy_header_value);
+    request_headers.SetHeader("some_random_header_after", "some_random_key");
+
+    base::Optional<uint64_t> page_id =
+        request_options()->GetPageIdFromRequestHeaders(request_headers);
+    EXPECT_EQ(test.expect_result, page_id.has_value());
+    if (test.expect_result) {
+      EXPECT_EQ(test.expected_page_id, page_id)
+          << test.chrome_proxy_header_key << ":"
+          << test.chrome_proxy_header_value;
+    }
   }
 }