Fix HTTP split parse edge case:

fix #257

This fixes a problem when doing split parsing (header then body).
The problem arises when the first buffer passed to the header
parser contains exactly the full header and nothing more. The
symptom is that when parsing the body, the parse will erroneously
be considered complete.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Split out and rename test stream classes
 * Restyle async result constructions
+* Fix HTTP split parse edge case
 
 --------------------------------------------------------------------------------
 

include/beast/http/basic_parser_v1.hpp

@@ -35,7 +35,8 @@ enum parse_flag
     trailing              =  16,
     upgrade               =  32,
     skipbody              =  64,
-    contentlength         = 128
+    contentlength         = 128,
+    paused                = 256
 };
 
 /** Body maximum size option.
@@ -280,12 +281,12 @@ private:
     std::uint64_t content_length_;
     pmf_t cb_;
     state s_              : 8;
-    unsigned flags_       : 8;
     unsigned fs_          : 8;
     unsigned pos_         : 8; // position in field state
     unsigned http_major_  : 16;
     unsigned http_minor_  : 16;
     unsigned status_code_ : 16;
+    unsigned flags_       : 9;
     bool upgrade_         : 1; // true if parser exited for upgrade
 
 public:
@@ -434,7 +435,7 @@ public:
         return
             s_ == s_restart ||
             s_ == s_closed_complete ||
-            s_ == s_body_pause;
+            (flags_ & parse_flag::paused);
     }
 
     /** Write a sequence of buffers to the parser.

include/beast/http/impl/basic_parser_v1.ipp

@@ -45,6 +45,7 @@ namespace http {
 template<bool isRequest, class Derived>
 basic_parser_v1<isRequest, Derived>::
 basic_parser_v1()
+    : flags_(0)
 {
     init();
 }
@@ -61,12 +62,12 @@ basic_parser_v1(basic_parser_v1<
     , content_length_(other.content_length_)
     , cb_(nullptr)
     , s_(other.s_)
-    , flags_(other.flags_)
     , fs_(other.fs_)
     , pos_(other.pos_)
     , http_major_(other.http_major_)
     , http_minor_(other.http_minor_)
     , status_code_(other.status_code_)
+    , flags_(other.flags_)
     , upgrade_(other.upgrade_)
 {
     BOOST_ASSERT(! other.cb_);
@@ -88,13 +89,14 @@ operator=(basic_parser_v1<
     content_length_ = other.content_length_;
     cb_ = nullptr;
     s_ = other.s_;
-    flags_ = other.flags_;
     fs_ = other.fs_;
     pos_ = other.pos_;
     http_major_ = other.http_major_;
     http_minor_ = other.http_minor_;
     status_code_ = other.status_code_;
+    flags_ = other.flags_;
     upgrade_ = other.upgrade_;
+    flags_ &= ~parse_flag::paused;
     return *this;
 }
 
@@ -966,6 +968,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             case body_what::pause:
                 ++p;
                 s_ = s_body_pause;
+                flags_ |= parse_flag::paused;
                 return used();
             }
             s_ = s_headers_done;

test/http/parser_v1.cpp

@@ -25,64 +25,8 @@ class parser_v1_test
     , public test::enable_yield_to
 {
 public:
-    void testRegressions()
+    void
-    {
+    testParse()
-        using boost::asio::buffer;
-
-        // consecutive empty header values
-        {
-            error_code ec;
-            parser_v1<true, string_body, fields> p;
-            std::string const s =
-                "GET / HTTP/1.1\r\n"
-                "X1:\r\n"
-                "X2:\r\n"
-                "X3:x\r\n"
-                "\r\n";
-            p.write(buffer(s), ec);
-            if(! BEAST_EXPECTS(! ec, ec.message()))
-                return;
-            BEAST_EXPECT(p.complete());
-            auto const msg = p.release();
-            BEAST_EXPECT(msg.fields.exists("X1"));
-            BEAST_EXPECT(msg.fields["X1"] == "");
-            BEAST_EXPECT(msg.fields.exists("X2"));
-            BEAST_EXPECT(msg.fields["X2"] == "");
-            BEAST_EXPECT(msg.fields.exists("X3"));
-            BEAST_EXPECT(msg.fields["X3"] == "x");
-        }
-    }
-
-    void testWithBody()
-    {
-        test::string_istream ss{ios_,
-            "GET / HTTP/1.1\r\n"
-            "User-Agent: test\r\n"
-            "Content-Length: 1\r\n"
-            "\r\n"
-            "*"};
-        streambuf rb;
-        header_parser_v1<true, fields> p0;
-        parse(ss, rb, p0);
-        request_header const& reqh = p0.get();
-        BEAST_EXPECT(reqh.method == "GET");
-        BEAST_EXPECT(reqh.url == "/");
-        BEAST_EXPECT(reqh.version == 11);
-        BEAST_EXPECT(reqh.fields["User-Agent"] == "test");
-        BEAST_EXPECT(reqh.fields["Content-Length"] == "1");
-        parser_v1<true, string_body, fields> p =
-            with_body<string_body>(p0);
-        BEAST_EXPECT(p.get().method == "GET");
-        BEAST_EXPECT(p.get().url == "/");
-        BEAST_EXPECT(p.get().version == 11);
-        BEAST_EXPECT(p.get().fields["User-Agent"] == "test");
-        BEAST_EXPECT(p.get().fields["Content-Length"] == "1");
-        parse(ss, rb, p);
-        request<string_body, fields> req = p.release();
-        BEAST_EXPECT(req.body == "*");
-    }
-
-    void run() override
     {
         using boost::asio::buffer;
         {
@@ -138,9 +82,75 @@ public:
             BEAST_EXPECT(! ec);
             BEAST_EXPECT(p.complete());
         }
+    }
 
-        testRegressions();
+    void
+    testWithBody()
+    {
+        std::string const raw =
+            "GET / HTTP/1.1\r\n"
+            "User-Agent: test\r\n"
+            "Content-Length: 1\r\n"
+            "\r\n"
+            "*";
+        test::string_istream ss{
+            ios_, raw, raw.size() - 1};
+
+        streambuf rb;
+        header_parser_v1<true, fields> p0;
+        parse(ss, rb, p0);
+        request_header const& reqh = p0.get();
+        BEAST_EXPECT(reqh.method == "GET");
+        BEAST_EXPECT(reqh.url == "/");
+        BEAST_EXPECT(reqh.version == 11);
+        BEAST_EXPECT(reqh.fields["User-Agent"] == "test");
+        BEAST_EXPECT(reqh.fields["Content-Length"] == "1");
+        parser_v1<true, string_body, fields> p =
+            with_body<string_body>(p0);
+        BEAST_EXPECT(p.get().method == "GET");
+        BEAST_EXPECT(p.get().url == "/");
+        BEAST_EXPECT(p.get().version == 11);
+        BEAST_EXPECT(p.get().fields["User-Agent"] == "test");
+        BEAST_EXPECT(p.get().fields["Content-Length"] == "1");
+        parse(ss, rb, p);
+        request<string_body, fields> req = p.release();
+        BEAST_EXPECT(req.body == "*");
+    }
+
+    void
+    testRegressions()
+    {
+        using boost::asio::buffer;
+
+        // consecutive empty header values
+        {
+            error_code ec;
+            parser_v1<true, string_body, fields> p;
+            std::string const s =
+                "GET / HTTP/1.1\r\n"
+                "X1:\r\n"
+                "X2:\r\n"
+                "X3:x\r\n"
+                "\r\n";
+            p.write(buffer(s), ec);
+            if(! BEAST_EXPECTS(! ec, ec.message()))
+                return;
+            BEAST_EXPECT(p.complete());
+            auto const msg = p.release();
+            BEAST_EXPECT(msg.fields.exists("X1"));
+            BEAST_EXPECT(msg.fields["X1"] == "");
+            BEAST_EXPECT(msg.fields.exists("X2"));
+            BEAST_EXPECT(msg.fields["X2"] == "");
+            BEAST_EXPECT(msg.fields.exists("X3"));
+            BEAST_EXPECT(msg.fields["X3"] == "x");
+        }
+    }
+
+    void run() override
+    {
+        testParse();
         testWithBody();
+        testRegressions();
     }
 };