HttpServer: avoid DCHECK'ing on non-HTTP/1.1 requests.

BUG=b/14249697
R=mef@chromium.org,byungchul@chromium.org

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

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

7 files changed, 76 insertions, 58 deletions

diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 940c048..4e18056 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -627,62 +627,21 @@ 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
-  parsed_http_version_ = ParseVersion(line_begin, line_end);
+  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_);
 
   // Clamp the version number to one of: {0.9, 1.0, 1.1}
-  if (parsed_http_version_ == HttpVersion(0, 9) && !has_headers) {
+  if (success && parsed_http_version_ == HttpVersion(0, 9) && !has_headers) {
     http_version_ = HttpVersion(0, 9);
     raw_headers_ = "HTTP/0.9";
-  } else if (parsed_http_version_ >= HttpVersion(1, 1)) {
+  } else if (success && parsed_http_version_ >= HttpVersion(1, 1)) {
     http_version_ = HttpVersion(1, 1);
     raw_headers_ = "HTTP/1.1";
   } else {
@@ -696,7 +655,7 @@ void HttpResponseHeaders::ParseStatusLine(
   }
 
   // TODO(eroman): this doesn't make sense if ParseVersion failed.
-  std::string::const_iterator p = std::find(line_begin, line_end, ' ');
+  std::string::const_iterator p = first_space;
 
   if (p == line_end) {
     DVLOG(1) << "missing response status; assuming 200 OK";
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 2bf4514..90ba7eb 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -335,14 +335,6 @@ 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:
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index f4f994a..481b104 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -77,6 +77,48 @@ size_t HttpUtil::FindDelimiter(const std::string& line,
 }
 
 // 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,
                                 std::string* charset,
diff --git a/net/http/http_util.h b/net/http/http_util.h
index ae65146..5ab27a6 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -40,6 +40,12 @@ 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
diff --git a/net/server/http_server.cc b/net/server/http_server.cc
index df77c36..20926e7 100644
--- a/net/server/http_server.cc
+++ b/net/server/http_server.cc
@@ -13,6 +13,7 @@
 #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"
@@ -282,8 +283,10 @@ bool HttpServer::ParseHeaders(HttpConnection* connection,
           buffer.clear();
           break;
         case ST_PROTO:
-          // TODO(mbelshe): Deal better with parsing protocol.
-          DCHECK(buffer == "HTTP/1.1");
+          if (!HttpUtil::ParseVersion(buffer, &info->http_version)) {
+            // Treat everything else like HTTP 1.0
+            info->http_version = HttpVersion(1, 0);
+          }
           buffer.clear();
           break;
         case ST_NAME:
diff --git a/net/server/http_server_request_info.h b/net/server/http_server_request_info.h
index 183da1c..0376b3a 100644
--- a/net/server/http_server_request_info.h
+++ b/net/server/http_server_request_info.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "net/base/ip_endpoint.h"
+#include "net/http/http_version.h"
 
 namespace net {
 
@@ -28,6 +29,9 @@ class HttpServerRequestInfo {
   // Request peer address.
   IPEndPoint peer;
 
+  // Request protocol/version
+  HttpVersion http_version;
+
   // Request method.
   std::string method;
 
diff --git a/net/server/http_server_unittest.cc b/net/server/http_server_unittest.cc
index de8690f..25a9cbc 100644
--- a/net/server/http_server_unittest.cc
+++ b/net/server/http_server_unittest.cc
@@ -218,11 +218,23 @@ 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_));