From 65aced94ca6609f23adad053b09b8be38eb32d01 Mon Sep 17 00:00:00 2001 From: indaco Date: Fri, 21 Aug 2026 16:57:30 +0200 Subject: [PATCH 1/2] fix(net): stop a silent origin from parking a command with no timeout and no Ctrl-C A host that accepted the connection and then never sent a response head froze malt indefinitely: the per-request timeout only ever reached the body, so nothing bounded the phase before it and nothing sampled the Ctrl-C flag during it. On a cask install that stall happens while db/malt.lock is held, so one silent origin blocked every other invocation until the process was killed. Every hop must now answer within the same timeout the rest of a read already gets, and Ctrl-C lands on the first press. The budget is per hop rather than per walk because a hop that answers is evidence its peer is alive, so a long redirect chain on a slow link is not failed for being long. A silent peer is terminal inside net, so its own walks do not re-dial it. That stops at net's edge: the ghcr path collapses the tag into a generic download failure that the install loop still retries, which is left for its own change. --- src/cli/install.zig | 1 + src/net/client.zig | 182 +++++++++++++++++++-- src/net/ghcr.zig | 1 + tests/net_redirect_auth_test.zig | 261 +++++++++++++++++++++++++++++++ 4 files changed, 435 insertions(+), 10 deletions(-) diff --git a/src/cli/install.zig b/src/cli/install.zig index a0218285..5500ff35 100644 --- a/src/cli/install.zig +++ b/src/cli/install.zig @@ -1336,6 +1336,7 @@ fn mapHeadResolveError(e: client_mod.HeadResolveError) ?InstallError { error.HttpRedirectLocationInvalid, error.HttpRedirectLocationOversize, error.TooManyHttpRedirects, + error.HeadTimeout, => InstallError.NetworkError, // One is malt running out of memory, the other the user stopping. error.OutOfMemory, error.Canceled => null, diff --git a/src/net/client.zig b/src/net/client.zig index aeab75eb..fd766798 100644 --- a/src/net/client.zig +++ b/src/net/client.zig @@ -38,6 +38,9 @@ pub const GetError = error{ ResponseTooLarge, ReadFailed, WatchdogSpawnFailed, + /// The peer accepted the connection and then said nothing for the whole + /// head budget. Silence that long is a diagnosis, not a blip. + HeadTimeout, Canceled, OutOfMemory, }; @@ -63,6 +66,7 @@ pub const HeadResolveError = RedirectError || error{ RequestFailed, InvalidUrl, InsecureUrlScheme, + HeadTimeout, Canceled, }; @@ -314,6 +318,14 @@ pub const HttpClient = struct { /// Per-request timeout in nanoseconds. Default: 30 seconds. timeout_ns: u64 = default_timeout_ns, + /// How long one hop may take to answer its head. Per hop, not per walk: + /// a hop that answers is proof its peer is alive, the same evidence the + /// body's idle watchdog reads from byte progress. Defaults to the request + /// timeout because "too quiet" is one question, not a separate one for the + /// head. Tests shrink it so a stall is observable without paying its + /// wall-clock, and without shortening the body's clock too. + head_timeout_ns: u64 = default_timeout_ns, + /// Optional cancellation predicate polled on every watchdog tick. /// Lets best-effort callers (e.g. the post-dispatch update probe) /// short-circuit a blackholed read on Ctrl-C without coupling @@ -608,15 +620,20 @@ pub const HttpClient = struct { var req = self.client.request(.HEAD, uri, .{ .extra_headers = &.{}, + // Stdlib returns every HEAD response before its redirect branch, + // so this is already the effective behaviour; pinning it keeps the + // watchdog's connection stable by contract, not by stdlib branch + // order. + .redirect_behavior = .unhandled, }) catch |e| return walkTransportError(e); defer req.deinit(); - req.sendBodiless() catch |e| return walkTransportError(e); - // 32 KiB — GHCR's multi-scope token + signed-URL redirects exceed // the 8 KiB default and tripped `HeaderBufferTooSmall`. var redirect_buf: [32 * 1024]u8 = undefined; - const response = req.receiveHead(&redirect_buf) catch |e| return walkTransportError(e); + var fired = std.atomic.Value(bool).init(false); + const response = self.receiveHeadDeadlined(&req, &redirect_buf, self.head_timeout_ns, &fired) catch |e| + return self.headWalkError(&fired, e); return @intFromEnum(response.head.status); } @@ -715,19 +732,29 @@ pub const HttpClient = struct { // Every exit below returns an error rather than what the walk reached // so far: a partial walk is indistinguishable from a resolved url, and // the caller picks a cask's artifact type from it. + var hops: usize = 0; while (true) : (hops += 1) { const uri = std.Uri.parse(resolved.final_url) catch return error.InvalidUrl; var req = self.client.request(.HEAD, uri, .{ .extra_headers = &.{}, + // This walk follows Location itself; stdlib returns a HEAD + // before its redirect branch anyway, so pinning it keeps the + // watchdog's connection stable by contract, not by stdlib + // branch order. + .redirect_behavior = .unhandled, }) catch |e| return walkTransportError(e); defer req.deinit(); - req.sendBodiless() catch |e| return walkTransportError(e); - var redirect_buf: [32 * 1024]u8 = undefined; - const response = req.receiveHead(&redirect_buf) catch |e| return walkTransportError(e); + var fired = std.atomic.Value(bool).init(false); + const response = self.receiveHeadDeadlined(&req, &redirect_buf, self.head_timeout_ns, &fired) catch |e| switch (self.headWalkError(&fired, e)) { + // `HeadResolveError` stays closed: a watchdog that could not + // start is one more way the request failed. + error.WatchdogSpawnFailed => return error.RequestFailed, + else => |mapped| return mapped, + }; if (resolved.content_disposition == null) { if (response.head.content_disposition) |cd| { @@ -835,6 +862,60 @@ pub const HttpClient = struct { return self.doGetWithRetry(url, extra_headers, max_metadata_bytes, null); } + /// Send the request and read its head under the same watchdog the body + /// gets: without one a connected-but-silent origin parks the walk forever + /// and nothing polls `cancel`. Idle and total share a clock because a head + /// read has no byte progress to measure. A stall inside `client.request` + /// stays uncovered - the watchdog needs a connection that call has not + /// returned yet. + fn receiveHeadDeadlined( + self: *HttpClient, + req: *std.http.Client.Request, + buf: []u8, + budget_ns: u64, + fired: *std.atomic.Value(bool), + ) !std.http.Client.Response { + var no_progress = std.atomic.Value(u64).init(0); + var wake = try Wake.init(); + const watchdog = std.Thread.spawn(.{}, watchdogFn, .{ + self.io, + wake.read_fd, + &no_progress, + budget_ns, + budget_ns, + req, + self.cancel, + fired, + }) catch { + wake.deinit(); + return error.WatchdogSpawnFailed; + }; + defer { + wake.signal(); + watchdog.join(); + wake.deinit(); + } + + try req.sendBodiless(); + return try req.receiveHead(buf); + } + + /// Why the head read failed decides whether a second walk is worth its + /// backoff. Only a read the watchdog itself cut short is the peer's + /// answer; anything else is an ordinary fault the retry policy handles. + fn headWalkError( + self: *HttpClient, + fired: *const std.atomic.Value(bool), + e: anyerror, + ) error{ Canceled, OutOfMemory, WatchdogSpawnFailed, HeadTimeout, RequestFailed } { + if (e == error.WatchdogSpawnFailed) return error.WatchdogSpawnFailed; + if (fired.load(.acquire)) { + if (self.cancel) |cancelled| if (cancelled()) return error.Canceled; + return error.HeadTimeout; + } + return walkTransportError(e); + } + /// Stream a response body into a caller-provided `sink` with the same /// decompress + idle/total watchdog + size cap the buffer path uses. The /// watchdog's spawn/join wraps the streaming call so a stalled sink is @@ -887,6 +968,7 @@ pub const HttpClient = struct { total_timeout_ns, req, self.cancel, + null, }) catch { wake.deinit(); return error.WatchdogSpawnFailed; @@ -934,6 +1016,9 @@ pub const HttpClient = struct { InsecureUrlScheme, ResponseTooLarge, OfflineRequired, + /// A peer that went silent for the whole budget has answered; re-dialling + /// it three more times only lengthens the wait under `db/malt.lock`. + HeadTimeout, /// The user's answer, not a fault to sleep off. Canceled, }; @@ -1146,9 +1231,10 @@ pub const HttpClient = struct { }) catch |e| return walkTransportError(e); errdefer req.deinit(); - req.sendBodiless() catch |e| return walkTransportError(e); var redirect_buf: [32 * 1024]u8 = undefined; - var response = req.receiveHead(&redirect_buf) catch |e| return walkTransportError(e); + var fired = std.atomic.Value(bool).init(false); + var response = self.receiveHeadDeadlined(&req, &redirect_buf, self.head_timeout_ns, &fired) catch |e| + return self.headWalkError(&fired, e); const status: u16 = @intFromEnum(response.head.status); const hop = self.nextRedirectHop(uri, status, response.head.location, hops) catch |e| return walkRedirectError(e); @@ -1308,9 +1394,10 @@ pub const HttpClient = struct { }) catch |e| return walkTransportError(e); errdefer req.deinit(); - req.sendBodiless() catch |e| return walkTransportError(e); var redirect_buf: [32 * 1024]u8 = undefined; - var response = req.receiveHead(&redirect_buf) catch |e| return walkTransportError(e); + var fired = std.atomic.Value(bool).init(false); + var response = self.receiveHeadDeadlined(&req, &redirect_buf, self.head_timeout_ns, &fired) catch |e| + return self.headWalkError(&fired, e); const status: u16 = @intFromEnum(response.head.status); const hop = self.nextRedirectHop(uri, status, response.head.location, hops) catch |e| return walkRedirectError(e); @@ -1359,6 +1446,10 @@ pub const HttpClient = struct { /// `shutdown(.both)` because setting closing alone does not wake a /// parked read - stalled TLS reads hung the previous single-deadline /// implementation. + /// + /// `fired` lets a caller tell its own shutdown from an unrelated transport + /// fault, which decides whether the failure is worth retrying. The body + /// path retries on neither, so it passes null. fn watchdogFn( io: std.Io, wake_fd: std.posix.fd_t, @@ -1367,8 +1458,11 @@ pub const HttpClient = struct { total_timeout_ns: u64, req: *std.http.Client.Request, cancel: ?*const fn () bool, + fired: ?*std.atomic.Value(bool), ) void { if (!watchdogLoop(io, wake_fd, bytes_progress, idle_timeout_ns, total_timeout_ns, cancel)) return; + // Ordered before the shutdown so the woken reader always sees it set. + if (fired) |f| f.store(true, .release); if (req.connection) |conn| { conn.closing = true; const fd = conn.stream_reader.stream.socket.handle; @@ -1682,6 +1776,9 @@ test "requireSecureOrigin: a digest-pinned payload may come over cleartext http" /// An inferred set drags std.http's own tags in, which is exactly what Rule U1 /// forbids a leaf from leaking. fn assertErrorSetFitsIn(comptime f: anytype, comptime Allowed: type, comptime name: []const u8) void { + // Every entry point compares each tag against the whole allowed set, so the + // pair count grows with both and outran the default quota. + @setEvalBranchQuota(10_000); const ret = @typeInfo(@TypeOf(f)).@"fn".return_type.?; const actual = @typeInfo(@typeInfo(ret).error_union.error_set).error_set.?; for (actual) |tag| { @@ -1737,6 +1834,60 @@ test "a collapsed redirect tag is still retriable exactly as before" { try std.testing.expect(!HttpClient.isRetriableWalkError(error.OutOfMemory)); // ...while a transport failure still earns a second walk. try std.testing.expect(HttpClient.isRetriableWalkError(error.RequestFailed)); + // A peer silent for the whole head budget has answered. Sleeping on it + // would re-dial a known-silent host while `db/malt.lock` stays held. + try std.testing.expect(!HttpClient.isRetriableWalkError(error.HeadTimeout)); +} + +test "headWalkError: only a watchdog-cut read is the peer's answer" { + // The decision table behind both guarantees: a stall must not be slept off + // as a blip, and a Ctrl-C must not be reported as one either. + const a = std.testing.allocator; + var http = HttpClient.init(std.Options.debug_io, .empty, a); + defer http.deinit(); + + var quiet = std.atomic.Value(bool).init(false); + var cut = std.atomic.Value(bool).init(true); + + // Watchdog never fired: an ordinary fault the retry policy still owns. + try std.testing.expectEqual(error.RequestFailed, http.headWalkError(&quiet, error.ReadFailed)); + try std.testing.expectEqual(error.OutOfMemory, http.headWalkError(&quiet, error.OutOfMemory)); + + // Watchdog cut the read and nothing asked to stop: the peer went silent. + try std.testing.expectEqual(error.HeadTimeout, http.headWalkError(&cut, error.ReadFailed)); + + // A watchdog that never started is not a verdict on the peer. + try std.testing.expectEqual( + error.WatchdogSpawnFailed, + http.headWalkError(&cut, error.WatchdogSpawnFailed), + ); +} + +test "headWalkError: a set cancel flag only speaks for a read the watchdog cut" { + const Probe = struct { + var answer: bool = false; + fn cancelled() bool { + return answer; + } + }; + + const a = std.testing.allocator; + var http = HttpClient.init(std.Options.debug_io, .empty, a); + defer http.deinit(); + http.cancel = &Probe.cancelled; + + var quiet = std.atomic.Value(bool).init(false); + var cut = std.atomic.Value(bool).init(true); + + // Present but unset: a stall is still a stall. + Probe.answer = false; + try std.testing.expectEqual(error.HeadTimeout, http.headWalkError(&cut, error.ReadFailed)); + + Probe.answer = true; + try std.testing.expectEqual(error.Canceled, http.headWalkError(&cut, error.ReadFailed)); + // Set, but this read failed on its own - consulting the flag here would + // report an unrelated fault as the user's answer. + try std.testing.expectEqual(error.RequestFailed, http.headWalkError(&quiet, error.ReadFailed)); } test "every buffered GET entry point exposes a closed error set" { @@ -2039,6 +2190,17 @@ test "shouldFireIdleWatchdog: cancellation fires regardless of elapsed time" { try std.testing.expect(shouldFireIdleWatchdog(1, 1, 999_999, 999_999, true)); } +test "the head phase is bounded by the same threshold as every other read" { + // Its own knob so a test can shorten the head phase alone, but no number + // of its own to justify: a third figure for "too quiet" is a third thing + // that can drift. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + var http = HttpClient.init(threaded.io(), std.process.Environ.empty, std.testing.allocator); + defer http.deinit(); + try std.testing.expectEqual(HttpClient.default_timeout_ns, http.head_timeout_ns); +} + // Patches an inner Io's vtable so `sleep` reports cancellation on the // configured call index; non-canceled sleeps return immediately so the // retry-table delays don't pad the test runtime. `cancel_at = 1` cancels diff --git a/src/net/ghcr.zig b/src/net/ghcr.zig index 4aec28dd..1ee94d12 100644 --- a/src/net/ghcr.zig +++ b/src/net/ghcr.zig @@ -286,6 +286,7 @@ pub const GhcrClient = struct { error.ResponseTooLarge, error.ReadFailed, error.WatchdogSpawnFailed, + error.HeadTimeout, error.Canceled, => return GhcrError.DownloadFailed, }; diff --git a/tests/net_redirect_auth_test.zig b/tests/net_redirect_auth_test.zig index 1f8a837e..1ba1add5 100644 --- a/tests/net_redirect_auth_test.zig +++ b/tests/net_redirect_auth_test.zig @@ -30,6 +30,13 @@ const Hop = struct { // Drop the first request without answering it, so an otherwise healthy hop // hands the client one transport failure. fail_first: bool = false, + // Read the request and never answer it, holding the connection open: the + // connected-but-silent origin the head-phase deadline exists for. + stall: bool = false, + // Answer, but only after burning this much of the hop's budget. Lets a + // test spend more than one budget's worth across a chain while leaving + // every single hop comfortably inside its own. + answer_delay_ns: u64 = 0, // Requests actually received, so a test can assert how many walks happened. requests: usize = 0, }; @@ -61,6 +68,15 @@ fn serveCount(hop: *Hop, count: usize) void { hop.fail_first = false; break; // close mid-request: the client sees a dead connection } + if (hop.answer_delay_ns > 0) { + std.Io.sleep(hop.io, std.Io.Duration.fromNanoseconds(@intCast(hop.answer_delay_ns)), .awake) catch {}; + } + if (hop.stall) { + // Park until the client gives up and drops the socket. Nothing + // else can end this wait, which is the point of the fixture. + _ = reader.interface.peekByte() catch {}; + return; + } answer(hop, &req); } // A connection carrying no request is `knock`: the client is done, so @@ -875,3 +891,248 @@ test "a hop that fails once is classified rather than reported as a network fail try std.testing.expectEqualStrings(cd, resolved.content_disposition.?); try std.testing.expectEqual(@as(usize, 2), hop1.requests); } + +// Short enough that a green run costs nothing, long enough that the watchdog's +// 100 ms tick floor gets a couple of ticks before it fires. +const stall_budget_ns: u64 = 300 * std.time.ns_per_ms; + +test "a classification walk gives up on an origin that never answers" { + // The stall is the severity driver: `mt install ` classifies under + // `db/malt.lock`, so a parked head read blocks every other invocation too. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/artifact", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + http.retry_backoff_ms = &.{}; + + const result = http.headResolved(url); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.HeadTimeout, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "a download gives up on an origin that never answers" { + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/blob", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + http.retry_backoff_ms = &.{}; + + const result = http.getWithHeaders(url, &.{}, null, .transport_only); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.HeadTimeout, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "Ctrl-C during a stalled head read is the answer, not a blip to retry" { + // Without the cancel check the fix makes Ctrl-C slower: the watchdog's + // socket shutdown looks like an ordinary transport fault, so the walk + // would sleep off the whole backoff and re-dial while the flag stays set. + const Probe = struct { + fn cancelled() bool { + return true; + } + }; + + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/artifact", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + http.cancel = &Probe.cancelled; + // A budget the retry loop could spend if it treated the cancel as a blip. + http.retry_backoff_ms = &.{ 0, 0, 0 }; + + const result = http.headResolved(url); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.Canceled, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "a blob download gives up on an origin that never answers" { + // `followGetToWriter` wires the deadline separately from the buffered + // walk, so its own fixture is what proves that wiring exists. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/bottle", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + http.retry_backoff_ms = &.{}; + + var sink: std.Io.Writer.Allocating = .init(std.testing.allocator); + defer sink.deinit(); + const result = http.getToWriter(url, &.{}, &sink.writer, null); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.HeadTimeout, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "a bare HEAD gives up on an origin that never answers" { + // `doctor`'s reachability probe is the caller here: a silent host used to + // park the whole check with nothing on screen. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/probe", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + + const result = http.head(url); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.HeadTimeout, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "a silent peer is not re-dialled three more times" { + // Silence for the whole budget is the peer's answer. Retrying it spends + // the backoff for nothing while `db/malt.lock` stays held - the cost that + // made the original hang severe in the first place. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + + var hop1 = Hop{ .io = io, .listener = &l1, .stall = true }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/artifact", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = stall_budget_ns; + // A full budget the walk would spend if it read the silence as a blip. + http.retry_backoff_ms = &.{ 0, 0, 0 }; + + const result = http.headResolved(url); + http.deinit(); + t1.join(); + + try std.testing.expectError(error.HeadTimeout, result); + try std.testing.expectEqual(@as(usize, 1), hop1.requests); +} + +test "a hop that answers refreshes the budget for the next one" { + // Per hop, not per walk: a chain is not at fault for being long, and each + // answering peer is fresh evidence of liveness. Both hops answer well + // inside their own budget while together exceeding one, so a single clock + // shared across the walk would cut the second hop off. + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + const budget_ns: u64 = 1000 * std.time.ns_per_ms; + const per_hop_ns: u64 = 600 * std.time.ns_per_ms; + + var l2 = try bindIp4(io); + defer l2.deinit(io); + const p2 = l2.socket.address.getPort(); + var hop2 = Hop{ .io = io, .listener = &l2, .answer_delay_ns = per_hop_ns }; + const t2 = try std.Thread.spawn(.{}, serveOne, .{&hop2}); + + var loc_buf: [64]u8 = undefined; + const loc = try std.fmt.bufPrint(&loc_buf, "http://127.0.0.1:{d}/artifact", .{p2}); + + var l1 = try bindIp4(io); + defer l1.deinit(io); + const p1 = l1.socket.address.getPort(); + var hop1 = Hop{ + .io = io, + .listener = &l1, + .redirect_to = loc, + .status = .found, + .answer_delay_ns = per_hop_ns, + }; + const t1 = try std.Thread.spawn(.{}, serveOne, .{&hop1}); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/start", .{p1}); + + var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator); + http.head_timeout_ns = budget_ns; + http.retry_backoff_ms = &.{}; + + const result = http.headResolved(url); + http.deinit(); + t1.join(); + t2.join(); + + var resolved = try result; + defer resolved.deinit(); + try std.testing.expectEqualStrings(loc, resolved.final_url); +} From a1d484981d335fc6f93700b50d79712c9ce27afb Mon Sep 17 00:00:00 2001 From: indaco Date: Fri, 21 Aug 2026 16:57:34 +0200 Subject: [PATCH 2/2] test(net): pin that a silent origin cannot park the request phase Judged through the integration test binary rather than the CLI: the assertion is wall-clock-bound and MALT_API_DOMAIN is https-only, so the real binary cannot be pointed at a cleartext loopback stall server. --- ...ne-response-head-reads-have-no-deadline.sh | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100755 scripts/regressions/head-read-deadline-response-head-reads-have-no-deadline.sh diff --git a/scripts/regressions/head-read-deadline-response-head-reads-have-no-deadline.sh b/scripts/regressions/head-read-deadline-response-head-reads-have-no-deadline.sh new file mode 100755 index 00000000..791b04aa --- /dev/null +++ b/scripts/regressions/head-read-deadline-response-head-reads-have-no-deadline.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Regression: the request/head phase of an HTTP walk must run under a deadline. +# +# The bug: the per-request timeout was only ever threaded into the body read, +# and the watchdog that enforces it - deadline plus the Ctrl-C predicate - was +# spawned inside `streamResponseBody`. Everything before the body ran with +# whatever the OS gave it, so an origin that completed TCP and TLS and then +# never sent a response head parked the walk forever, and nothing sampled the +# cancel flag during that window. On a cask install the stall happens after +# `db/malt.lock` is taken, blocking every other invocation. +# +# The fix runs the send+head pair under that same watchdog, one budget per hop, +# and reports a read the watchdog cut short as the peer's answer rather than a +# blip to sleep off. +# +# The assertion is behavioural and wall-clock-bound, and `MALT_API_DOMAIN` is +# https-only so the real binary cannot be pointed at a cleartext loopback stall +# server. It is judged through the integration test binary instead: loopback +# only, no network, well under 30s. + +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +cd "$ROOT" + +SRC="src/net/client.zig" +TESTS="tests/net_redirect_auth_test.zig" +BIN="$ROOT/zig-out/test-bin/net_redirect_auth_test" + +fail() { + echo "FAIL: $1" >&2 + exit 1 +} + +# A dropped guard or a dropped fixture would let the binary go green vacuously. +grep -Fqs -- "head_timeout_ns" "$SRC" || fail "the head-phase deadline is gone" +grep -Fqs -- "receiveHeadDeadlined" "$SRC" || fail "the deadlined head helper is gone" +grep -Fqs -- "stall: bool" "$TESTS" || fail "the stalling-hop fixture is gone" + +# Every head read must go through the helper; a raw one is the pre-fix shape. +if [[ "$(grep -Fc -- "req.receiveHead(&redirect_buf)" "$SRC")" -ne 0 ]]; then + fail "a head read still runs with no deadline" +fi + +# Always rebuild: a prebuilt binary could predate the fix, and zig 0.16 has no +# --test-filter to narrow this down. +zig build test-bin >/dev/null 2>&1 || fail "could not build the test binaries" + +OUT=$(mktemp -t head-deadline) +trap 'rm -f "$OUT"' EXIT + +start=$(date +%s) +timeout 25 "$BIN" >"$OUT" 2>&1 || fail "a silent origin was not cut off: $(tail -3 "$OUT")" +elapsed=$(($(date +%s) - start)) + +((elapsed < 20)) || fail "the head phase took ${elapsed}s - the deadline did not fire" + +echo "PASS: a silent origin cannot park the request phase"