diff --git a/CHANGELOG.md b/CHANGELOG.md index b0f943d8..cf019482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ Version 288: +* Fix Content-Length parsing -------------------------------------------------------------------------------- diff --git a/include/boost/beast/http/basic_parser.hpp b/include/boost/beast/http/basic_parser.hpp index 023bdd7a..9cff88b8 100644 --- a/include/boost/beast/http/basic_parser.hpp +++ b/include/boost/beast/http/basic_parser.hpp @@ -624,6 +624,10 @@ protected: on_finish_impl(error_code& ec) = 0; private: + + boost::optional + content_length_unchecked() const; + template std::size_t put_from_stack( diff --git a/include/boost/beast/http/impl/basic_parser.hpp b/include/boost/beast/http/impl/basic_parser.hpp index 1303c5bd..5b0b78e4 100644 --- a/include/boost/beast/http/impl/basic_parser.hpp +++ b/include/boost/beast/http/impl/basic_parser.hpp @@ -56,6 +56,16 @@ put(ConstBufferSequence const& buffers, buf_.get(), size}, ec); } +template +boost::optional +basic_parser:: +content_length_unchecked() const +{ + if(f_ & flagContentLength) + return len0_; + return boost::none; +} + template template std::size_t diff --git a/include/boost/beast/http/impl/basic_parser.ipp b/include/boost/beast/http/impl/basic_parser.ipp index 5cab22a8..bc9870b7 100644 --- a/include/boost/beast/http/impl/basic_parser.ipp +++ b/include/boost/beast/http/impl/basic_parser.ipp @@ -50,9 +50,7 @@ basic_parser:: content_length() const { BOOST_ASSERT(is_header_done()); - if(! (f_ & flagContentLength)) - return boost::none; - return len0_; + return content_length_unchecked(); } template @@ -786,30 +784,48 @@ do_field(field f, // Content-Length if(f == field::content_length) { - if(f_ & flagContentLength) + auto bad_content_length = [&ec] { - // duplicate ec = error::bad_content_length; - return; - } + }; + // conflicting field if(f_ & flagChunked) + return bad_content_length(); + + // Content-length may be a comma-separated list of integers + auto tokens_unprocessed = 1 + + std::count(value.begin(), value.end(), ','); + auto tokens = opt_token_list(value); + if (tokens.begin() == tokens.end() || + !validate_list(tokens)) + return bad_content_length(); + + auto existing = this->content_length_unchecked(); + for (auto tok : tokens) { - // conflicting field - ec = error::bad_content_length; - return; + std::uint64_t v; + if (!parse_dec(tok, v)) + return bad_content_length(); + --tokens_unprocessed; + if (existing.has_value()) + { + if (v != *existing) + return bad_content_length(); + } + else + { + existing = v; + } } - std::uint64_t v; - if(! parse_dec(value, v)) - { - ec = error::bad_content_length; - return; - } + if (tokens_unprocessed) + return bad_content_length(); + BOOST_ASSERT(existing.has_value()); ec = {}; - len_ = v; - len0_ = v; + len_ = *existing; + len0_ = *existing; f_ |= flagContentLength; return; } diff --git a/test/beast/http/basic_parser.cpp b/test/beast/http/basic_parser.cpp index 2cfa7b9d..f4073b74 100644 --- a/test/beast/http/basic_parser.cpp +++ b/test/beast/http/basic_parser.cpp @@ -692,6 +692,8 @@ public: parsegrind

(m("Content-LengtX: 0\r\n"), expect_flags{*this, 0}); parsegrind

(m("Content-Lengths: many\r\n"), expect_flags{*this, 0}); parsegrind

(m("Content: full\r\n"), expect_flags{*this, 0}); + parsegrind

(m("Content-Length: 0\r\n" + "Content-Length: 0\r\n"), expect_flags{*this, 0}); failgrind

(c("\r\n"), error::bad_content_length); failgrind

(c("18446744073709551616\r\n"), error::bad_content_length); @@ -699,8 +701,8 @@ public: failgrind

(c("0 1\r\n"), error::bad_content_length); failgrind

(c(",\r\n"), error::bad_content_length); failgrind

(c("0,\r\n"), error::bad_content_length); - failgrind

(m( - "Content-Length: 0\r\nContent-Length: 0\r\n"), error::bad_content_length); + failgrind

(m("Content-Length: 0\r\n" + "Content-Length: 100\r\n"), error::bad_content_length); } void diff --git a/test/beast/http/parser.cpp b/test/beast/http/parser.cpp index 6d87a759..d6842622 100644 --- a/test/beast/http/parser.cpp +++ b/test/beast/http/parser.cpp @@ -357,6 +357,65 @@ public: BEAST_EXPECT(p.need_eof()); } + void + testIssue1880() + { + // A user raised the issue that multiple Content-Length fields and + // values are permissible provided all values are the same. + // See rfc7230 section-3.3.2 + // https://tools.ietf.org/html/rfc7230#section-3.3.2 + // Credit: Dimitry Bulsunov + + auto checkPass = [&](std::string const& message) + { + response_parser parser; + error_code ec; + parser.put(net::buffer(message), ec); + BEAST_EXPECTS(!ec.failed(), ec.message()); + }; + + auto checkFail = [&](std::string const& message) + { + response_parser parser; + error_code ec; + parser.put(net::buffer(message), ec); + BEAST_EXPECTS(ec == error::bad_content_length, ec.message()); + }; + + // multiple contents lengths the same + checkPass( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 0\r\n" + "Content-Length: 0\r\n" + "\r\n"); + + // multiple contents lengths different + checkFail( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 0\r\n" + "Content-Length: 1\r\n" + "\r\n"); + + // multiple content in same header + checkPass( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 0, 0, 0\r\n" + "\r\n"); + + // multiple content in same header but mismatch (case 1) + checkFail( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 0, 0, 1\r\n" + "\r\n"); + + // multiple content in same header but mismatch (case 2) + checkFail( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 0, 0, 0\r\n" + "Content-Length: 1\r\n" + "\r\n"); + } + void run() override { @@ -366,6 +425,7 @@ public: testGotSome(); testIssue818(); testIssue1187(); + testIssue1880(); } };