Commit 4e2570baaf

Jacob Young <jacobly0@users.noreply.github.com>
2024-02-26 17:04:28
http: fix fetching a github release
* Support different keep alive defaults with different http versions. * Fix incorrect usage of `copyBackwards`, which copies in a backwards direction allowing data to be moved forward in a buffer, not backwards in a buffer.
1 parent 8775d8b
Changed files (4)
lib/std/http/Client.zig
@@ -430,7 +430,7 @@ pub const Response = struct {
     /// Points into the user-provided `server_header_buffer`.
     content_disposition: ?[]const u8 = null,
 
-    keep_alive: bool = false,
+    keep_alive: bool,
 
     /// If present, the number of bytes in the response body.
     content_length: ?u64 = null,
@@ -477,6 +477,10 @@ pub const Response = struct {
         res.version = version;
         res.status = status;
         res.reason = reason;
+        res.keep_alive = switch (version) {
+            .@"HTTP/1.0" => false,
+            .@"HTTP/1.1" => true,
+        };
 
         while (it.next()) |line| {
             if (line.len == 0) return;
@@ -684,9 +688,10 @@ pub const Request = struct {
         req.response.parser.reset();
 
         req.response = .{
+            .version = undefined,
             .status = undefined,
             .reason = undefined,
-            .version = undefined,
+            .keep_alive = undefined,
             .parser = req.response.parser,
         };
     }
@@ -1564,9 +1569,10 @@ pub fn open(
         .redirect_behavior = options.redirect_behavior,
         .handle_continue = options.handle_continue,
         .response = .{
+            .version = undefined,
             .status = undefined,
             .reason = undefined,
-            .version = undefined,
+            .keep_alive = undefined,
             .parser = proto.HeadersParser.init(options.server_header_buffer),
         },
         .headers = options.headers,
lib/std/http/Server.zig
@@ -25,7 +25,7 @@ pub const State = enum {
     /// The client is uploading something to this Server.
     receiving_body,
     /// The connection is eligible for another HTTP request, however the client
-    /// and server did not negotiate connection: keep-alive.
+    /// and server did not negotiate a persistent connection.
     closing,
 };
 
@@ -197,7 +197,10 @@ pub const Request = struct {
                 .content_length = null,
                 .transfer_encoding = .none,
                 .transfer_compression = .identity,
-                .keep_alive = false,
+                .keep_alive = switch (version) {
+                    .@"HTTP/1.0" => false,
+                    .@"HTTP/1.1" => true,
+                },
                 .compression = .none,
             };
 
@@ -330,7 +333,7 @@ pub const Request = struct {
             // reader() and hence discardBody() above sets expect to null if it
             // is handled. So the fact that it is not null here means unhandled.
             h.appendSliceAssumeCapacity("HTTP/1.1 417 Expectation Failed\r\n");
-            if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n");
+            if (!keep_alive) h.appendSliceAssumeCapacity("connection: close\r\n");
             h.appendSliceAssumeCapacity("content-length: 0\r\n\r\n");
             try request.server.connection.stream.writeAll(h.items);
             return;
@@ -339,7 +342,10 @@ pub const Request = struct {
             @tagName(options.version), @intFromEnum(options.status), phrase,
         }) catch unreachable;
 
-        if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n");
+        switch (options.version) {
+            .@"HTTP/1.0" => if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n"),
+            .@"HTTP/1.1" => if (!keep_alive) h.appendSliceAssumeCapacity("connection: close\r\n"),
+        }
 
         if (options.transfer_encoding) |transfer_encoding| switch (transfer_encoding) {
             .none => {},
@@ -480,14 +486,18 @@ pub const Request = struct {
             // reader() and hence discardBody() above sets expect to null if it
             // is handled. So the fact that it is not null here means unhandled.
             h.appendSliceAssumeCapacity("HTTP/1.1 417 Expectation Failed\r\n");
-            if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n");
+            if (!keep_alive) h.appendSliceAssumeCapacity("connection: close\r\n");
             h.appendSliceAssumeCapacity("content-length: 0\r\n\r\n");
             break :eb true;
         } else eb: {
             h.fixedWriter().print("{s} {d} {s}\r\n", .{
                 @tagName(o.version), @intFromEnum(o.status), phrase,
             }) catch unreachable;
-            if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n");
+
+            switch (o.version) {
+                .@"HTTP/1.0" => if (keep_alive) h.appendSliceAssumeCapacity("connection: keep-alive\r\n"),
+                .@"HTTP/1.1" => if (!keep_alive) h.appendSliceAssumeCapacity("connection: close\r\n"),
+            }
 
             if (o.transfer_encoding) |transfer_encoding| switch (transfer_encoding) {
                 .chunked => h.appendSliceAssumeCapacity("transfer-encoding: chunked\r\n"),
@@ -694,7 +704,7 @@ pub const Request = struct {
         }
     }
 
-    /// Returns whether the connection: keep-alive header should be sent to the client.
+    /// Returns whether the connection should remain persistent.
     /// If it would fail, it instead sets the Server state to `receiving_body`
     /// and returns false.
     fn discardBody(request: *Request, keep_alive: bool) bool {
lib/std/http/test.zig
@@ -73,12 +73,6 @@ test "trailers" {
         try expectEqualStrings("Hello, World!\n", body);
 
         var it = req.response.iterateHeaders();
-        {
-            const header = it.next().?;
-            try expect(!it.is_trailer);
-            try expectEqualStrings("connection", header.name);
-            try expectEqualStrings("keep-alive", header.value);
-        }
         {
             const header = it.next().?;
             try expect(!it.is_trailer);
@@ -143,15 +137,15 @@ test "HTTP server handles a chunked transfer coding request" {
     defer stream.close();
     try stream.writeAll(request_bytes);
 
-    const response = try stream.reader().readAllAlloc(gpa, 100);
-    defer gpa.free(response);
-
     const expected_response =
         "HTTP/1.1 200 OK\r\n" ++
+        "connection: close\r\n" ++
         "content-length: 21\r\n" ++
         "content-type: text/plain\r\n" ++
         "\r\n" ++
         "message from server!\n";
+    const response = try stream.reader().readAllAlloc(gpa, expected_response.len);
+    defer gpa.free(response);
     try expectEqualStrings(expected_response, response);
 }
 
@@ -285,7 +279,7 @@ test "Server.Request.respondStreaming non-chunked, unknown content-length" {
     var expected_response = std.ArrayList(u8).init(gpa);
     defer expected_response.deinit();
 
-    try expected_response.appendSlice("HTTP/1.1 200 OK\r\n\r\n");
+    try expected_response.appendSlice("HTTP/1.1 200 OK\r\nconnection: close\r\n\r\n");
 
     {
         var total: usize = 0;
@@ -349,6 +343,7 @@ test "receiving arbitrary http headers from the client" {
     defer expected_response.deinit();
 
     try expected_response.appendSlice("HTTP/1.1 200 OK\r\n");
+    try expected_response.appendSlice("connection: close\r\n");
     try expected_response.appendSlice("content-length: 0\r\n\r\n");
     try expectEqualStrings(expected_response.items, response);
 }
@@ -700,12 +695,6 @@ test "general client/server API coverage" {
         try expectEqualStrings("", body);
 
         var it = req.response.iterateHeaders();
-        {
-            const header = it.next().?;
-            try expect(!it.is_trailer);
-            try expectEqualStrings("connection", header.name);
-            try expectEqualStrings("keep-alive", header.value);
-        }
         {
             const header = it.next().?;
             try expect(!it.is_trailer);
lib/std/Uri.zig
@@ -367,7 +367,7 @@ pub const ResolveInplaceError = ParseError || error{OutOfMemory};
 /// If a merge needs to take place, the newly constructed path will be stored
 /// in `aux_buf` just after the copied `new`.
 pub fn resolve_inplace(base: Uri, new: []const u8, aux_buf: []u8) ResolveInplaceError!Uri {
-    std.mem.copyBackwards(u8, aux_buf, new);
+    std.mem.copyForwards(u8, aux_buf, new);
     // At this point, new is an invalid pointer.
     const new_mut = aux_buf[0..new.len];