55484465

refactor(quic): one owner for the dial-address grammar

a73x 2026-08-13 20:05

Commit message

refactor(quic): one owner for the dial-address grammar

`mux quic://HOST:PORT` and `muxa --quic HOST:PORT` parsed the same
grammar through two copies of parseQuicAddr/resolveHost — brackets,
default port, the unbracketed-IPv6 refusal, all of it duplicated. A human
and an agent pointing at the same daemon must be able to type the same
thing, and two copies of a grammar drift into two dialects of one flag.
Both now call quic.parseAddr, which lives beside the default_port they
already share.

The sequencing was deliberate: the fold waited until test/agent.sh had
pinned muxa's spellings end to end, so the dedup is checked against a
green e2e rather than against a reading of the two functions.

muxa's allocator-taking signature is the one adopted — client.zig's copy
hardcoded std.heap.page_allocator inside resolveHost, so the caller could
not say where the resolver's list was allocated. Its two call sites both
had an allocator in scope already.

Suite green (255+ unit tests, e2e 23 scenarios / 35 convergence points,
agent 9/9), with the parse's own pins carried over intact — the merged
test keeps every spelling either copy asserted, and still spells 4433 as
a literal rather than reading the constant under test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

build.zig

src/client.zig

Old		New
@@ -11,6 +11,7 @@ const Replica = @import("replica").Replica;
11	const proto = @import("protocol");	11	const proto = @import("protocol");
12	const TmpDir = @import("testtmp").TmpDir;	12	const TmpDir = @import("testtmp").TmpDir;
13	const quic_client = @import("quic_client");	13	const quic_client = @import("quic_client");
		14	const quic = @import("quic");
