authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-08-06 18:37:48-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-08-07 10:04:52-07:00
log618a435ad4ea259d2e2546ad4b85e2e01f087493
tree083b90643a1fba2b0018e4bacda48ce313bb3594
parent858716aa4d569d280e9e31c91fe804eb2a29ef5d

std.http.Server: add safety for invalidated Head strings

and fix bad unit test API usage that it finds

3 files changed, 70 insertions(+), 38 deletions(-)

lib/std/http/Client.zig+15-3
...@@ -444,8 +444,8 @@ pub const Connection = struct {...@@ -444,8 +444,8 @@ pub const Connection = struct {
444444
445pub const Response = struct {445pub const Response = struct {
446 request: *Request,446 request: *Request,
447 /// Pointers in this struct are invalidated with the next call to447 /// Pointers in this struct are invalidated when the response body stream
448 /// `receiveHead`.448 /// is initialized.
449 head: Head,449 head: Head,
450450
451 pub const Head = struct {451 pub const Head = struct {
...@@ -671,6 +671,16 @@ pub const Response = struct {...@@ -671,6 +671,16 @@ pub const Response = struct {
671 try expectEqual(@as(u10, 418), parseInt3("418"));671 try expectEqual(@as(u10, 418), parseInt3("418"));
672 try expectEqual(@as(u10, 999), parseInt3("999"));672 try expectEqual(@as(u10, 999), parseInt3("999"));
673 }673 }
674
675 /// Help the programmer avoid bugs by calling this when the string
676 /// memory of `Head` becomes invalidated.
677 fn invalidateStrings(h: *Head) void {
678 h.bytes = undefined;
679 h.reason = undefined;
680 if (h.location) |*s| s.* = undefined;
681 if (h.content_type) |*s| s.* = undefined;
682 if (h.content_disposition) |*s| s.* = undefined;
683 }
674 };684 };
675685
676 /// If compressed body has been negotiated this will return compressed bytes.686 /// If compressed body has been negotiated this will return compressed bytes.
...@@ -682,7 +692,8 @@ pub const Response = struct {...@@ -682,7 +692,8 @@ pub const Response = struct {
682 ///692 ///
683 /// See also:693 /// See also:
684 /// * `readerDecompressing`694 /// * `readerDecompressing`
685 pub fn reader(response: *const Response, buffer: []u8) *Reader {695 pub fn reader(response: *Response, buffer: []u8) *Reader {
696 response.head.invalidateStrings();
686 const req = response.request;697 const req = response.request;
687 if (!req.method.responseHasBody()) return .ending;698 if (!req.method.responseHasBody()) return .ending;
688 const head = &response.head;699 const head = &response.head;
...@@ -703,6 +714,7 @@ pub const Response = struct {...@@ -703,6 +714,7 @@ pub const Response = struct {
703 decompressor: *http.Decompressor,714 decompressor: *http.Decompressor,
704 decompression_buffer: []u8,715 decompression_buffer: []u8,
705 ) *Reader {716 ) *Reader {
717 response.head.invalidateStrings();
706 const head = &response.head;718 const head = &response.head;
707 return response.request.reader.bodyReaderDecompressing(719 return response.request.reader.bodyReaderDecompressing(
708 head.transfer_encoding,720 head.transfer_encoding,
lib/std/http/Server.zig+13-2
...@@ -55,8 +55,8 @@ pub fn receiveHead(s: *Server) ReceiveHeadError!Request {...@@ -55,8 +55,8 @@ pub fn receiveHead(s: *Server) ReceiveHeadError!Request {
5555
56pub const Request = struct {56pub const Request = struct {
57 server: *Server,57 server: *Server,
58 /// Pointers in this struct are invalidated with the next call to58 /// Pointers in this struct are invalidated when the request body stream is
59 /// `receiveHead`.59 /// initialized.
60 head: Head,60 head: Head,
61 head_buffer: []const u8,61 head_buffer: []const u8,
62 respond_err: ?RespondError = null,62 respond_err: ?RespondError = null,
...@@ -224,6 +224,14 @@ pub const Request = struct {...@@ -224,6 +224,14 @@ pub const Request = struct {
224 inline fn int64(array: *const [8]u8) u64 {224 inline fn int64(array: *const [8]u8) u64 {
225 return @bitCast(array.*);225 return @bitCast(array.*);
226 }226 }
227
228 /// Help the programmer avoid bugs by calling this when the string
229 /// memory of `Head` becomes invalidated.
230 fn invalidateStrings(h: *Head) void {
231 h.target = undefined;
232 if (h.expect) |*s| s.* = undefined;
233 if (h.content_type) |*s| s.* = undefined;
234 }
227 };235 };
228236
229 pub fn iterateHeaders(r: *const Request) http.HeaderIterator {237 pub fn iterateHeaders(r: *const Request) http.HeaderIterator {
...@@ -578,9 +586,12 @@ pub const Request = struct {...@@ -578,9 +586,12 @@ pub const Request = struct {
578 /// this function.586 /// this function.
579 ///587 ///
580 /// Asserts that this function is only called once.588 /// Asserts that this function is only called once.
589 ///
590 /// Invalidates the string memory inside `Head`.
581 pub fn readerExpectNone(request: *Request, buffer: []u8) *Reader {591 pub fn readerExpectNone(request: *Request, buffer: []u8) *Reader {
582 assert(request.server.reader.state == .received_head);592 assert(request.server.reader.state == .received_head);
583 assert(request.head.expect == null);593 assert(request.head.expect == null);
594 request.head.invalidateStrings();
584 if (!request.head.method.requestHasBody()) return .ending;595 if (!request.head.method.requestHasBody()) return .ending;
585 return request.server.reader.bodyReader(buffer, request.head.transfer_encoding, request.head.content_length);596 return request.server.reader.bodyReader(buffer, request.head.transfer_encoding, request.head.content_length);
586 }597 }
lib/std/http/test.zig+42-33
...@@ -65,23 +65,22 @@ test "trailers" {...@@ -65,23 +65,22 @@ test "trailers" {
65 try req.sendBodiless();65 try req.sendBodiless();
66 var response = try req.receiveHead(&.{});66 var response = try req.receiveHead(&.{});
6767
68 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
69 defer gpa.free(body);
70
71 try expectEqualStrings("Hello, World!\n", body);
72
73 {68 {
74 var it = response.head.iterateHeaders();69 var it = response.head.iterateHeaders();
75 const header = it.next().?;70 const header = it.next().?;
76 try expect(!it.is_trailer);
77 try expectEqualStrings("transfer-encoding", header.name);71 try expectEqualStrings("transfer-encoding", header.name);
78 try expectEqualStrings("chunked", header.value);72 try expectEqualStrings("chunked", header.value);
79 try expectEqual(null, it.next());73 try expectEqual(null, it.next());
80 }74 }
75
76 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
77 defer gpa.free(body);
78
79 try expectEqualStrings("Hello, World!\n", body);
80
81 {81 {
82 var it = response.iterateTrailers();82 var it = response.iterateTrailers();
83 const header = it.next().?;83 const header = it.next().?;
84 try expect(it.is_trailer);
85 try expectEqualStrings("X-Checksum", header.name);84 try expectEqualStrings("X-Checksum", header.name);
86 try expectEqualStrings("aaaa", header.value);85 try expectEqualStrings("aaaa", header.value);
87 try expectEqual(null, it.next());86 try expectEqual(null, it.next());
...@@ -208,12 +207,14 @@ test "echo content server" {...@@ -208,12 +207,14 @@ test "echo content server" {
208 // request.head.target,207 // request.head.target,
209 //});208 //});
210209
210 try expect(mem.startsWith(u8, request.head.target, "/echo-content"));
211 try expectEqualStrings("text/plain", request.head.content_type.?);
212
213 // head strings expire here
211 const body = try (try request.readerExpectContinue(&.{})).allocRemaining(std.testing.allocator, .unlimited);214 const body = try (try request.readerExpectContinue(&.{})).allocRemaining(std.testing.allocator, .unlimited);
212 defer std.testing.allocator.free(body);215 defer std.testing.allocator.free(body);
213216
214 try expect(mem.startsWith(u8, request.head.target, "/echo-content"));
215 try expectEqualStrings("Hello, World!\n", body);217 try expectEqualStrings("Hello, World!\n", body);
216 try expectEqualStrings("text/plain", request.head.content_type.?);
217218
218 var response = try request.respondStreaming(&.{}, .{219 var response = try request.respondStreaming(&.{}, .{
219 .content_length = switch (request.head.transfer_encoding) {220 .content_length = switch (request.head.transfer_encoding) {
...@@ -410,17 +411,19 @@ test "general client/server API coverage" {...@@ -410,17 +411,19 @@ test "general client/server API coverage" {
410411
411 fn handleRequest(request: *http.Server.Request, listen_port: u16) !void {412 fn handleRequest(request: *http.Server.Request, listen_port: u16) !void {
412 const log = std.log.scoped(.server);413 const log = std.log.scoped(.server);
414 const gpa = std.testing.allocator;
413415
414 log.info("{f} {t} {s}", .{ request.head.method, request.head.version, request.head.target });416 log.info("{f} {t} {s}", .{ request.head.method, request.head.version, request.head.target });
417 const target = try gpa.dupe(u8, request.head.target);
418 defer gpa.free(target);
415419
416 const gpa = std.testing.allocator;
417 const reader = (try request.readerExpectContinue(&.{}));420 const reader = (try request.readerExpectContinue(&.{}));
418 const body = try reader.allocRemaining(gpa, .unlimited);421 const body = try reader.allocRemaining(gpa, .unlimited);
419 defer gpa.free(body);422 defer gpa.free(body);
420423
421 if (mem.startsWith(u8, request.head.target, "/get")) {424 if (mem.startsWith(u8, target, "/get")) {
422 var response = try request.respondStreaming(&.{}, .{425 var response = try request.respondStreaming(&.{}, .{
423 .content_length = if (mem.indexOf(u8, request.head.target, "?chunked") == null)426 .content_length = if (mem.indexOf(u8, target, "?chunked") == null)
424 14427 14
425 else428 else
426 null,429 null,
...@@ -435,7 +438,7 @@ test "general client/server API coverage" {...@@ -435,7 +438,7 @@ test "general client/server API coverage" {
435 try w.writeAll("World!\n");438 try w.writeAll("World!\n");
436 try response.end();439 try response.end();
437 // Writing again would cause an assertion failure.440 // Writing again would cause an assertion failure.
438 } else if (mem.startsWith(u8, request.head.target, "/large")) {441 } else if (mem.startsWith(u8, target, "/large")) {
439 var response = try request.respondStreaming(&.{}, .{442 var response = try request.respondStreaming(&.{}, .{
440 .content_length = 14 * 1024 + 14 * 10,443 .content_length = 14 * 1024 + 14 * 10,
441 });444 });
...@@ -458,7 +461,7 @@ test "general client/server API coverage" {...@@ -458,7 +461,7 @@ test "general client/server API coverage" {
458 }461 }
459462
460 try response.end();463 try response.end();
461 } else if (mem.eql(u8, request.head.target, "/redirect/1")) {464 } else if (mem.eql(u8, target, "/redirect/1")) {
462 var response = try request.respondStreaming(&.{}, .{465 var response = try request.respondStreaming(&.{}, .{
463 .respond_options = .{466 .respond_options = .{
464 .status = .found,467 .status = .found,
...@@ -472,14 +475,14 @@ test "general client/server API coverage" {...@@ -472,14 +475,14 @@ test "general client/server API coverage" {
472 try w.writeAll("Hello, ");475 try w.writeAll("Hello, ");
473 try w.writeAll("Redirected!\n");476 try w.writeAll("Redirected!\n");
474 try response.end();477 try response.end();
475 } else if (mem.eql(u8, request.head.target, "/redirect/2")) {478 } else if (mem.eql(u8, target, "/redirect/2")) {
476 try request.respond("Hello, Redirected!\n", .{479 try request.respond("Hello, Redirected!\n", .{
477 .status = .found,480 .status = .found,
478 .extra_headers = &.{481 .extra_headers = &.{
479 .{ .name = "location", .value = "/redirect/1" },482 .{ .name = "location", .value = "/redirect/1" },
480 },483 },
481 });484 });
482 } else if (mem.eql(u8, request.head.target, "/redirect/3")) {485 } else if (mem.eql(u8, target, "/redirect/3")) {
483 const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/redirect/2", .{486 const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/redirect/2", .{
484 listen_port,487 listen_port,
485 });488 });
...@@ -491,23 +494,23 @@ test "general client/server API coverage" {...@@ -491,23 +494,23 @@ test "general client/server API coverage" {
491 .{ .name = "location", .value = location },494 .{ .name = "location", .value = location },
492 },495 },
493 });496 });
494 } else if (mem.eql(u8, request.head.target, "/redirect/4")) {497 } else if (mem.eql(u8, target, "/redirect/4")) {
495 try request.respond("Hello, Redirected!\n", .{498 try request.respond("Hello, Redirected!\n", .{
496 .status = .found,499 .status = .found,
497 .extra_headers = &.{500 .extra_headers = &.{
498 .{ .name = "location", .value = "/redirect/3" },501 .{ .name = "location", .value = "/redirect/3" },
499 },502 },
500 });503 });
501 } else if (mem.eql(u8, request.head.target, "/redirect/5")) {504 } else if (mem.eql(u8, target, "/redirect/5")) {
502 try request.respond("Hello, Redirected!\n", .{505 try request.respond("Hello, Redirected!\n", .{
503 .status = .found,506 .status = .found,
504 .extra_headers = &.{507 .extra_headers = &.{
505 .{ .name = "location", .value = "/%2525" },508 .{ .name = "location", .value = "/%2525" },
506 },509 },
507 });510 });
508 } else if (mem.eql(u8, request.head.target, "/%2525")) {511 } else if (mem.eql(u8, target, "/%2525")) {
509 try request.respond("Encoded redirect successful!\n", .{});512 try request.respond("Encoded redirect successful!\n", .{});
510 } else if (mem.eql(u8, request.head.target, "/redirect/invalid")) {513 } else if (mem.eql(u8, target, "/redirect/invalid")) {
511 const invalid_port = try getUnusedTcpPort();514 const invalid_port = try getUnusedTcpPort();
512 const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}", .{invalid_port});515 const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}", .{invalid_port});
513 defer gpa.free(location);516 defer gpa.free(location);
...@@ -518,7 +521,7 @@ test "general client/server API coverage" {...@@ -518,7 +521,7 @@ test "general client/server API coverage" {
518 .{ .name = "location", .value = location },521 .{ .name = "location", .value = location },
519 },522 },
520 });523 });
521 } else if (mem.eql(u8, request.head.target, "/empty")) {524 } else if (mem.eql(u8, target, "/empty")) {
522 try request.respond("", .{525 try request.respond("", .{
523 .extra_headers = &.{526 .extra_headers = &.{
524 .{ .name = "empty", .value = "" },527 .{ .name = "empty", .value = "" },
...@@ -559,11 +562,12 @@ test "general client/server API coverage" {...@@ -559,11 +562,12 @@ test "general client/server API coverage" {
559 try req.sendBodiless();562 try req.sendBodiless();
560 var response = try req.receiveHead(&redirect_buffer);563 var response = try req.receiveHead(&redirect_buffer);
561564
565 try expectEqualStrings("text/plain", response.head.content_type.?);
566
562 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);567 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
563 defer gpa.free(body);568 defer gpa.free(body);
564569
565 try expectEqualStrings("Hello, World!\n", body);570 try expectEqualStrings("Hello, World!\n", body);
566 try expectEqualStrings("text/plain", response.head.content_type.?);
567 }571 }
568572
569 // connection has been kept alive573 // connection has been kept alive
...@@ -604,12 +608,13 @@ test "general client/server API coverage" {...@@ -604,12 +608,13 @@ test "general client/server API coverage" {
604 try req.sendBodiless();608 try req.sendBodiless();
605 var response = try req.receiveHead(&redirect_buffer);609 var response = try req.receiveHead(&redirect_buffer);
606610
611 try expectEqualStrings("text/plain", response.head.content_type.?);
612 try expectEqual(14, response.head.content_length.?);
613
607 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);614 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
608 defer gpa.free(body);615 defer gpa.free(body);
609616
610 try expectEqualStrings("", body);617 try expectEqualStrings("", body);
611 try expectEqualStrings("text/plain", response.head.content_type.?);
612 try expectEqual(14, response.head.content_length.?);
613 }618 }
614619
615 // connection has been kept alive620 // connection has been kept alive
...@@ -628,11 +633,12 @@ test "general client/server API coverage" {...@@ -628,11 +633,12 @@ test "general client/server API coverage" {
628 try req.sendBodiless();633 try req.sendBodiless();
629 var response = try req.receiveHead(&redirect_buffer);634 var response = try req.receiveHead(&redirect_buffer);
630635
636 try expectEqualStrings("text/plain", response.head.content_type.?);
637
631 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);638 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
632 defer gpa.free(body);639 defer gpa.free(body);
633640
634 try expectEqualStrings("Hello, World!\n", body);641 try expectEqualStrings("Hello, World!\n", body);
635 try expectEqualStrings("text/plain", response.head.content_type.?);
636 }642 }
637643
638 // connection has been kept alive644 // connection has been kept alive
...@@ -651,12 +657,13 @@ test "general client/server API coverage" {...@@ -651,12 +657,13 @@ test "general client/server API coverage" {
651 try req.sendBodiless();657 try req.sendBodiless();
652 var response = try req.receiveHead(&redirect_buffer);658 var response = try req.receiveHead(&redirect_buffer);
653659
660 try expectEqualStrings("text/plain", response.head.content_type.?);
661 try expect(response.head.transfer_encoding == .chunked);
662
654 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);663 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
655 defer gpa.free(body);664 defer gpa.free(body);
656665
657 try expectEqualStrings("", body);666 try expectEqualStrings("", body);
658 try expectEqualStrings("text/plain", response.head.content_type.?);
659 try expect(response.head.transfer_encoding == .chunked);
660 }667 }
661668
662 // connection has been kept alive669 // connection has been kept alive
...@@ -677,11 +684,12 @@ test "general client/server API coverage" {...@@ -677,11 +684,12 @@ test "general client/server API coverage" {
677 try req.sendBodiless();684 try req.sendBodiless();
678 var response = try req.receiveHead(&redirect_buffer);685 var response = try req.receiveHead(&redirect_buffer);
679686
687 try expectEqualStrings("text/plain", response.head.content_type.?);
688
680 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);689 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
681 defer gpa.free(body);690 defer gpa.free(body);
682691
683 try expectEqualStrings("Hello, World!\n", body);692 try expectEqualStrings("Hello, World!\n", body);
684 try expectEqualStrings("text/plain", response.head.content_type.?);
685 }693 }
686694
687 // connection has been closed695 // connection has been closed
...@@ -706,11 +714,6 @@ test "general client/server API coverage" {...@@ -706,11 +714,6 @@ test "general client/server API coverage" {
706714
707 try std.testing.expectEqual(.ok, response.head.status);715 try std.testing.expectEqual(.ok, response.head.status);
708716
709 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
710 defer gpa.free(body);
711
712 try expectEqualStrings("", body);
713
714 var it = response.head.iterateHeaders();717 var it = response.head.iterateHeaders();
715 {718 {
716 const header = it.next().?;719 const header = it.next().?;
...@@ -718,6 +721,12 @@ test "general client/server API coverage" {...@@ -718,6 +721,12 @@ test "general client/server API coverage" {
718 try expectEqualStrings("content-length", header.name);721 try expectEqualStrings("content-length", header.name);
719 try expectEqualStrings("0", header.value);722 try expectEqualStrings("0", header.value);
720 }723 }
724
725 const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited);
726 defer gpa.free(body);
727
728 try expectEqualStrings("", body);
729
721 {730 {
722 const header = it.next().?;731 const header = it.next().?;
723 try expect(!it.is_trailer);732 try expect(!it.is_trailer);