Reject unadvertised encodings

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index a5395d758bcc4c9d26ef115ca43da28ce9424a0c..568da5fdcb59f482c833a5fc113bc084bd7c7355 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -32,6 +32,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/upload_data_stream.h"
 #include "net/base/url_util.h"
+#include "net/filter/filter_source_stream.h"
 #include "net/http/http_auth.h"
 #include "net/http/http_auth_handler.h"
 #include "net/http/http_auth_handler_factory.h"
@@ -545,6 +546,13 @@ void HttpNetworkTransaction::OnNeedsProxyAuth(
   establishing_tunnel_ = true;
   response_.headers = proxy_response.headers;
   response_.auth_challenge = proxy_response.auth_challenge;
+
+  if (response_.headers.get() && !ContentEncodingsValid()) {
+    FilterSourceStream::ReportContentDecodingFailed(SourceStream::TYPE_UNKNOWN);
+    DoCallback(ERR_CONTENT_DECODING_FAILED);
+    return;
+  }
+
   headers_valid_ = true;
   server_ssl_config_ = used_ssl_config;
   proxy_info_ = used_proxy_info;
@@ -1246,6 +1254,11 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
 
   DCHECK(response_.headers.get());
 
+  if (response_.headers.get() && !ContentEncodingsValid()) {
+    FilterSourceStream::ReportContentDecodingFailed(SourceStream::TYPE_UNKNOWN);
+    return ERR_CONTENT_DECODING_FAILED;
+  }
+
   // On a 408 response from the server ("Request Timeout") on a stale socket,
   // retry the request.
   // Headers can be NULL because of http://crbug.com/384554.
@@ -1711,4 +1724,28 @@ void HttpNetworkTransaction::CopyConnectionAttemptsFromStreamRequest() {
     connection_attempts_.push_back(attempt);
 }
 
+bool HttpNetworkTransaction::ContentEncodingsValid() const {
+  HttpResponseHeaders* headers = GetResponseHeaders();
+  DCHECK(headers);
+
+  std::string accept_encoding;
+  request_headers_.GetHeader(HttpRequestHeaders::kAcceptEncoding,
+                             &accept_encoding);
+  std::set<std::string> allowed_encodings;
+  if (!HttpUtil::ParseAcceptEncoding(accept_encoding, &allowed_encodings))

[Randy Smith (Not in Mondays), 2017/03/21 21:23:20]

Does this code handle the case where neither header is included in the
request/response pair?  (Which I think your last PS handled?)   If it does, it
would seem like that's reliant on ParseAcceptEncoding() succeeding with a null
string as input, which seems worth documenting in the header file?

(I'm presuming there's no spec difference between the lack of an Accept-Encoding
header and "Accept-Encoding: "?)

[eustas, 2017/03/27 10:27:11]

Added augmentation in parser: "" -> {"*"}.
Now asterisk case is checked explicitly.

+    return false;
+
+  std::string content_encoding;
+  headers->GetNormalizedHeader("Content-Encoding", &content_encoding);
+  std::set<std::string> used_encodings;
+  if (!HttpUtil::ParseContentEncoding(content_encoding, &used_encodings))
+    return false;
+
+  for (auto const& encoding : used_encodings) {

[Randy Smith (Not in Mondays), 2017/03/21 21:23:20]

This doesn't appear to handle "Accept-Encoding: *".  I'm ok with that, but I'd
like a comment to that effect.

[eustas, 2017/03/27 10:27:11]

Fixed.

+    if (allowed_encodings.find(encoding) == allowed_encodings.end())
+      return false;
+  }
+  return true;
+}
+
 }  // namespace net