Fix Content-Length parsing

Fix #1880

CHANGELOG.md

@@ -1,5 +1,6 @@
 Version 288:
 
+* Fix Content-Length parsing
 
 --------------------------------------------------------------------------------
 

include/boost/beast/http/basic_parser.hpp

@@ -624,6 +624,10 @@ protected:
     on_finish_impl(error_code& ec) = 0;
 
 private:
+
+    boost::optional<std::uint64_t>
+    content_length_unchecked() const;
+
     template<class ConstBufferSequence>
     std::size_t
     put_from_stack(

include/boost/beast/http/impl/basic_parser.hpp

@@ -56,6 +56,16 @@ put(ConstBufferSequence const& buffers,
         buf_.get(), size}, ec);
 }
 
+template<bool isRequest>
+boost::optional<std::uint64_t>
+basic_parser<isRequest>::
+content_length_unchecked() const
+{
+    if(f_ & flagContentLength)
+        return len0_;
+    return boost::none;
+}
+
 template<bool isRequest>
 template<class ConstBufferSequence>
 std::size_t

include/boost/beast/http/impl/basic_parser.ipp

@@ -50,9 +50,7 @@ basic_parser<isRequest>::
 content_length() const
 {
     BOOST_ASSERT(is_header_done());
-    if(! (f_ & flagContentLength))
+    return content_length_unchecked();
-        return boost::none;
-    return len0_;
 }
 
 template<bool isRequest>
@@ -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
+            std::uint64_t v;
-            ec = error::bad_content_length;
+            if (!parse_dec(tok, v))
-            return;
+                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 (tokens_unprocessed)
-        if(! parse_dec(value, v))
+            return bad_content_length();
-        {
-            ec = error::bad_content_length;
-            return;
-        }
 
+        BOOST_ASSERT(existing.has_value());
         ec = {};
-        len_ = v;
+        len_ = *existing;
-        len0_ = v;
+        len0_ = *existing;
         f_ |= flagContentLength;
         return;
     }

test/beast/http/basic_parser.cpp

@@ -692,6 +692,8 @@ public:
         parsegrind<P>(m("Content-LengtX: 0\r\n"),           expect_flags{*this, 0});
         parsegrind<P>(m("Content-Lengths: many\r\n"),       expect_flags{*this, 0});
         parsegrind<P>(m("Content: full\r\n"),               expect_flags{*this, 0});
+        parsegrind<P>(m("Content-Length: 0\r\n"
+                        "Content-Length: 0\r\n"),           expect_flags{*this, 0});
 
         failgrind<P>(c("\r\n"),                             error::bad_content_length);
         failgrind<P>(c("18446744073709551616\r\n"),         error::bad_content_length);
@@ -699,8 +701,8 @@ public:
         failgrind<P>(c("0 1\r\n"),                          error::bad_content_length);
         failgrind<P>(c(",\r\n"),                            error::bad_content_length);
         failgrind<P>(c("0,\r\n"),                           error::bad_content_length);
-        failgrind<P>(m(
+        failgrind<P>(m("Content-Length: 0\r\n"
-            "Content-Length: 0\r\nContent-Length: 0\r\n"),  error::bad_content_length);
+                        "Content-Length: 100\r\n"),         error::bad_content_length);
     }
 
     void

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<string_body> parser;
+            error_code ec;
+            parser.put(net::buffer(message), ec);
+            BEAST_EXPECTS(!ec.failed(), ec.message());
+        };
+
+        auto checkFail = [&](std::string const& message)
+        {
+            response_parser<string_body> 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();
     }
 };