14	const predict = @import("predict");	15	const predict = @import("predict");
15	const handoff = @import("handoff");	16	const handoff = @import("handoff");
16	// Named `paint_mod` because paintOverlay holds a local ArrayList called	17	// Named `paint_mod` because paintOverlay holds a local ArrayList called
@@ -257,7 +258,7 @@ pub const Transport = struct {
257	.hand => \|h\| return openHandoff(alloc, h, carry, abort_fd),	258	.hand => \|h\| return openHandoff(alloc, h, carry, abort_fd),
258	.quic => \|q\| {	259	.quic => \|q\| {
259	const key = try quic_client.Key.load(q.key_path);	260	const key = try quic_client.Key.load(q.key_path);
260	const addr = try parseQuicAddr(q.host_port);	261	const addr = try quic.parseAddr(alloc, q.host_port);
261	return quicTransport(alloc, addr, key, q.idle_ms, q.deadline_ms, carry, abort_fd);	262	return quicTransport(alloc, addr, key, q.idle_ms, q.deadline_ms, carry, abort_fd);
262	},	263	},
263	.via => \|cmd\| return pipeTransport(try spawnPipe(alloc, cmd)),	264	.via => \|cmd\| return pipeTransport(try spawnPipe(alloc, cmd)),
@@ -372,7 +373,7 @@ pub const Transport = struct {
372	carry: ?*std.ArrayList(u8),	373	carry: ?*std.ArrayList(u8),
373	abort_fd: std.posix.fd_t,	374	abort_fd: std.posix.fd_t,
374	) !Transport {	375	) !Transport {
375	const addr = try resolveHost(handoff.dialHost(h.host), ep.port);	376	const addr = try quic.resolveHost(alloc, handoff.dialHost(h.host), ep.port);
376	const key = quic_client.Key{ .bytes = ep.key };	377	const key = quic_client.Key{ .bytes = ep.key };
377	return quicTransport(alloc, addr, key, h.idle_ms, h.deadline_ms, carry, abort_fd);	378	return quicTransport(alloc, addr, key, h.idle_ms, h.deadline_ms, carry, abort_fd);
378	}	379	}
@@ -677,43 +678,6 @@ fn readAnnounceAbortable(
677	}	678	}
678	}	679	}
679		680
680	/// `HOST:PORT` for a `quic://` target. Literal addresses only on the muxd
681	/// side because a bind address that resolves to several is a question; here
682	/// a NAME is exactly what a user types, so this one does resolve.
683	fn parseQuicAddr(host_port: []const u8) !std.net.Address {
684	// `[::1]` — bracketed, portless: the brackets say where the address
685	// stops, so the port can default.
686	if (host_port.len >= 2 and host_port[0] == '[' and host_port[host_port.len - 1] == ']')
687	return resolveHost(host_port[1 .. host_port.len - 1], quic_client.default_port);
688	const colon = std.mem.lastIndexOfScalar(u8, host_port, ':') orelse
689	return resolveHost(host_port, quic_client.default_port);
690	var host = host_port[0..colon];
691	const port_s = host_port[colon + 1 ..];
692	// `[::1]:4433` — brackets are how an IPv6 literal says where it stops.
693	if (host.len >= 2 and host[0] == '[' and host[host.len - 1] == ']') {
694	host = host[1 .. host.len - 1];
695	} else if (std.mem.indexOfScalar(u8, host, ':') != null) {
696	// Unbracketed and full of colons: an IPv6 literal missing its
697	// brackets, which would otherwise have its last group taken as a
698	// port. Refused rather than guessed at.
699	return error.MalformedAddress;
700	}
701	const port = std.fmt.parseInt(u16, port_s, 10) catch return error.MalformedAddress;
702	return resolveHost(host, port);
703	}
704
705	/// A host that is already known to be unambiguous, plus the port it goes
706	/// with: literal if it parses as one, resolved if it does not.
707	fn resolveHost(host: []const u8, port: u16) !std.net.Address {
708	if (host.len == 0) return error.MalformedAddress;
709	if (std.net.Address.parseIp(host, port)) \|addr\| return addr else \|_\| {}
710	// Not a literal: resolve it. A remote host is normally a name.
711	const list = try std.net.getAddressList(std.heap.page_allocator, host, port);
712	defer list.deinit();
713	if (list.addrs.len == 0) return error.UnknownHostName;
714	return list.addrs[0];
715	}
716
717	/// Whether a failed handoff failed at the announce — meaning ssh itself	681	/// Whether a failed handoff failed at the announce — meaning ssh itself
718	/// worked and what came back was not the line we needed — rather than	682	/// worked and what came back was not the line we needed — rather than
719	/// before it.	683	/// before it.
@@ -2360,21 +2324,6 @@ test "drainStdinForQuit: closed stdin still paces the retry instead of spinning"
2360	try std.testing.expect(elapsed_ms >= 150);	2324	try std.testing.expect(elapsed_ms >= 150);
2361	}	2325	}
2362		2326
2363	test "parseQuicAddr: no port means 4433, explicit port wins" {
2364	// 4433 spelled out, not `quic_client.default_port`: asserting against
2365	// the constant the code under test reads would hold for any value, so
2366	// it could never catch the number changing — and this is precisely the
2367	// number the daemon must agree with.
2368	const d = try parseQuicAddr("127.0.0.1");
2369	try std.testing.expectEqual(@as(u16, 4433), d.getPort());
2370	const e = try parseQuicAddr("127.0.0.1:9");
2371	try std.testing.expectEqual(@as(u16, 9), e.getPort());
2372	const b = try parseQuicAddr("[::1]");
2373	try std.testing.expectEqual(@as(u16, 4433), b.getPort());
2374	// Unbracketed IPv6 stays ambiguous and refused, with or without ports.
2375	try std.testing.expectError(error.MalformedAddress, parseQuicAddr("fe80::1:4433"));
2376	}
2377
2378	test "lostMsg: only a --via transport that never connected gets the new wording" {	2327	test "lostMsg: only a --via transport that never connected gets the new wording" {
2379	// The case the message exists for: a command that failed to start. It	2328	// The case the message exists for: a command that failed to start. It
2380	// names what happened and guesses no cause — ssh's own stderr passes	2329	// names what happened and guesses no cause — ssh's own stderr passes

src/muxa.zig

Old		New
@@ -6,6 +6,7 @@ const std = @import("std");
6	const proto = @import("protocol");	6	const proto = @import("protocol");
7	const sockpath = @import("sockpath");	7	const sockpath = @import("sockpath");
8	const quic_client = @import("quic_client");	8	const quic_client = @import("quic_client");
		9	const quic = @import("quic");
9	const xdg = @import("xdg");	10	const xdg = @import("xdg");
10		11
11	const usage =	12	const usage =
@@ -997,7 +998,7 @@ fn openQuicConn(
997	var buf: [quic_client.key_refusal_len]u8 = undefined;	998	var buf: [quic_client.key_refusal_len]u8 = undefined;
998	return .{ .exit = fail("quic: unusable key", quic_client.keyRefusalBody(&buf, e, key_path)) };	999	return .{ .exit = fail("quic: unusable key", quic_client.keyRefusalBody(&buf, e, key_path)) };
999	};	1000	};
1000	const addr = parseQuicAddr(alloc, host_port) catch \|e\| {	1001	const addr = quic.parseAddr(alloc, host_port) catch \|e\| {
1001	var buf: [512]u8 = undefined;	1002	var buf: [512]u8 = undefined;
1002	const detail = std.fmt.bufPrint(&buf, "{s}: {s}", .{ host_port, @errorName(e) }) catch	1003	const detail = std.fmt.bufPrint(&buf, "{s}: {s}", .{ host_port, @errorName(e) }) catch
1003	@errorName(e);	1004	@errorName(e);
@@ -1012,79 +1013,6 @@ fn openQuicConn(
1012	return .{ .conn = conn };	1013	return .{ .conn = conn };
1013	}	1014	}
1014		1015
1015	/// `HOST[:PORT]`, with an omitted port meaning `quic_client.default_port`.
1016	/// A name is resolved rather than refused: unlike muxd's `--quic`, which
1017	/// names an address to BIND, this one names a box to reach, and a box is
1018	/// normally spelled with a name.
1019	///
1020	/// The grammar is the CLI client's — `mux quic://HOST:PORT` accepts these
1021	/// same spellings, brackets and all, and the two must not diverge: an
1022	/// agent and a human pointing at the same daemon type the same thing. It
1023	/// is a second copy of client.zig's `parseQuicAddr` and knowingly so:
1024	/// folding both into the shared `quic` module is the right home for it and
1025	/// is a change to the CLI client, which this one is not.
1026	fn parseQuicAddr(alloc: std.mem.Allocator, host_port: []const u8) !std.net.Address {
1027	// `[::1]` — bracketed and portless: the brackets say where the address
1028	// stops, so the port can default.
1029	if (host_port.len >= 2 and host_port[0] == '[' and host_port[host_port.len - 1] == ']')
1030	return resolveHost(alloc, host_port[1 .. host_port.len - 1], quic_client.default_port);
1031	const colon = std.mem.lastIndexOfScalar(u8, host_port, ':') orelse
1032	return resolveHost(alloc, host_port, quic_client.default_port);
1033	var host = host_port[0..colon];
1034	const port_s = host_port[colon + 1 ..];
1035	if (host.len >= 2 and host[0] == '[' and host[host.len - 1] == ']') {
1036	host = host[1 .. host.len - 1];
1037	} else if (std.mem.indexOfScalar(u8, host, ':') != null) {
1038	// An unbracketed IPv6 literal would have its last group taken as a
1039	// port. Refused rather than guessed at.
1040	return error.MalformedAddress;
1041	}
1042	const port = std.fmt.parseInt(u16, port_s, 10) catch return error.MalformedAddress;
1043	return resolveHost(alloc, host, port);
1044	}
1045
1046	fn resolveHost(alloc: std.mem.Allocator, host: []const u8, port: u16) !std.net.Address {
1047	if (host.len == 0) return error.MalformedAddress;
1048	if (std.net.Address.parseIp(host, port)) \|addr\| return addr else \|_\| {}
1049	const list = try std.net.getAddressList(alloc, host, port);
1050	defer list.deinit();
1051	if (list.addrs.len == 0) return error.UnknownHostName;
1052	return list.addrs[0];
1053	}
1054
1055	test "parseQuicAddr: literals, brackets, and the spellings that are refused" {
1056	const alloc = std.testing.allocator;
1057	// Literals only here: a name would send this test to a resolver, and
1058	// what it answered would depend on the machine running it.
1059	try std.testing.expectEqual(
1060	@as(u16, 4433),
1061	(try parseQuicAddr(alloc, "127.0.0.1:4433")).getPort(),
1062	);
1063
1064	// An omitted port means mux's own. Spelled out rather than written
1065	// `quic_client.default_port`, because comparing the parse's answer
1066	// against the constant the parse reads would hold for any value and
1067	// say nothing about the port — and this is the number the daemon at
1068	// the other end has to agree on.
1069	try std.testing.expectEqual(@as(u16, 4433), (try parseQuicAddr(alloc, "10.0.0.2")).getPort());
1070
1071	const six = try parseQuicAddr(alloc, "[::1]:9999");
1072	try std.testing.expectEqual(@as(u16, 9999), six.getPort());
1073	try std.testing.expect(six.any.family == std.posix.AF.INET6);
1074	// Bracketed and portless: the brackets say where the address stops, so
1075	// the port can default.
1076	try std.testing.expectEqual(@as(u16, 4433), (try parseQuicAddr(alloc, "[::1]")).getPort());
1077
1078	// An unbracketed IPv6 literal would have its last group read as a
1079	// port. Refused rather than guessed at — the same refusal muxd's
1080	// splitHostPort makes about its bind address.
1081	try std.testing.expectError(error.MalformedAddress, parseQuicAddr(alloc, "fe80::1:4433"));
1082	try std.testing.expectError(error.MalformedAddress, parseQuicAddr(alloc, "127.0.0.1:"));
1083	try std.testing.expectError(error.MalformedAddress, parseQuicAddr(alloc, "127.0.0.1:99999"));
1084	try std.testing.expectError(error.MalformedAddress, parseQuicAddr(alloc, ""));
1085	try std.testing.expectError(error.MalformedAddress, parseQuicAddr(alloc, ":4433"));
1086	}
1087
1088	fn verbStatus(alloc: std.mem.Allocator, conn: *Conn, deadline: i64) !u8 {	1016	fn verbStatus(alloc: std.mem.Allocator, conn: *Conn, deadline: i64) !u8 {
1089	conn.sendFrame(.status_req, "", deadline) catch \|e\| return fail("status: send failed", @errorName(e));	1017	conn.sendFrame(.status_req, "", deadline) catch \|e\| return fail("status: send failed", @errorName(e));
1090	const frame = conn.awaitFrame(alloc, .status_reply, deadline) catch \|e\| switch (e) {	1018	const frame = conn.awaitFrame(alloc, .status_reply, deadline) catch \|e\| switch (e) {

src/quic.zig

+pub const alpn = "\x03mux";
+// ---------------------------------------------------------------------------
+// The dial address grammar
+// ---------------------------------------------------------------------------
+/// `HOST[:PORT]`, as a client types it: `mux quic://HOST:PORT` and
+/// `muxa --quic HOST:PORT` accept exactly these spellings, brackets and
+/// all. One owner for the same reason `default_port` has one — an agent
+/// and a human pointing at the same daemon must be able to type the same
+/// thing, and two copies of a grammar drift into two dialects of one flag.
+///
+/// A name is resolved rather than refused: unlike muxd's `--quic`, which
+/// names an address to BIND, this one names a box to reach, and a box is
+/// normally spelled with a name.
+pub fn parseAddr(alloc: std.mem.Allocator, host_port: []const u8) !std.net.Address {
+    // `[::1]` — bracketed and portless: the brackets say where the address
+    // stops, so the port can default.
+    if (host_port.len >= 2 and host_port[0] == '[' and host_port[host_port.len - 1] == ']')
+        return resolveHost(alloc, host_port[1 .. host_port.len - 1], default_port);
+    const colon = std.mem.lastIndexOfScalar(u8, host_port, ':') orelse
+        return resolveHost(alloc, host_port, default_port);
+    var host = host_port[0..colon];
+    const port_s = host_port[colon + 1 ..];
+    // `[::1]:4433` — brackets are how an IPv6 literal says where it stops.
+    if (host.len >= 2 and host[0] == '[' and host[host.len - 1] == ']') {
+        host = host[1 .. host.len - 1];
+    } else if (std.mem.indexOfScalar(u8, host, ':') != null) {
+        // Unbracketed and full of colons: an IPv6 literal missing its
+        // brackets, which would otherwise have its last group taken as a
+        // port. Refused rather than guessed at.
+        return error.MalformedAddress;
+    }
+    const port = std.fmt.parseInt(u16, port_s, 10) catch return error.MalformedAddress;
+    return resolveHost(alloc, host, port);
+}
+/// A host that is already known to be unambiguous, plus the port it goes
+/// with: literal if it parses as one, resolved if it does not.
+pub fn resolveHost(alloc: std.mem.Allocator, host: []const u8, port: u16) !std.net.Address {
+    if (host.len == 0) return error.MalformedAddress;
+    if (std.net.Address.parseIp(host, port)) |addr| return addr else |_| {}
+    // Not a literal: resolve it. A remote host is normally a name.
+    const list = try std.net.getAddressList(alloc, host, port);
+    defer list.deinit();
+    if (list.addrs.len == 0) return error.UnknownHostName;
+    return list.addrs[0];
+}
+test "parseAddr: literals, brackets, and the spellings that are refused" {
+    const alloc = std.testing.allocator;
+    // Literals only here: a name would send this test to a resolver, and
+    // what it answered would depend on the machine running it.
+    try std.testing.expectEqual(
+        @as(u16, 4433),
+        (try parseAddr(alloc, "127.0.0.1:4433")).getPort(),
+    );
+    try std.testing.expectEqual(@as(u16, 9), (try parseAddr(alloc, "127.0.0.1:9")).getPort());
+    // An omitted port means mux's own. Spelled out rather than written
+    // `default_port`, because comparing the parse's answer against the
+    // constant the parse reads would hold for any value and say nothing
+    // about the port — and this is the number the daemon at the other end
+    // has to agree on.
+    try std.testing.expectEqual(@as(u16, 4433), (try parseAddr(alloc, "10.0.0.2")).getPort());
+    const six = try parseAddr(alloc, "[::1]:9999");
+    try std.testing.expectEqual(@as(u16, 9999), six.getPort());
+    try std.testing.expect(six.any.family == std.posix.AF.INET6);
+    // Bracketed and portless: the brackets say where the address stops, so
+    // the port can default.
+    try std.testing.expectEqual(@as(u16, 4433), (try parseAddr(alloc, "[::1]")).getPort());
+    // An unbracketed IPv6 literal would have its last group read as a
+    // port. Refused rather than guessed at — the same refusal muxd's
+    // splitHostPort makes about its bind address.
+    try std.testing.expectError(error.MalformedAddress, parseAddr(alloc, "fe80::1:4433"));
+    try std.testing.expectError(error.MalformedAddress, parseAddr(alloc, "127.0.0.1:"));
+    try std.testing.expectError(error.MalformedAddress, parseAddr(alloc, "127.0.0.1:99999"));
+    try std.testing.expectError(error.MalformedAddress, parseAddr(alloc, ""));
+    try std.testing.expectError(error.MalformedAddress, parseAddr(alloc, ":4433"));
+}
+// ---------------------------------------------------------------------------
+// The pre-shared key
+// ---------------------------------------------------------------------------

Old		New
@@ -316,6 +316,7 @@ pub fn build(b: *std.Build) void {
316	client_mod.addImport("replica", replica_mod);	316	client_mod.addImport("replica", replica_mod);
317	client_mod.addImport("testtmp", testtmp_mod);	317	client_mod.addImport("testtmp", testtmp_mod);
318	client_mod.addImport("quic_client", quic_client_mod);	318	client_mod.addImport("quic_client", quic_client_mod);
		319	client_mod.addImport("quic", quic_mod);
319	// The client is the only thing that predicts: the overlay is a local	320	// The client is the only thing that predicts: the overlay is a local
320	// display decision and never becomes state anybody else can see.	321	// display decision and never becomes state anybody else can see.
321	client_mod.addImport("predict", predict_mod);	322	client_mod.addImport("predict", predict_mod);
@@ -431,6 +432,7 @@ pub fn build(b: *std.Build) void {
431	muxa_mod.addImport("protocol", protocol_mod);	432	muxa_mod.addImport("protocol", protocol_mod);
432	muxa_mod.addImport("sockpath", sockpath_mod);	433	muxa_mod.addImport("sockpath", sockpath_mod);
433	muxa_mod.addImport("quic_client", quic_client_mod);	434	muxa_mod.addImport("quic_client", quic_client_mod);
		435	muxa_mod.addImport("quic", quic_mod);
434	muxa_mod.addImport("xdg", xdg_mod);	436	muxa_mod.addImport("xdg", xdg_mod);
435		437
436	const mux_exe = b.addExecutable(.{ .name = "mux", .root_module = mux_mod });	438	const mux_exe = b.addExecutable(.{ .name = "mux", .root_module = mux_mod });