Reject unadvertised encodings

net/http/http_network_transaction.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/http/http_network_transaction.h"
 
 #include <memory>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 14 matching lines @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/load_timing_info.h"
 #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"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_chunked_decoder.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
@@ skipping 493 matching lines @@
     const HttpResponseInfo& proxy_response,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpAuthController* auth_controller) {
   DCHECK(stream_request_.get());
   DCHECK_EQ(STATE_CREATE_STREAM_COMPLETE, next_state_);
 
   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;
 
   auth_controllers_[HttpAuth::AUTH_PROXY] = auth_controller;
   pending_auth_target_ = HttpAuth::AUTH_PROXY;
 
   DoCallback(OK);
 }
 
@@ skipping 681 matching lines @@
   // bizarre for SPDY. Assuming this logic is useful at all.
   // TODO(davidben): Bubble the error code up so we do not cache?
   if (result == ERR_CONNECTION_CLOSED && response_.headers.get())
     result = OK;
 
   if (result < 0)
     return HandleIOError(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.
   if (response_.headers.get() && response_.headers->response_code() == 408 &&
       stream_->IsConnectionReused()) {
     net_log_.AddEventWithNetErrorCode(
         NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR,
         response_.headers->response_code());
     // This will close the socket - it would be weird to try and reuse it, even
     // if the server doesn't actually close it.
@@ skipping 445 matching lines @@
 void HttpNetworkTransaction::CopyConnectionAttemptsFromStreamRequest() {
   DCHECK(stream_request_);
 
   // Since the transaction can restart with auth credentials, it may create a
   // stream more than once. Accumulate all of the connection attempts across
   // those streams by appending them to the vector:
   for (const auto& attempt : stream_request_->connection_attempts())
     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