http: dont' re-use connections which are likely in a bad state.

Some status-codes should never have a body except for a single trailing blank
line. If we don't handle these, then we end up with a dirty connection in our
connection pool:

1 - read the header, but not the body
2 - put the connection back in the pool
3 - try to read the header, but actually get the body from #1

WPT /fetch/api/basic/response-null-body.any.html exercises this path and is
flaky (because it depends whether the request goes back out on a keep-alive
connection)..but for a given run,you'll almost always get 1-3 failures.

This commit processes the request, but tells libcurl not to re-use the
connection.

src/network/HttpClient.zig

@@ -3312,6 +3312,7 @@ pub const Transfer = struct {
 
         // Set callbacks and per-client settings on the pooled connection.
         try conn.setWriteCallback(Transfer.dataCallback);
+        try conn.setHeaderCallback(Transfer.headerCallback);
         try conn.setFollowLocation(false);
         try conn.setProxy(client.http_proxy);
         try conn.setTlsVerify(client.tls_verify, client.use_proxy);
@@ -3746,6 +3747,57 @@ pub const Transfer = struct {
         self.abortParked(error.AbortAuthChallenge);
     }
 
+    // The only reason we hook into this is to try to detect bad responses which
+    // makes it so we can't re-use the connection. We're quick to return a
+    // connection to the pool, so this is the first and last place we can check
+    // this in all cases
+    fn headerCallback(buffer: [*]const u8, chunk_count: usize, chunk_len: usize, data: *anyopaque) callconv(.c) usize {
+        if (comptime lp.IS_DEBUG) {
+            // libcurl emits 1 header line at a time
+            std.debug.assert(chunk_count == 1);
+        }
+
+        if (announcesBody(buffer[0..chunk_len]) == false) {
+            return chunk_len;
+        }
+
+        // If we're here, then the header line announces a body.Let's make sure
+        // the rest of the header agrees that this should have a body
+
+        const conn: *http.Connection = @ptrCast(@alignCast(data));
+        const status = conn.getResponseCode() catch |err| {
+            log.err(.http, "getResponseCode", .{ .err = err, .source = "header callback" });
+            return chunk_len;
+        };
+
+        if ((status >= 100 and status < 200) or status == 204 or status == 304) {
+            // We received a response with a body-less status code but that says
+            // it has a body. This connection isn't safe to re-use.
+            conn.setForbidReuse() catch |err| {
+                log.err(.http, "forbid reuse", .{ .err = err, .source = "header callback" });
+            };
+        }
+
+        return chunk_len;
+    }
+
+    // Whether this response header line claims the message has a body.
+    fn announcesBody(line: []const u8) bool {
+        if (std.ascii.startsWithIgnoreCase(line, "transfer-encoding:")) {
+            return true;
+        }
+        const prefix = "content-length:";
+        if (std.ascii.startsWithIgnoreCase(line, prefix) == false) {
+            return false;
+        }
+        const value = std.mem.trim(u8, line[prefix.len..], " \t\r\n");
+        // If we can't parse it, treat it as though it announces a body. This is
+        // safer as we're using this to determine if the connection can be
+        // kept-alive.
+        const length = std.fmt.parseInt(u64, value, 10) catch return true;
+        return length > 0;
+    }
+
     fn dataCallback(buffer: [*]const u8, chunk_count: usize, chunk_len: usize, data: *anyopaque) callconv(.c) usize {
         // libcurl should only ever emit 1 chunk at a time
         if (comptime lp.IS_DEBUG) {
@@ -5892,3 +5944,15 @@ test "HttpClient: throttled navigations wait for their per-host slot" {
     try testing.expectEqual(null, client.delayed_queue.first);
     try testing.expectEqual(null, client.pending_queue.first);
 }
+
+test "HttpClient: bodyless status announcing a body" {
+    try testing.expectEqual(true, Transfer.announcesBody("Content-Length: 11\r\n"));
+    try testing.expectEqual(true, Transfer.announcesBody("content-length:11\r\n"));
+    try testing.expectEqual(true, Transfer.announcesBody("Transfer-Encoding: chunked\r\n"));
+    // Nothing good comes of reusing a connection whose framing we can't read.
+    try testing.expectEqual(true, Transfer.announcesBody("Content-Length: nope\r\n"));
+
+    try testing.expectEqual(false, Transfer.announcesBody("Content-Length: 0\r\n"));
+    try testing.expectEqual(false, Transfer.announcesBody("Content-Type: text/html\r\n"));
+    try testing.expectEqual(false, Transfer.announcesBody("\r\n"));
+}

src/network/http.zig

@@ -420,6 +420,13 @@ pub const Connection = struct {
         try libcurl.curl_easy_setopt(self._easy, .connect_only, value);
     }
 
+    // Close this connection when the transfer ends instead of returning it to
+    // libcurl's keepalive pool. Read by libcurl when the transfer completes,
+    // so it can be set while the response is being received.
+    pub fn setForbidReuse(self: *const Connection) !void {
+        try libcurl.curl_easy_setopt(self._easy, .forbid_reuse, true);
+    }
+
     pub fn setWriteCallback(
         self: *Connection,
         comptime data_cb: libcurl.CurlWriteFunction,

src/sys/libcurl.zig

@@ -225,6 +225,7 @@ const CurlOption = enum(c.CURLoption) {
     read_data = c.CURLOPT_READDATA,
     read_function = c.CURLOPT_READFUNCTION,
     connect_only = c.CURLOPT_CONNECT_ONLY,
+    forbid_reuse = c.CURLOPT_FORBID_REUSE,
     pipewait = c.CURLOPT_PIPEWAIT,
     upload = c.CURLOPT_UPLOAD,
     opensocket_function = c.CURLOPT_OPENSOCKETFUNCTION,
@@ -611,6 +612,7 @@ pub fn curl_easy_setopt(easy: *Curl, comptime option: CurlOption, value: anytype
         .http_get,
         .pipewait,
         .no_body,
+        .forbid_reuse,
         .ssl_verify_peer,
         .proxy_ssl_verify_peer,
         => @as(c_long, @intFromBool(value)),