Modify data_reduction_proxy_header to support tamper detection logic.

components/data_reduction_proxy/common/data_reduction_proxy_headers.cc

Index: components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
diff --git a/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc b/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
index 15d35301f390f19d44de969bb7208a787ba03da2..56e2a07bde85609083b19611b4b7d0bbfcb7a4db 100644
--- a/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
+++ b/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
@@ -18,24 +18,59 @@ using base::StringPiece;
 using base::TimeDelta;
 using net::ProxyService;
 
+namespace {
+const char kChromeProxyHeader[] = "chrome-proxy";
+}  // namespace
+
 namespace data_reduction_proxy {
 
+const char kChromeProxyActionFingerprintChromeProxy[]   = "fcp";
+const char kChromeProxyActionFingerprintVia[]           = "fvia";
+const char kChromeProxyActionFingerprintOtherHeaders[]  = "foh";
+const char kChromeProxyActionFingerprintContentLength[] = "fcl";

[bengr, 2014/07/29 17:50:28]

I really don't like that you expose these names. Is there really no way to
provide methods that access what you need to know about them? E.g.,
bool HasChromeProxyFingerprint(...) or
bool GetChromeProxyFingerprint(std::string* fingerprint)

[xingx1, 2014/07/30 03:32:55]

Adding additional functions so that these constants would not be exposed to
namespace.

Done.

+
+bool GetDataReductionProxyActionValue(
+    const net::HttpResponseHeaders* headers,
+    const std::string& action_prefix,
+    std::string* action_value) {
+  DCHECK(!action_prefix.empty());

[bengr, 2014/07/29 17:50:28]

You should also check that header is not NULL.

[xingx1, 2014/07/30 03:32:55]

Done.

+  void* iter = NULL;
+  std::string value;
+  std::string prefix = action_prefix + "=";

[bengr, 2014/07/29 17:50:27]

Define "=" as kActionValueDelimiter in the anonymous namespace and use here and
below.

[xingx1, 2014/07/30 03:32:55]

Done.

+
+  while (headers->EnumerateHeader(&iter, kChromeProxyHeader, &value)) {
+    // ">=" to allow empty action value.

[bengr, 2014/07/29 17:50:27]

I don't think an empty value should be legal. Look at RFC2616, etc., to see
which pattern is used for other headers, e.g., cache-control.

[xingx1, 2014/07/30 03:32:56]

Done.

Previously I'm allowing empty action value to sort of send a command from
Flywheel without any parameter, say "via=" means "please check whether there is
intermediary on Via header".

+    if (value.size() >= prefix.size()) {
+      if (LowerCaseEqualsASCII(value.begin(),
+                               value.begin() + prefix.size(),
+                               prefix.c_str())) {
+        if (action_value)
+          *action_value = value.substr(prefix.size());
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool GetDataReductionProxyBypassDuration(
     const net::HttpResponseHeaders* headers,
     const std::string& action_prefix,
     base::TimeDelta* duration) {
+  DCHECK(!action_prefix.empty());
+  DCHECK(action_prefix[action_prefix.size() - 1] != '=');

[bolian, 2014/07/29 17:03:50]

same here, you don't need this DCHECK. Why not just allow '=' in the prefix.

[bengr, 2014/07/29 17:50:27]

I asked for the '=' to be separate from the prefix. This looks a little hacky
though.

[xingx1, 2014/07/30 03:32:55]

Done.

[xingx1, 2014/07/30 03:32:56]

I coded DCHECK there for now.

   void* iter = NULL;
   std::string value;
-  std::string name = "chrome-proxy";
+  std::string prefix = action_prefix + "=";
 
-  while (headers->EnumerateHeader(&iter, name, &value)) {
-    if (value.size() > action_prefix.size()) {
+  while (headers->EnumerateHeader(&iter, kChromeProxyHeader, &value)) {
+    if (value.size() > prefix.size()) {
       if (LowerCaseEqualsASCII(value.begin(),
-                               value.begin() + action_prefix.size(),
-                               action_prefix.c_str())) {
+                               value.begin() + prefix.size(),
+                               prefix.c_str())) {
         int64 seconds;
         if (!base::StringToInt64(
-                StringPiece(value.begin() + action_prefix.size(), value.end()),
+                StringPiece(value.begin() + prefix.size(), value.end()),
                 &seconds) || seconds < 0) {
           continue;  // In case there is a well formed instruction.
         }
@@ -63,20 +98,21 @@ bool GetDataReductionProxyInfo(const net::HttpResponseHeaders* headers,
   // 'block' takes precedence over 'bypass', so look for it first.
   // TODO(bengr): Reduce checks for 'block' and 'bypass' to a single loop.
   if (GetDataReductionProxyBypassDuration(
-      headers, "block=", &proxy_info->bypass_duration)) {
+      headers, "block", &proxy_info->bypass_duration)) {

[bengr, 2014/07/29 17:50:28]

Add to the anonymous namespace:
const char kChromeProxyActionBlock[] = "block";
const char kChromeProxyActionBypass[] = "bypass";

[xingx1, 2014/07/30 03:32:55]

Done.

     proxy_info->bypass_all = true;
     return true;
   }
 
   // Next, look for 'bypass'.
   if (GetDataReductionProxyBypassDuration(
-      headers, "bypass=", &proxy_info->bypass_duration)) {
+      headers, "bypass", &proxy_info->bypass_duration)) {
     return true;
   }
   return false;
 }
 
-bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers) {
+bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers,
+                                    bool* has_intermediary) {
   const size_t kVersionSize = 4;
   const char kDataReductionProxyViaValue[] = "Chrome-Compression-Proxy";
   size_t value_len = strlen(kDataReductionProxyViaValue);
@@ -88,8 +124,11 @@ bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers) {
   // 'Via: 1.1 Chrome-Compression-Proxy'
   while (headers->EnumerateHeader(&iter, "via", &value)) {
     if (value.size() >= kVersionSize + value_len &&
-        !value.compare(kVersionSize, value_len, kDataReductionProxyViaValue))
+        !value.compare(kVersionSize, value_len, kDataReductionProxyViaValue)) {
+      if (has_intermediary)

[bengr, 2014/07/29 17:50:27]

Add a comment here saying that we presume an intermediary if there is another
via header after the data reduction proxy's.

[xingx1, 2014/07/30 03:32:56]

Done.

+        *has_intermediary = !(headers->EnumerateHeader(&iter, "via", &value));
       return true;
+    }
   }
 
   // TODO(bengr): Remove deprecated header value.
@@ -97,8 +136,11 @@ bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers) {
       "1.1 Chrome Compression Proxy";
   iter = NULL;
   while (headers->EnumerateHeader(&iter, "via", &value))
-    if (value == kDeprecatedDataReductionProxyViaValue)
+    if (value == kDeprecatedDataReductionProxyViaValue) {
+      if (has_intermediary)
+        *has_intermediary = !(headers->EnumerateHeader(&iter, "via", &value));
       return true;
+    }
 
   return false;
 }
@@ -136,7 +178,7 @@ GetDataReductionProxyBypassType(
       !headers->HasHeader("Proxy-Authenticate")) {
     return ProxyService::MALFORMED_407;
   }
-  if (!HasDataReductionProxyViaHeader(headers) &&
+  if (!HasDataReductionProxyViaHeader(headers, NULL) &&
       (headers->response_code() != net::HTTP_NOT_MODIFIED)) {
     // A Via header might not be present in a 304. Since the goal of a 304
     // response is to minimize information transfer, a sender in general