From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sun, 1 Jun 2014 11:53:02 +0000 (-0700)
Subject: Update HTTP-version parsing with RFC 7230 octet magics
X-Git-Tag: merge-candidate-3-v1~506^2~32
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9651320ad2ac2e45d362f743d7bcca7fb386a27e;p=thirdparty%2Fsquid.git

Update HTTP-version parsing with RFC 7230 octet magics

RFC 7230 replaces RFC 2616 and defines HTTP-version for HTTP/1 protocol
as having exact case-sensitive octets "HTTP/1." and a variable minor
version consisting of exactly one DIGIT.

This allows us to use magic-octet matching to detect the HTTP-version
field and remove slow matching logics for unknown version and HTTP major
version number (DIGIT '1').
---

diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc
index c6b406a5b9..5d39c49089 100644
--- a/src/http/one/Parser.cc
+++ b/src/http/one/Parser.cc
@@ -2,6 +2,9 @@
 #include "Debug.h"
 #include "http/one/Parser.h"
 
+/// RFC 7230 section 2.6 - 7 magic octets
+const SBuf Http::One::Parser::Http1magic("HTTP/1.");
+
 void
 Http::One::Parser::clear()
 {
diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h
index aacb0def38..9b9717853f 100644
--- a/src/http/one/Parser.h
+++ b/src/http/one/Parser.h
@@ -72,6 +72,9 @@ public:
     char *getHeaderField(const char *name);
 
 public:
+    /// RFC 7230 section 2.6 - 7 magic octets
+    static const SBuf Http1magic;
+
     SBuf buf;
 
 protected:
diff --git a/src/http/one/RequestParser.cc b/src/http/one/RequestParser.cc
index e446a16c60..7640dd26b8 100644
--- a/src/http/one/RequestParser.cc
+++ b/src/http/one/RequestParser.cc
@@ -80,6 +80,7 @@ Http::One::RequestParser::skipGarbageLines()
  * Governed by:
  *  RFC 1945 section 5.1
  *  RFC 2616 section 5.1
+ *  RFC 7230
  *
  * Parsing state is stored between calls. However the current implementation
  * begins parsing from scratch on every call.
@@ -240,48 +241,18 @@ Http::One::RequestParser::parseRequestFirstLine()
     req.v_start = last_whitespace + 1;
     req.v_end = line_end;
 
-    // We only accept HTTP protocol requests right now.
-    // TODO: accept other protocols; RFC 2326 (RTSP protocol) etc
-    if ((req.v_end - req.v_start +1) < 5 || buf.substr(req.v_start, 5).caseCmp(SBuf("HTTP/")) != 0) {
-#if USE_HTTP_VIOLATIONS
-        // being lax; old parser accepted strange versions
-        // there is a LOT of cases which are ambiguous, therefore we cannot use relaxed_header_parser here.
-        msgProtocol_ = Http::ProtocolVersion(0,9);
-        req.u_end = line_end;
-        request_parse_status = Http::scOkay; // treat as HTTP/0.9
-        return 1;
-#else
-        // protocol not supported / implemented.
+    /* RFC 2616 section 10.5.6 : handle unsupported HTTP major versions cleanly. */
+    if ((req.v_end - req.v_start +1) < (int)Http1magic.length() || !buf.substr(req.v_start, SBuf::npos).startsWith(Http1magic)) {
+        // non-HTTP/1 protocols not supported / implemented.
         request_parse_status = Http::scHttpVersionNotSupported;
         return -1;
-#endif
     }
+    // NP: magic octets include the protocol name and major version DIGIT.
     msgProtocol_.protocol = AnyP::PROTO_HTTP;
+    msgProtocol_.major = 1;
 
-    int i = req.v_start + sizeof("HTTP/") -1;
+    int i = req.v_start + Http1magic.length() -1;
 
-    /* next should be 1 or more digits */
-    if (!isdigit(buf[i])) {
-        request_parse_status = Http::scHttpVersionNotSupported;
-        return -1;
-    }
-    int maj = 0;
-    for (; i <= line_end && (isdigit(buf[i])) && maj < 65536; ++i) {
-        maj = maj * 10;
-        maj = maj + (buf[i]) - '0';
-    }
-    // catch too-big values or missing remainders
-    if (maj >= 65536 || i > line_end) {
-        request_parse_status = Http::scHttpVersionNotSupported;
-        return -1;
-    }
-    msgProtocol_.major = maj;
-
-    /* next should be .; we -have- to have this as we have a whole line.. */
-    if (buf[i] != '.') {
-        request_parse_status = Http::scHttpVersionNotSupported;
-        return -1;
-    }
     // catch missing minor part
     if (++i > line_end) {
         request_parse_status = Http::scHttpVersionNotSupported;
@@ -304,13 +275,6 @@ Http::One::RequestParser::parseRequestFirstLine()
     }
     msgProtocol_.minor = min;
 
-    /* RFC 2616 section 10.5.6 : handle unsupported HTTP major versions cleanly. */
-    /* We currently only support 0.9, 1.0, 1.1 properly in this parser */
-    if ((maj == 0 && min != 9) || (maj > 1)) {
-        request_parse_status = Http::scHttpVersionNotSupported;
-        return -1;
-    }
-
     /*
      * Rightio - we have all the schtuff. Return true; we've got enough.
      */
diff --git a/src/tests/testHttp1Parser.cc b/src/tests/testHttp1Parser.cc
index b5127866ac..d93075672e 100644
--- a/src/tests/testHttp1Parser.cc
+++ b/src/tests/testHttp1Parser.cc
@@ -221,10 +221,8 @@ testHttp1Parser::testParseRequestLineProtocols()
         input.clear();
     }
 
-    // RFC 2616 : future version full-request
+    // RFC 7230 : future versions do not use request-line syntax
     {
-        // IETF HTTPbis WG has made this two-digits format invalid.
-        // it gets treated same as HTTP/0.9 for now
         input.append("GET / HTTP/10.12\r\n", 18);
         struct resultSet expect = {
             .parsed = false,
@@ -242,7 +240,7 @@ testHttp1Parser::testParseRequestLineProtocols()
             .uri = "/",
             .versionStart = 6,
             .versionEnd = 15,
-            .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,10,12)
+            .version = AnyP::ProtocolVersion()
         };
         output.clear();
         testResults(__LINE__, input, output, expect);
@@ -251,8 +249,6 @@ testHttp1Parser::testParseRequestLineProtocols()
 
     // unknown non-HTTP protocol names
     {
-        // XXX: violations mode treats them as HTTP/0.9 requests! which is wrong.
-#if !USE_HTTP_VIOLATIONS
         input.append("GET / FOO/1.0\n", 14);
         struct resultSet expect = {
             .parsed = false,
@@ -275,7 +271,6 @@ testHttp1Parser::testParseRequestLineProtocols()
         output.clear();
         testResults(__LINE__, input, output, expect);
         input.clear();
-#endif
     }
 
     // no version
@@ -297,7 +292,7 @@ testHttp1Parser::testParseRequestLineProtocols()
             .uri = "/",
             .versionStart = 6,
             .versionEnd = 10,
-            .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,0,0)
+            .version = AnyP::ProtocolVersion()
         };
         output.clear();
         testResults(__LINE__, input, output, expect);
@@ -323,7 +318,7 @@ testHttp1Parser::testParseRequestLineProtocols()
             .uri = "/",
             .versionStart = 6,
             .versionEnd = 12,
-            .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,0,0)
+            .version = AnyP::ProtocolVersion()
         };
         output.clear();
         testResults(__LINE__, input, output, expect);
@@ -349,7 +344,7 @@ testHttp1Parser::testParseRequestLineProtocols()
             .uri = "/",
             .versionStart = 6,
             .versionEnd = 12,
-            .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,0,0)
+            .version = AnyP::ProtocolVersion()
         };
         output.clear();
         testResults(__LINE__, input, output, expect);
@@ -375,7 +370,7 @@ testHttp1Parser::testParseRequestLineProtocols()
             .uri = "/",
             .versionStart = 6,
             .versionEnd = 19,
-            .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,0,0)
+            .version = AnyP::ProtocolVersion()
         };
         output.clear();
         testResults(__LINE__, input, output, expect);