Merge pull request #19077 from Techatrix/http-header-parse

http: check for empty header name instead of value

lib/std/http/Client.zig

@@ -488,7 +488,7 @@ pub const Response = struct {
             var line_it = mem.splitSequence(u8, line, ": ");
             const header_name = line_it.next().?;
             const header_value = line_it.rest();
-            if (header_value.len == 0) return error.HttpHeadersInvalid;
+            if (header_name.len == 0) return error.HttpHeadersInvalid;
 
             if (std.ascii.eqlIgnoreCase(header_name, "connection")) {
                 res.keep_alive = !std.ascii.eqlIgnoreCase(header_value, "close");
@@ -774,7 +774,7 @@ pub const Request = struct {
         }
 
         for (req.extra_headers) |header| {
-            assert(header.value.len != 0);
+            assert(header.name.len != 0);
 
             try w.writeAll(header.name);
             try w.writeAll(": ");
@@ -1515,11 +1515,13 @@ pub fn open(
 ) RequestError!Request {
     if (std.debug.runtime_safety) {
         for (options.extra_headers) |header| {
+            assert(header.name.len != 0);
             assert(std.mem.indexOfScalar(u8, header.name, ':') == null);
             assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null);
             assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null);
         }
         for (options.privileged_headers) |header| {
+            assert(header.name.len != 0);
             assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null);
             assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null);
         }

lib/std/http/HeaderIterator.zig

@@ -15,7 +15,7 @@ pub fn next(it: *HeaderIterator) ?std.http.Header {
     var kv_it = std.mem.splitSequence(u8, it.bytes[it.index..end], ": ");
     const name = kv_it.next().?;
     const value = kv_it.rest();
-    if (value.len == 0) {
+    if (name.len == 0 and value.len == 0) {
         if (it.is_trailer) return null;
         const next_end = std.mem.indexOfPosLinear(u8, it.bytes, end + 2, "\r\n") orelse
             return null;
@@ -35,7 +35,7 @@ pub fn next(it: *HeaderIterator) ?std.http.Header {
 }
 
 test next {
-    var it = HeaderIterator.init("200 OK\r\na: b\r\nc: d\r\n\r\ne: f\r\n\r\n");
+    var it = HeaderIterator.init("200 OK\r\na: b\r\nc: \r\nd: e\r\n\r\nf: g\r\n\r\n");
     try std.testing.expect(!it.is_trailer);
     {
         const header = it.next().?;
@@ -47,13 +47,19 @@ test next {
         const header = it.next().?;
         try std.testing.expect(!it.is_trailer);
         try std.testing.expectEqualStrings("c", header.name);
-        try std.testing.expectEqualStrings("d", header.value);
+        try std.testing.expectEqualStrings("", header.value);
+    }
+    {
+        const header = it.next().?;
+        try std.testing.expect(!it.is_trailer);
+        try std.testing.expectEqualStrings("d", header.name);
+        try std.testing.expectEqualStrings("e", header.value);
     }
     {
         const header = it.next().?;
         try std.testing.expect(it.is_trailer);
-        try std.testing.expectEqualStrings("e", header.name);
+        try std.testing.expectEqualStrings("f", header.name);
-        try std.testing.expectEqualStrings("f", header.value);
+        try std.testing.expectEqualStrings("g", header.value);
     }
     try std.testing.expectEqual(null, it.next());
 }

lib/std/http/Server.zig

@@ -211,7 +211,7 @@ pub const Request = struct {
                 var line_it = mem.splitSequence(u8, line, ": ");
                 const header_name = line_it.next().?;
                 const header_value = line_it.rest();
-                if (header_value.len == 0) return error.HttpHeadersInvalid;
+                if (header_name.len == 0) return error.HttpHeadersInvalid;
 
                 if (std.ascii.eqlIgnoreCase(header_name, "connection")) {
                     head.keep_alive = !std.ascii.eqlIgnoreCase(header_value, "close");
@@ -311,6 +311,7 @@ pub const Request = struct {
         assert(options.extra_headers.len <= max_extra_headers);
         if (std.debug.runtime_safety) {
             for (options.extra_headers) |header| {
+                assert(header.name.len != 0);
                 assert(std.mem.indexOfScalar(u8, header.name, ':') == null);
                 assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null);
                 assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null);
@@ -370,11 +371,13 @@ pub const Request = struct {
             };
             iovecs_len += 1;
 
-            iovecs[iovecs_len] = .{
+            if (header.value.len != 0) {
-                .iov_base = header.value.ptr,
+                iovecs[iovecs_len] = .{
-                .iov_len = header.value.len,
+                    .iov_base = header.value.ptr,
-            };
+                    .iov_len = header.value.len,
-            iovecs_len += 1;
+                };
+                iovecs_len += 1;
+            }
 
             iovecs[iovecs_len] = .{
                 .iov_base = "\r\n",
@@ -496,6 +499,7 @@ pub const Request = struct {
             }
 
             for (o.extra_headers) |header| {
+                assert(header.name.len != 0);
                 h.appendSliceAssumeCapacity(header.name);
                 h.appendSliceAssumeCapacity(": ");
                 h.appendSliceAssumeCapacity(header.value);
@@ -986,11 +990,13 @@ pub const Response = struct {
                 };
                 iovecs_len += 1;
 
-                iovecs[iovecs_len] = .{
+                if (trailer.value.len != 0) {
-                    .iov_base = trailer.value.ptr,
+                    iovecs[iovecs_len] = .{
-                    .iov_len = trailer.value.len,
+                        .iov_base = trailer.value.ptr,
-                };
+                        .iov_len = trailer.value.len,
-                iovecs_len += 1;
+                    };
+                    iovecs_len += 1;
+                }
 
                 iovecs[iovecs_len] = .{
                     .iov_base = "\r\n",

lib/std/http/test.zig

@@ -490,6 +490,12 @@ test "general client/server API coverage" {
                         .{ .name = "location", .value = location },
                     },
                 });
+            } else if (mem.eql(u8, request.head.target, "/empty")) {
+                try request.respond("", .{
+                    .extra_headers = &.{
+                        .{ .name = "empty", .value = "" },
+                    },
+                });
             } else {
                 try request.respond("", .{ .status = .not_found });
             }
@@ -502,7 +508,10 @@ test "general client/server API coverage" {
             return s.listen_address.in.getPort();
         }
     });
-    defer test_server.destroy();
+    defer {
+        global.handle_new_requests = false;
+        test_server.destroy();
+    }
 
     const log = std.log.scoped(.client);
 
@@ -665,6 +674,56 @@ test "general client/server API coverage" {
     // connection has been closed
     try expect(client.connection_pool.free_len == 0);
 
+    { // handle empty header field value
+        const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/empty", .{port});
+        defer gpa.free(location);
+        const uri = try std.Uri.parse(location);
+
+        log.info("{s}", .{location});
+        var server_header_buffer: [1024]u8 = undefined;
+        var req = try client.open(.GET, uri, .{
+            .server_header_buffer = &server_header_buffer,
+            .extra_headers = &.{
+                .{ .name = "empty", .value = "" },
+            },
+        });
+        defer req.deinit();
+
+        try req.send(.{});
+        try req.wait();
+
+        try std.testing.expectEqual(.ok, req.response.status);
+
+        const body = try req.reader().readAllAlloc(gpa, 8192);
+        defer gpa.free(body);
+
+        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);
+            try expectEqualStrings("content-length", header.name);
+            try expectEqualStrings("0", header.value);
+        }
+        {
+            const header = it.next().?;
+            try expect(!it.is_trailer);
+            try expectEqualStrings("empty", header.name);
+            try expectEqualStrings("", header.value);
+        }
+        try expectEqual(null, it.next());
+    }
+
+    // connection has been kept alive
+    try expect(client.http_proxy != null or client.connection_pool.free_len == 1);
+
     { // relative redirect
         const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/redirect/1", .{port});
         defer gpa.free(location);

lib/std/mem.zig

@@ -1346,6 +1346,7 @@ pub fn lastIndexOfLinear(comptime T: type, haystack: []const T, needle: []const
 /// Consider using `indexOfPos` instead of this, which will automatically use a
 /// more sophisticated algorithm on larger inputs.
 pub fn indexOfPosLinear(comptime T: type, haystack: []const T, start_index: usize, needle: []const T) ?usize {
+    if (needle.len > haystack.len) return null;
     var i: usize = start_index;
     const end = haystack.len - needle.len;
     while (i <= end) : (i += 1) {
@@ -1354,6 +1355,26 @@ pub fn indexOfPosLinear(comptime T: type, haystack: []const T, start_index: usiz
     return null;
 }
 
+test indexOfPosLinear {
+    try testing.expectEqual(0, indexOfPosLinear(u8, "", 0, ""));
+    try testing.expectEqual(0, indexOfPosLinear(u8, "123", 0, ""));
+
+    try testing.expectEqual(null, indexOfPosLinear(u8, "", 0, "1"));
+    try testing.expectEqual(0, indexOfPosLinear(u8, "1", 0, "1"));
+    try testing.expectEqual(null, indexOfPosLinear(u8, "2", 0, "1"));
+    try testing.expectEqual(1, indexOfPosLinear(u8, "21", 0, "1"));
+    try testing.expectEqual(null, indexOfPosLinear(u8, "222", 0, "1"));
+
+    try testing.expectEqual(null, indexOfPosLinear(u8, "", 0, "12"));
+    try testing.expectEqual(null, indexOfPosLinear(u8, "1", 0, "12"));
+    try testing.expectEqual(null, indexOfPosLinear(u8, "2", 0, "12"));
+    try testing.expectEqual(0, indexOfPosLinear(u8, "12", 0, "12"));
+    try testing.expectEqual(null, indexOfPosLinear(u8, "21", 0, "12"));
+    try testing.expectEqual(1, indexOfPosLinear(u8, "212", 0, "12"));
+    try testing.expectEqual(0, indexOfPosLinear(u8, "122", 0, "12"));
+    try testing.expectEqual(1, indexOfPosLinear(u8, "212112", 0, "12"));
+}
+
 fn boyerMooreHorspoolPreprocessReverse(pattern: []const u8, table: *[256]usize) void {
     for (table) |*c| {
         c.* = pattern.len;