Add brotli content-encoding filter.

net/url_request/url_request_http_job.cc

 // Copyright (c) 2012 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 "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 573 matching lines @@
       packet_timing_enabled_ = true;
       if (base::RandDouble() < .01) {
         sdch_test_control_ = true;  // 1% probability.
         dictionaries_advertised_.reset();
         advertise_sdch = false;
       } else {
         sdch_test_activated_ = true;
       }
     }
 
+    // Advertise "br" encoding only if transferred data is opaque to proxy.
+    bool advertise_brotli = false;
+    const HttpNetworkSession::Params* network_session_params =
+        request()->context()->GetNetworkSessionParams();
+    if (network_session_params && network_session_params->enable_brotli) {

[Randy Smith (Not in Mondays), 2015/12/04 16:07:52]

nit: No curly braces needed if both conditional and statement are single line
(and its more consistent with surrounding code not to have them).

[eustas, 2015/12/07 12:30:38]

Done.

+      advertise_brotli = !request()->url().SchemeIs(url::kHttpScheme);

[Randy Smith (Not in Mondays), 2015/12/04 16:07:52]

I *think* you want SchemeIsCryptographic(); that's what I've seen elsewhere for
this type of test.  If that doesn't look right to you, push back and I'll dig up
someone who'll know for sure.

[eustas, 2015/12/07 12:30:38]

I'm not sure. The reason of this check is to avoid problems with old proxy
servers.
If proxy server deals well, for example, with ws/wss protocols, then brotli
encoding doesn't seem to be a problem.

Even "http" scheme check is too strict, because spdy/http2 protocols almost
guarantee (IMHO) that nothing bad should happen to the data with non
gzip/deflate Content-Encoding.

[Randy Smith (Not in Mondays), 2015/12/07 22:27:28]

So I'm confused as to why ws: or ftp: (both of which the current check allows)
would be ok.  I would think that any cleartext protocol allows interference from
proxies, whereas any end-to-end cryptographic protocol does not.  Thus my
suggestion.
I'm confused by this comment; this check is saying that if it's *not* http
you'll advertise brotli, which should include spdy2/http2.

[Ryan Sleevi, 2015/12/08 18:58:50]

+1 - I agree with Randy here.

!SchemeIs(url::kHttpScheme) smells like an anti-pattern here, and security has
been working to remove such ad-hoc checks in favour of SchemeIsCryptographic

Further, when we consider what other user agents may look for with implementing,
it's far more problematic to suggest a scheme-by-scheme whitelist as opposed to
simply stating 'secure origins'.

+    }
+
     // Supply Accept-Encoding headers first so that it is more likely that they
     // will be in the first transmitted packet. This can sometimes make it
     // easier to filter and analyze the streams to assure that a proxy has not
     // damaged these headers. Some proxies deliberately corrupt Accept-Encoding
     // headers.
-    if (!advertise_sdch) {
-      // Tell the server what compression formats we support (other than SDCH).
+    std::string advertised_encodings = "gzip, deflate";
+    if (advertise_sdch)
+      advertised_encodings += ", sdch";
+    if (advertise_brotli)
+      advertised_encodings += ", br";
+    // Tell the server what compression formats we support.

[xunjieli, 2015/12/04 15:21:19]

nit: avoid using first person pronouns. Maybe change to "Tell the server what
compression formats are supported."

[eustas, 2015/12/04 16:14:31]

Nice! Fixed.

+    request_info_.extra_headers.SetHeader(HttpRequestHeaders::kAcceptEncoding,
+                                          advertised_encodings);
+
+    if (dictionaries_advertised_) {
       request_info_.extra_headers.SetHeader(
-          HttpRequestHeaders::kAcceptEncoding, "gzip, deflate");
-    } else {
-      // Include SDCH in acceptable list.
-      request_info_.extra_headers.SetHeader(
-          HttpRequestHeaders::kAcceptEncoding, "gzip, deflate, sdch");
-      if (dictionaries_advertised_) {
-        request_info_.extra_headers.SetHeader(
-            kAvailDictionaryHeader,
-            dictionaries_advertised_->GetDictionaryClientHashList());
-        // Since we're tagging this transaction as advertising a dictionary,
-        // we'll definitely employ an SDCH filter (or tentative sdch filter)
-        // when we get a response. When done, we'll record histograms via
-        // SDCH_DECODE or SDCH_PASSTHROUGH. Hence we need to record packet
-        // arrival times.
-        packet_timing_enabled_ = true;
-      }
+          kAvailDictionaryHeader,
+          dictionaries_advertised_->GetDictionaryClientHashList());
+      // Since we're tagging this transaction as advertising a dictionary,
+      // we'll definitely employ an SDCH filter (or tentative sdch filter)
+      // when we get a response. When done, we'll record histograms via
+      // SDCH_DECODE or SDCH_PASSTHROUGH. Hence we need to record packet
+      // arrival times.
+      packet_timing_enabled_ = true;
     }
   }
 
   if (http_user_agent_settings_) {
     // Only add default Accept-Language if the request didn't have it
     // specified.
     std::string accept_language =
         http_user_agent_settings_->GetAcceptLanguage();
     if (!accept_language.empty()) {
       request_info_.extra_headers.SetHeaderIfMissing(
@@ skipping 937 matching lines @@
   return override_response_headers_.get() ?
              override_response_headers_.get() :
              transaction_->GetResponseInfo()->headers.get();
 }
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 }  // namespace net