Add detection of Shoutcast responses and accept them as valid HTTP/0.9

net/http/http_stream_parser.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_stream_parser.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 950 matching lines @@
   if (response_header_start_offset_ >= 0) {
     received_bytes_ += end_offset;
     headers = new HttpResponseHeaders(
         HttpUtil::AssembleRawHeaders(read_buf_->StartOfBuffer(), end_offset));
   } else {
     // Enough data was read -- there is no status line, so this is HTTP/0.9, or
     // the server is broken / doesn't speak HTTP.
 
     // If the port is not the default for the scheme, assume it's not a real
     // HTTP/0.9 response, and fail the request.
-    // TODO(crbug.com/624462):  Further restrict the cases in which we allow
-    // HTTP/0.9.
-    std::string scheme(request_->url.scheme());
+    base::StringPiece scheme(request_->url.scheme_piece());

[eroman, 2017/01/24 00:37:29]

[optional] style-nit: use assignment

[mmenke, 2017/01/24 17:27:02]

Done.  I find I tend to default to this style because it's what I use for
unique_ptr

     if (!http_09_on_non_default_ports_enabled_ &&
-        url::DefaultPortForScheme(scheme.c_str(), scheme.length()) !=
+        url::DefaultPortForScheme(scheme.data(), scheme.length()) !=
             request_->url.EffectiveIntPort()) {
-      return ERR_INVALID_HTTP_RESPONSE;
+      // Allow Shoutcast responses over HTTP through, as it's somewhat common

[eroman, 2017/01/24 00:37:29]

nit: delete "through" as it feels redundant with "allow"

[mmenke, 2017/01/24 17:27:02]

Done.

+      // and relies on HTTP/0.9 on weird ports to work.

[eroman, 2017/01/24 00:37:29]

Can you add a link to an explanation of what the icy headers are and how they
are used? (I was unfamiliar with this until reading the discussion on our
mailinglist).

Could be a bug link, or even the intent to implement thread which explains it
well.

[mmenke, 2017/01/24 17:27:02]

Done, link to the intent to implement (Bug has way too much noise)

+      if (read_buf_->offset() < 3 || scheme != "http" ||
+          !base::LowerCaseEqualsASCII(
+              base::StringPiece(read_buf_->StartOfBuffer(), 3), "icy")) {
+        return ERR_INVALID_HTTP_RESPONSE;
+      }
     }
 
     headers = new HttpResponseHeaders(std::string("HTTP/0.9 200 OK"));
   }
 
   // Check for multiple Content-Length headers when the response is not
   // chunked-encoded.  If they exist, and have distinct values, it's a potential
   // response smuggling attack.
   if (!headers->IsChunkEncoded()) {
     if (HeadersContainMultipleCopiesOfField(*headers, "Content-Length"))
@@ skipping 186 matching lines @@
   }
   return false;
 }
 
 bool HttpStreamParser::SendRequestBuffersEmpty() {
   return request_headers_ == nullptr && request_body_send_buf_ == nullptr &&
          request_body_read_buf_ == nullptr;
 }
 
 }  // namespace net