Record errors that trigger a data reduction proxy bypass

net/http/http_response_headers.cc

Index: net/http/http_response_headers.cc
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 7ecf37a2352704f02685e9012c64a04c296999bd..676f16db5aa90156a743e77b99823903b67d2964 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -1504,6 +1504,19 @@ HttpResponseHeaders::GetDataReductionProxyBypassEventType(
     // response is to minimize information transfer, a sender in general
     // should not generate representation metadata other than Cache-Control,
     // Content-Location, Date, ETag, Expires, and Vary.
+
+    // A Via header might also not be present in a 4xx response. If there is
+    // evidence that the 4xx response was generated by the proxy, bypass it.
+    if (response_code() >= HTTP_BAD_REQUEST &&
+        response_code() < HTTP_INTERNAL_SERVER_ERROR) {
+      // If the data reduction proxy ever deprecates use of this "Server"
+      // header, a 4xx response from the proxy will be interpreted as
+      // a response that is missing a "Via" header. Either way, the proxy will
+      // be bypassed.
+      if (HasHeaderValue("Server", "GFE/2.0"))

[cbentzel, 2014/05/29 20:04:41]

Actually, I'm confused about why we even need this check here and why we can't
just return PROXY_4XX_BYPASS.

At the point this is called, I think that:
  a) We expect that the request was directed to the proxy.
  b) The proxy was accessed via SPDY (and TLS more importantly).
  c) There is not a "Via 1.1 Chrome Compression Proxy" header.
  d) There is a 4xx response code.

In that case, we should be able to expect that this response is coming from the
proxy itself and not from an origin server, and also not due to headers being
stripped by intermediaries. If there are cases where the Via headers are being
removed from the proxy even though it is forwarding a 4xx response from the
origin servers then these are probably bugs in the proxy itself and should be
resolved there.

I also don't know if the behavior of responding with 4xx response codes is
technically correct for the proxy in this case, and will try to see if there is
a better way to signal a proxy-internal issue.

[Matt Welsh, 2014/05/29 20:49:18]

I think the reason for the GFE/2.0 header check is to distinguish the case where
we *think* we're talking to the proxy (via HTTP) but we're actually talking to
some other middlebox which is sending us a 4xx. Example would be a middlebox
that sends a 404 for some request.

The main rationale for this change is that the GFE can send 4xx responses on
things like too-large URLs and the like, and we don't want to turn off the proxy
in this case.

That said we don't have UMA data telling us how often this happens. So one
proposal:
  (1) Revert this change and continue bypassing 4xx responses that lack a Via
header;
  (2) Make sure that the UMA reporting on bypass reasons includes 4xx bypass so
we can track when it happens.


On 2014/05/29 20:04:41, cbentzel wrote:

[bengr, 2014/05/30 06:47:44]

Done.

+        return ProxyService::PROXY_4XX_BYPASS;
+      // Otherwise, bypass on the missing Via header.
+    }
     return ProxyService::MISSING_VIA_HEADER;
   }
   // There is no bypass event.