cli: handle with chunk tranfer encoding properly

See #957.
Chilledheart · May 17, 2024 · bfdfbb9 · bfdfbb9
1 parent 555389c
commit bfdfbb9
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 29 deletions.
diff --git a/src/cli/cli_connection.cpp b/src/cli/cli_connection.cpp
@@ -722,6 +722,12 @@ asio::error_code CliConnection::OnReadHttpRequest(std::shared_ptr<IOBuf> buf) {
       http_keep_alive_remaining_bytes_ += parser.content_length() + header.size() - buf->length();
       VLOG(3) << "Connection (client) " << connection_id() << " Host: " << http_host_ << " PORT: " << http_port_
               << " KEEPALIVE: " << std::boolalpha << http_is_keep_alive_;
+      if (parser.transfer_encoding_is_chunked()) {
+        // See #957
+        LOG(WARNING) << "Connection (client) " << connection_id()
+                     << " detected chunked transfer encoding, disabling keep alive handling";
+        http_is_keep_alive_ = false;
+      }
     } else {
       VLOG(3) << "Connection (client) " << connection_id() << " CONNECT: " << http_host_ << " PORT: " << http_port_;
     }

diff --git a/src/net/http_parser.cpp b/src/net/http_parser.cpp
@@ -260,18 +260,6 @@ void HttpRequestParser::ProcessHeaders(const quiche::BalsaHeaders& headers) {
       http_host_ = hostname;
       http_port_ = portnum;
     }
-    if (key == "Content-Length") {
-      std::string length = std::string(value);
-
-      std::optional<uint64_t> lengthnum_opt = StringToIntegerU64(length);
-      if (!lengthnum_opt.has_value()) {
-        VLOG(1) << "parser failed: bad http field: content-length: " << length;
-        status_ = ParserStatus::Error;
-        break;
-      }
-      const uint64_t lengthnum = lengthnum_opt.value();
-      content_length_ = lengthnum;
-    }
     if (key == "Content-Type") {
       content_type_ = std::string(value);
     }
@@ -476,11 +464,19 @@ int HttpRequestParser::status_code() const {
   return parser_->status_code;
 }
 
+uint64_t HttpRequestParser::content_length() const {
+  return parser_->content_length;
+}
+
 std::string_view HttpRequestParser::connection() const {
   using std::string_view_literals::operator""sv;
   return http_should_keep_alive(parser_) ? "Keep-Alive"sv : "Close"sv;
 }
 
+bool HttpRequestParser::transfer_encoding_is_chunked() const {
+  return parser_->flags & F_CHUNKED;
+}
+
 static int OnHttpRequestParseUrl(const char* buf, size_t len, std::string* host, uint16_t* port, int is_connect) {
   struct http_parser_url url;
 
@@ -541,17 +537,6 @@ int HttpRequestParser::OnReadHttpRequestHeaderValue(http_parser* parser, const c
     self->http_port_ = portnum;
   }
 
-  if (self->http_field_ == "Content-Length") {
-    std::string length = std::string(buf, len);
-
-    std::optional<uint64_t> lengthnum_opt = StringToIntegerU64(length);
-    if (!lengthnum_opt.has_value()) {
-      VLOG(1) << "parser failed: bad http field: content-length: " << length;
-      return -1;
-    }
-    const uint64_t lengthnum = lengthnum_opt.value();
-    self->content_length_ = lengthnum;
-  }
   if (self->http_field_ == "Content-Type") {
     self->content_type_ = std::string(buf, len);
   }

diff --git a/src/net/http_parser.hpp b/src/net/http_parser.hpp
@@ -47,9 +47,10 @@ class HttpRequestParser : public quiche::BalsaVisitorInterface {
   const std::string& host() const { return http_host_; }
   uint16_t port() const { return http_port_; }
   bool is_connect() const { return http_is_connect_; }
-  uint64_t content_length() const { return content_length_; }
+  uint64_t content_length() const { return headers_.content_length(); }
   const std::string& content_type() const { return content_type_; }
   const std::string& connection() const { return connection_; }
+  bool transfer_encoding_is_chunked() const { return headers_.transfer_encoding_is_chunked(); }
 
   void ReforgeHttpRequest(std::string* header,
                           const absl::flat_hash_map<std::string, std::string>* additional_headers = nullptr);
@@ -101,8 +102,6 @@ class HttpRequestParser : public quiche::BalsaVisitorInterface {
   absl::flat_hash_map<std::string, std::string> http_headers_;
   /// copy of connect method
   bool http_is_connect_ = false;
-  /// copy of content length
-  uint64_t content_length_ = 0;
   /// copy of content type
   std::string content_type_;
   /// copy of connection
@@ -136,9 +135,10 @@ class HttpRequestParser {
   const std::string& host() const { return http_host_; }
   uint16_t port() const { return http_port_; }
   bool is_connect() const { return http_is_connect_; }
-  uint64_t content_length() const { return content_length_; }
+  uint64_t content_length() const;
   const std::string& content_type() const { return content_type_; }
   std::string_view connection() const;
+  bool transfer_encoding_is_chunked() const;
 
   int status_code() const;
 
@@ -168,8 +168,6 @@ class HttpRequestParser {
   absl::flat_hash_map<std::string, std::string> http_headers_;
   /// copy of connect method
   bool http_is_connect_ = false;
-  /// copy of content length
-  uint64_t content_length_ = 0;
   /// copy of content type
   std::string content_type_;
 };