Revert of HttpServer: avoid DCHECK'ing on non-HTTP/1.1 requests. (htt…

…ps://codereview.chromium.org/251213004/)

Reason for revert:
Speculative revert since it might cause http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%282%29/builds/20802.

Original issue's description:
> HttpServer: avoid DCHECK'ing on non-HTTP/1.1 requests.
> 
> BUG=b/14249697
> R=mef@chromium.org,byungchul@chromium.org
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267066

TBR=byungchul@chromium.org,mef@chromium.org,gunsch@google.com,gunsch@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=b/14249697

Review URL: https://codereview.chromium.org/265603002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267149 0039d316-1c4b-4281-b951-d872f2087c98

net/http/http_response_headers.cc

@@ -625,23 +625,64 @@ HttpResponseHeaders::HttpResponseHeaders() : response_code_(-1) {
 HttpResponseHeaders::~HttpResponseHeaders() {
 }
 
+// Note: this implementation implicitly assumes that line_end points at a valid
+// sentinel character (such as '\0').
+// static
+HttpVersion HttpResponseHeaders::ParseVersion(
+    std::string::const_iterator line_begin,
+    std::string::const_iterator line_end) {
+  std::string::const_iterator p = line_begin;
+
+  // RFC2616 sec 3.1: HTTP-Version   = "HTTP" "/" 1*DIGIT "." 1*DIGIT
+  // TODO: (1*DIGIT apparently means one or more digits, but we only handle 1).
+  // TODO: handle leading zeros, which is allowed by the rfc1616 sec 3.1.
+
+  if ((line_end - p < 4) || !LowerCaseEqualsASCII(p, p + 4, "http")) {
+    DVLOG(1) << "missing status line";
+    return HttpVersion();
+  }
+
+  p += 4;
+
+  if (p >= line_end || *p != '/') {
+    DVLOG(1) << "missing version";
+    return HttpVersion();
+  }
+
+  std::string::const_iterator dot = std::find(p, line_end, '.');
+  if (dot == line_end) {
+    DVLOG(1) << "malformed version";
+    return HttpVersion();
+  }
+
+  ++p;  // from / to first digit.
+  ++dot;  // from . to second digit.
+
+  if (!(*p >= '0' && *p <= '9' && *dot >= '0' && *dot <= '9')) {
+    DVLOG(1) << "malformed version number";
+    return HttpVersion();
+  }
+
+  uint16 major = *p - '0';
+  uint16 minor = *dot - '0';
+
+  return HttpVersion(major, minor);
+}
+
 // Note: this implementation implicitly assumes that line_end points at a valid
 // sentinel character (such as '\0').
 void HttpResponseHeaders::ParseStatusLine(
     std::string::const_iterator line_begin,
     std::string::const_iterator line_end,
     bool has_headers) {
   // Extract the version number
-  std::string::const_iterator first_space =
-      std::find(line_begin, line_end, ' ');
-  std::string version(line_begin, first_space);
-  bool success = HttpUtil::ParseVersion(version, &parsed_http_version_);
+  parsed_http_version_ = ParseVersion(line_begin, line_end);
 
   // Clamp the version number to one of: {0.9, 1.0, 1.1}
-  if (success && parsed_http_version_ == HttpVersion(0, 9) && !has_headers) {
+  if (parsed_http_version_ == HttpVersion(0, 9) && !has_headers) {
     http_version_ = HttpVersion(0, 9);
     raw_headers_ = "HTTP/0.9";
-  } else if (success && parsed_http_version_ >= HttpVersion(1, 1)) {
+  } else if (parsed_http_version_ >= HttpVersion(1, 1)) {
     http_version_ = HttpVersion(1, 1);
     raw_headers_ = "HTTP/1.1";
   } else {
@@ -655,7 +696,7 @@ void HttpResponseHeaders::ParseStatusLine(
   }
 
   // TODO(eroman): this doesn't make sense if ParseVersion failed.
-  std::string::const_iterator p = first_space;
+  std::string::const_iterator p = std::find(line_begin, line_end, ' ');
 
   if (p == line_end) {
     DVLOG(1) << "missing response status; assuming 200 OK";

net/http/http_response_headers.h

@@ -335,6 +335,14 @@ class NET_EXPORT HttpResponseHeaders
   // Initializes from the given raw headers.
   void Parse(const std::string& raw_input);
 
+  // Helper function for ParseStatusLine.
+  // Tries to extract the "HTTP/X.Y" from a status line formatted like:
+  //    HTTP/1.1 200 OK
+  // with line_begin and end pointing at the begin and end of this line.  If the
+  // status line is malformed, returns HttpVersion(0,0).
+  static HttpVersion ParseVersion(std::string::const_iterator line_begin,
+                                  std::string::const_iterator line_end);
+
   // Tries to extract the status line from a header block, given the first
   // line of said header block.  If the status line is malformed, we'll
   // construct a valid one.  Example input:

net/http/http_util.cc

@@ -76,48 +76,6 @@ size_t HttpUtil::FindDelimiter(const std::string& line,
   return line.length();
 }
 
-// static
-bool HttpUtil::ParseVersion(const std::string& version_str,
-                            HttpVersion* version) {
-  std::string::const_iterator p = version_str.begin();
-
-  // RFC2616 sec 3.1: HTTP-Version   = "HTTP" "/" 1*DIGIT "." 1*DIGIT
-  // TODO: (1*DIGIT apparently means one or more digits, but we only handle 1).
-  // TODO: handle leading zeros, which is allowed by the rfc1616 sec 3.1.
-
-  if (version_str.length() < 4 || !LowerCaseEqualsASCII(p, p + 4, "http")) {
-    DVLOG(1) << "missing status line";
-    return false;
-  }
-
-  p += 4;
-
-  if (p >= version_str.end() || *p != '/') {
-    DVLOG(1) << "missing version";
-    return false;
-  }
-
-  std::string::const_iterator dot = std::find(p, version_str.end(), '.');
-  if (dot == version_str.end()) {
-    DVLOG(1) << "malformed version";
-    return false;
-  }
-
-  ++p;  // from / to first digit.
-  ++dot;  // from . to second digit.
-
-  if (!(*p >= '0' && *p <= '9' && *dot >= '0' && *dot <= '9')) {
-    DVLOG(1) << "malformed version number";
-    return false;
-  }
-
-  uint16 major = *p - '0';
-  uint16 minor = *dot - '0';
-
-  *version = HttpVersion(major, minor);
-  return true;
-}
-
 // static
 void HttpUtil::ParseContentType(const std::string& content_type_str,
                                 std::string* mime_type,

net/http/http_util.h

@@ -40,12 +40,6 @@ class NET_EXPORT HttpUtil {
                               size_t search_start,
                               char delimiter);
 
-  // Parses an "HTTP/X.Y" version from a string, such as:
-  //   "HTTP/1.1"
-  // Returns whether or not the version string was successfully parsed.
-  static bool ParseVersion(const std::string& version_str,
-                           HttpVersion* version);
-
   // Parses the value of a Content-Type header.  The resulting mime_type and
   // charset values are normalized to lowercase.  The mime_type and charset
   // output values are only modified if the content_type_str contains a mime

net/server/http_server.cc

@@ -13,7 +13,6 @@
 #include "base/sys_byteorder.h"
 #include "build/build_config.h"
 #include "net/base/net_errors.h"
-#include "net/http/http_util.h"
 #include "net/server/http_connection.h"
 #include "net/server/http_server_request_info.h"
 #include "net/server/http_server_response_info.h"
@@ -283,10 +282,8 @@ bool HttpServer::ParseHeaders(HttpConnection* connection,
           buffer.clear();
           break;
         case ST_PROTO:
-          if (!HttpUtil::ParseVersion(buffer, &info->http_version)) {
-            // Treat everything else like HTTP 1.0
-            info->http_version = HttpVersion(1, 0);
-          }
+          // TODO(mbelshe): Deal better with parsing protocol.
+          DCHECK(buffer == "HTTP/1.1");
           buffer.clear();
           break;
         case ST_NAME:

net/server/http_server_request_info.h

@@ -9,7 +9,6 @@
 #include <string>
 
 #include "net/base/ip_endpoint.h"
-#include "net/http/http_version.h"
 
 namespace net {
 
@@ -29,9 +28,6 @@ class HttpServerRequestInfo {
   // Request peer address.
   IPEndPoint peer;
 
-  // Request protocol/version
-  HttpVersion http_version;
-
   // Request method.
   std::string method;
 

net/server/http_server_unittest.cc

@@ -218,23 +218,11 @@ TEST_F(HttpServerTest, Request) {
   ASSERT_EQ("/test", GetRequest(0).path);
   ASSERT_EQ("", GetRequest(0).data);
   ASSERT_EQ(0u, GetRequest(0).headers.size());
-  ASSERT_EQ(HttpVersion(1, 1), GetRequest(0).http_version);
   ASSERT_TRUE(StartsWithASCII(GetRequest(0).peer.ToString(),
                               "127.0.0.1",
                               true));
 }
 
-TEST_F(HttpServerTest, InvalidHttpVersion) {
-  TestHttpClient client;
-  ASSERT_EQ(OK, client.ConnectAndWait(server_address_));
-  client.Send("GET /test HTTP//1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
-  ASSERT_EQ("GET", GetRequest(0).method);
-  ASSERT_EQ("/test", GetRequest(0).path);
-  ASSERT_EQ("", GetRequest(0).data);
-  ASSERT_EQ(HttpVersion(1, 0), GetRequest(0).http_version);
-}
-
 TEST_F(HttpServerTest, RequestWithHeaders) {
   TestHttpClient client;
   ASSERT_EQ(OK, client.ConnectAndWait(server_address_));