From 2b799b637179056e6f0462a989de974aa76b1cbd Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 7 Feb 2017 15:38:45 -0500 Subject: [PATCH] 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 | 1 + include/beast/http/basic_parser_v1.hpp | 7 +- include/beast/http/impl/basic_parser_v1.ipp | 7 +- test/http/parser_v1.cpp | 128 +++++++++++--------- 4 files changed, 79 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd6b04b8..0b913b49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Split out and rename test stream classes * Restyle async result constructions +* Fix HTTP split parse edge case -------------------------------------------------------------------------------- diff --git a/include/beast/http/basic_parser_v1.hpp b/include/beast/http/basic_parser_v1.hpp index fae0d0b1..939cf2c6 100644 --- a/include/beast/http/basic_parser_v1.hpp +++ b/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. diff --git a/include/beast/http/impl/basic_parser_v1.ipp b/include/beast/http/impl/basic_parser_v1.ipp index 8348f399..9b1be47d 100644 --- a/include/beast/http/impl/basic_parser_v1.ipp +++ b/include/beast/http/impl/basic_parser_v1.ipp @@ -45,6 +45,7 @@ namespace http { template basic_parser_v1:: 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; diff --git a/test/http/parser_v1.cpp b/test/http/parser_v1.cpp index e9df99f3..20c207d7 100644 --- a/test/http/parser_v1.cpp +++ b/test/http/parser_v1.cpp @@ -25,64 +25,8 @@ class parser_v1_test , public test::enable_yield_to { public: - void testRegressions() - { - using boost::asio::buffer; - - // consecutive empty header values - { - error_code ec; - parser_v1 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 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 p = - with_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 req = p.release(); - BEAST_EXPECT(req.body == "*"); - } - - void run() override + void + testParse() { 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 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 p = + with_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 req = p.release(); + BEAST_EXPECT(req.body == "*"); + } + + void + testRegressions() + { + using boost::asio::buffer; + + // consecutive empty header values + { + error_code ec; + parser_v1 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(); } };