Explorar o código

End the stream at the header section for HEAD, 204 and 304 in open_stream (#2636)

ClientImpl::open_stream built the body reader from Content-Length and Transfer-Encoding whatever the method or status, so a stream on a response that RFC 9112 6.3 ends at its header section was read on: with a keep-alive server read() blocked until the read timeout, and when the server closed it reported a read error for a complete response. stream::Head() against cpp-httplib's own server, which puts the GET body's Content-Length on a HEAD response, hit it every time.

open_stream_() now returns with the body reader at its end for a response to HEAD and for a 204 or 304, as the buffered path skips the body for them. Content-Encoding is skipped too, so a coding the build lacks no longer fails such a stream.
metsw24-max hai 2 días
pai
achega
67b88030e8
Modificáronse 2 ficheiros con 51 adicións e 9 borrados
  1. 14 9
      httplib.h
  2. 37 0
      test/test.cc

+ 14 - 9
httplib.h

@@ -15939,20 +15939,25 @@ inline ClientImpl::StreamHandle ClientImpl::open_stream_(Request &req) {
     return handle;
   }
 
-  // Same framing check as ClientImpl::process_request(). A HEAD or bodyless
-  // (204/304) response legitimately carries framing headers with no body.
-  if (req.method != "HEAD" &&
-      handle.response->status != StatusCode::NoContent_204 &&
-      handle.response->status != StatusCode::NotModified_304 &&
-      detail::has_conflicting_content_length(handle.response->headers)) {
+  handle.body_reader_.stream = handle.stream_;
+  handle.body_reader_.payload_max_length = payload_max_length_;
+
+  // RFC 9112 6.3: a response to HEAD, and a 204 or 304 response, ends with its
+  // header section, so none of its framing or coding headers apply.
+  if (req.method == "HEAD" ||
+      handle.response->status == StatusCode::NoContent_204 ||
+      handle.response->status == StatusCode::NotModified_304) {
+    handle.body_reader_.eof = true;
+    return handle;
+  }
+
+  // Same framing check as ClientImpl::process_request()
+  if (detail::has_conflicting_content_length(handle.response->headers)) {
     handle.error = Error::Read;
     handle.response.reset();
     return handle;
   }
 
-  handle.body_reader_.stream = handle.stream_;
-  handle.body_reader_.payload_max_length = payload_max_length_;
-
   if (handle.response->has_header("Content-Length")) {
     bool is_invalid = false;
     auto content_length = detail::get_header_value_u64(

+ 37 - 0
test/test.cc

@@ -23742,6 +23742,43 @@ TEST(ClientResponseSmugglingTest, BodylessResponseNotRejected) {
   }
 }
 
+// RFC 9112 6.3: a response to HEAD, and a 204 or 304 response, ends with its
+// header section, so a stream opened on one has nothing to read, whatever
+// framing or coding headers it carries.
+TEST(ClientResponseSmugglingTest, BodylessResponseStreamEndsAtOnce) {
+  struct Case {
+    const char *method;
+    const char *status;
+  };
+  // A reader of the declared body reports an early close when nothing
+  // follows, and hands back what does.
+  struct Framing {
+    const char *headers;
+    const char *after;
+  };
+  for (const auto &c : {Case{"HEAD", "200 OK"}, Case{"GET", "204 No Content"},
+                        Case{"GET", "304 Not Modified"}}) {
+    for (const auto &f :
+         {Framing{"Content-Length: 5\r\n", ""},
+          Framing{"Content-Length: 5\r\n", "hello"},
+          Framing{"Transfer-Encoding: chunked\r\n", ""},
+          Framing{"Transfer-Encoding: chunked\r\n", "5\r\nhello\r\n0\r\n\r\n"},
+          // Not decoded, so a build without Brotli opens it too
+          Framing{"Content-Encoding: br\r\n", ""}}) {
+      auto response = std::string("HTTP/1.1 ") + c.status + "\r\n" + f.headers +
+                      "Connection: close\r\n"
+                      "\r\n" +
+                      f.after;
+      with_single_response(response, [&](Client &cli) {
+        auto handle = cli.open_stream(c.method, "/");
+        ASSERT_TRUE(handle.is_valid()) << c.method << ' ' << response;
+        EXPECT_EQ("", read_all(handle)) << c.method << ' ' << response;
+        EXPECT_FALSE(handle.has_read_error()) << c.method << ' ' << response;
+      });
+    }
+  }
+}
+
 // RFC 9112 6.1: Transfer-Encoding next to a Content-Length, or in an HTTP/1.0
 // response, is faulty framing, so the connection is not reused after it.
 TEST(ClientResponseSmugglingTest, FaultyTransferEncodingClosesConnection) {