a73x

80dab7a2

refactor: one owner each for the sun_path bound and the key-picking rule

a73x   2026-08-13 13:58

Commit message
refactor: one owner each for the sun_path bound and the key-picking rule

M17 simplify round, hub batch: the mains. No behaviour changes except
the two the reviewer asked for by name.

  - `sockpath.max_sun_path`. The literal 107 stood in three binaries
    beside three hand-written "max 107" strings. The number is the
    kernel's (sun_path is 108 with the NUL) and now lives once, in the
    module that already owns what a socket path is; each binary keeps
    its own prefix and prints the constant. The e2e pins match on
    "<binary>: socket path too long", which is deliberately the part
    that did not move.

  - `xdg.pickKey(flag, env)`. "--key beats MUX_KEY_FILE, empty means
    unset" was spelled twice, and a drift between the copies would have
    meant two binaries authenticating a quic:// dial with different
    keys — the M17 field trial already showed what key confusion looks
    like from the outside (pure silence, indistinguishable from a dead
    box). One function beside resolveKeyPath, which is the step after
    it. Its own test covers the case both copies got right and neither
    stated: an empty --key does not fall through to the environment.

  - muxweb's target resolution moves onto an ArenaAllocator. The
    hand-rolled `owned` list existed to remember which of the resolved
    strings were allocations and which were borrowed from argv; every
    one of them lives as long as the hub does, so the distinction was
    bookkeeping for a lifetime nobody needed to track. `resolveKeyPath`
    collapses to `.given, .default => |p| p` and the `.missing` arm
    stops freeing on its way out the door.

  - muxweb's parseArgs refuses with `error.Usage` instead of a
    `.usage_error` value. Seven refusals each carried their own
    `tiles.deinit` beside the existing errdefer — seven chances to
    forget one. The tests assert the error and, through the testing
    allocator, that no refusing path leaks: three of the new cases
    refuse with a tile already on the list, so the cleanup is exercised
    rather than assumed.

The two behaviour changes:

  - `muxweb --port 0` is a usage error. Zero asks the kernel to pick,
    while the hub prints the port it was ASKED for as the door to
    open — so it announced a door it had not opened. Same shape as the
    `--quic-idle-ms 0` refusal, same reason: the number inverts what
    typing it means.

  - `mux HOST --quic-idle-ms N` no longer accepts the flag and drops
    it. The .host ParseResult carried no idle_ms at all, so the value
    never reached HandoffTarget and the handoff's QUIC link ran on the
    15s default however the flag was spelled. muxweb's HOST tiles have
    always threaded it; mux catches up, and the parse test pins both
    the value and the zero refusal on this spelling. The threading past
    the parse — HandoffTarget's field — has no seam under test and
    rides on muxweb's identical construction.

make test green; make e2e green (23 scenarios / 35 convergence
points, unchanged).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build.zig
Old New
@@ -317,6 +317,8 @@ pub fn build(b: *std.Build) void {
317 mux_mod.addImport("xdg", xdg_mod); 317 mux_mod.addImport("xdg", xdg_mod);
318 mux_mod.addImport("spawn", spawn_mod); 318 mux_mod.addImport("spawn", spawn_mod);
319 mux_mod.addImport("handoff", handoff_mod); 319 mux_mod.addImport("handoff", handoff_mod);
320 // The sun_path bound only; the client binds no socket itself.
321 mux_mod.addImport("sockpath", sockpath_mod);
320 322
321 // Test helpers, built as real binaries because that is how the suite 323 // Test helpers, built as real binaries because that is how the suite
322 // uses them: rawmode is a deterministic stand-in for an editor (nvim's 324 // uses them: rawmode is a deterministic stand-in for an editor (nvim's
@@ -377,6 +379,8 @@ pub fn build(b: *std.Build) void {
377 exe_mod.addImport("spawn", spawn_mod); 379 exe_mod.addImport("spawn", spawn_mod);
378 // `muxd endpoint` prints the announce line this module spells. 380 // `muxd endpoint` prints the announce line this module spells.
379 exe_mod.addImport("handoff", handoff_mod); 381 exe_mod.addImport("handoff", handoff_mod);
382 // The sun_path bound, checked before any verb acts on the path.
383 exe_mod.addImport("sockpath", sockpath_mod);
380 384
381 const exe = b.addExecutable(.{ .name = "muxd", .root_module = exe_mod }); 385 const exe = b.addExecutable(.{ .name = "muxd", .root_module = exe_mod });
382 // Zig 0.15's self-hosted x86_64 linker can't handle the .sframe 386 // Zig 0.15's self-hosted x86_64 linker can't handle the .sframe
@@ -503,6 +507,8 @@ pub fn build(b: *std.Build) void {
503 webhub_main_mod.addImport("xdg", xdg_mod); 507 webhub_main_mod.addImport("xdg", xdg_mod);
504 // The HOST tile's recipe comes from the same owner mux_main uses. 508 // The HOST tile's recipe comes from the same owner mux_main uses.
505 webhub_main_mod.addImport("handoff", handoff_mod); 509 webhub_main_mod.addImport("handoff", handoff_mod);
510 // ...and the sun_path bound its --sock tiles are refused against.
511 webhub_main_mod.addImport("sockpath", sockpath_mod);
506 webhub_main_mod.addImport("build_options", version_opts.createModule()); 512 webhub_main_mod.addImport("build_options", version_opts.createModule());
507 webhub_main_mod.addAnonymousImport("index.html", .{ .root_source_file = b.path("web/index.html") }); 513 webhub_main_mod.addAnonymousImport("index.html", .{ .root_source_file = b.path("web/index.html") });
508 webhub_main_mod.addAnonymousImport("mux.js", .{ .root_source_file = b.path("web/mux.js") }); 514 webhub_main_mod.addAnonymousImport("mux.js", .{ .root_source_file = b.path("web/mux.js") });
src/main.zig
Old New
@@ -11,6 +11,7 @@ const build_options = @import("build_options");
11 const xdg = @import("xdg"); 11 const xdg = @import("xdg");
12 const spawn = @import("spawn"); 12 const spawn = @import("spawn");
13 const handoff = @import("handoff"); 13 const handoff = @import("handoff");
14 const sockpath = @import("sockpath");
14 15
15 const usage = 16 const usage =
16 \\usage: 17 \\usage:
@@ -295,19 +296,19 @@ pub fn main() !u8 {
295 try defaultSockPath(alloc); 296 try defaultSockPath(alloc);
296 defer alloc.free(sock_path); 297 defer alloc.free(sock_path);
297 298
298 // sun_path is 108 bytes including the NUL. Checked here, once, before 299 // The sun_path bound (sockpath.max_sun_path). Checked here, once,
299 // any command acts: the alternative is a spawned daemon that can never 300 // before any command acts: the alternative is a spawned daemon that can
300 // answer and a 2s timeout story about a path that was doomed at parse. 301 // never answer and a 2s timeout story about a path doomed at parse.
301 // `--version` and `keygen` are dispatched from the switch below, i.e. 302 // `--version` and `keygen` are dispatched from the switch below, i.e.
302 // after this point, so they are exempted by their rows rather than by 303 // after this point, so they are exempted by their rows rather than by
303 // order — neither touches the socket, and neither should be refused 304 // order — neither touches the socket, and neither should be refused
304 // over it. Which verbs those are is their rows' business, not this 305 // over it. Which verbs those are is their rows' business, not this
305 // line's. 306 // line's.
306 const uses_socket = specForCmd(o.cmd).uses_socket; 307 const uses_socket = specForCmd(o.cmd).uses_socket;
307 if (uses_socket and sock_path.len > 107) { 308 if (uses_socket and sock_path.len > sockpath.max_sun_path) {
308 std.debug.print( 309 std.debug.print(
309 "muxd: socket path too long ({d} bytes, max 107): {s}\n", 310 "muxd: socket path too long ({d} bytes, max {d}): {s}\n",
310 .{ sock_path.len, sock_path }, 311 .{ sock_path.len, sockpath.max_sun_path, sock_path },
311 ); 312 );
312 return 1; 313 return 1;
313 } 314 }
src/mux_main.zig
Old New
@@ -9,6 +9,7 @@ const build_options = @import("build_options");
9 const xdg = @import("xdg"); 9 const xdg = @import("xdg");
10 const spawn = @import("spawn"); 10 const spawn = @import("spawn");
11 const handoff = @import("handoff"); 11 const handoff = @import("handoff");
12 const sockpath = @import("sockpath");
12 13
13 const usage = 14 const usage =
14 \\usage: mux [HOST | --sock PATH | --via CMD | quic://HOST[:PORT]] 15 \\usage: mux [HOST | --sock PATH | --via CMD | quic://HOST[:PORT]]
@@ -30,8 +31,12 @@ const ParseResult = union(enum) {
30 /// At most one of these is set; both null means the default local socket. 31 /// At most one of these is set; both null means the default local socket.
31 attach: struct { sock: ?[]const u8 = null, via: ?[]const u8 = null }, 32 attach: struct { sock: ?[]const u8 = null, via: ?[]const u8 = null },
32 /// A bare hostname: the ssh recipe is built from it in main, where there 33 /// A bare hostname: the ssh recipe is built from it in main, where there
33 /// is an allocator to build it with. 34 /// is an allocator to build it with. `idle_ms` rides along because the
34 host: []const u8, 35 /// handoff ends in a QUIC link like any other — muxweb's HOST tiles
36 /// have always carried it, and mux dropping it on the floor made
37 /// `--quic-idle-ms` silently do nothing on exactly the spelling most
38 /// people use.
39 host: struct { name: []const u8, idle_ms: u32 },
35 /// A direct QUIC attach. The key is resolved in main, where the 40 /// A direct QUIC attach. The key is resolved in main, where the
36 /// environment can be consulted. 41 /// environment can be consulted.
37 quic: struct { host_port: []const u8, key: ?[]const u8, idle_ms: u32 }, 42 quic: struct { host_port: []const u8, key: ?[]const u8, idle_ms: u32 },
@@ -108,22 +113,20 @@ fn parseArgs(args: []const [:0]const u8, env_key: ?[]const u8) ParseResult {
108 if (named > 1) return .conflict; 113 if (named > 1) return .conflict;
109 114
110 if (quic) |hp| { 115 if (quic) |hp| {
111 // --key wins over the environment; the environment exists so a 116 // Neither spelling being set is not a refusal: main has a default
112 // shell can set it once rather than repeating it per invocation. 117 // path to try, and parse is not allowed to look at the filesystem.
113 // Neither being set is not a refusal: main has a default path to 118 return .{ .quic = .{
114 // try, and parse is not allowed to look at the filesystem. 119 .host_port = hp,
115 var k = key orelse env_key; 120 .key = xdg.pickKey(key, env_key),
116 if (k) |kk| { 121 .idle_ms = idle_ms,
117 if (kk.len == 0) k = null; 122 } };
118 }
119 return .{ .quic = .{ .host_port = hp, .key = k, .idle_ms = idle_ms } };
120 } 123 }
121 // A key with no quic:// has nothing to authenticate and is ignored 124 // A key with no quic:// has nothing to authenticate and is ignored
122 // rather than refused: unlike muxd, where --key without --quic means a 125 // rather than refused: unlike muxd, where --key without --quic means a
123 // listener was meant, here it is one env var away from being set for 126 // listener was meant, here it is one env var away from being set for
124 // every invocation in a shell, and refusing `mux --sock ...` because 127 // every invocation in a shell, and refusing `mux --sock ...` because
125 // MUX_KEY_FILE happens to be exported would be absurd. 128 // MUX_KEY_FILE happens to be exported would be absurd.
126 if (host) |h| return .{ .host = h }; 129 if (host) |h| return .{ .host = .{ .name = h, .idle_ms = idle_ms } };
127 return .{ .attach = .{ .sock = sock, .via = via } }; 130 return .{ .attach = .{ .sock = sock, .via = via } };
128 } 131 }
129 132
@@ -182,12 +185,13 @@ pub fn main() !u8 {
182 // warm attach dials from the cache and never spawns ssh at all. 185 // warm attach dials from the cache and never spawns ssh at all.
183 // handoff.recipeFor owns both pieces; muxweb builds its HOST 186 // handoff.recipeFor owns both pieces; muxweb builds its HOST
184 // tiles from the same call. 187 // tiles from the same call.
185 const r = try handoff.recipeFor(alloc, h); 188 const r = try handoff.recipeFor(alloc, h.name);
186 defer r.deinit(alloc); 189 defer r.deinit(alloc);
187 return client.attach(alloc, .{ .hand = .{ 190 return client.attach(alloc, .{ .hand = .{
188 .host = h, 191 .host = h.name,
189 .ssh_cmd = r.ssh_cmd, 192 .ssh_cmd = r.ssh_cmd,
190 .cache_path = r.cache_path, 193 .cache_path = r.cache_path,
194 .idle_ms = h.idle_ms,
191 } }); 195 } });
192 }, 196 },
193 .attach => |t| { 197 .attach => |t| {
@@ -200,17 +204,17 @@ pub fn main() !u8 {
200 try std.fmt.allocPrint(alloc, "/tmp/muxd-{d}.sock", .{std.os.linux.getuid()}); 204 try std.fmt.allocPrint(alloc, "/tmp/muxd-{d}.sock", .{std.os.linux.getuid()});
201 defer alloc.free(sock_path); 205 defer alloc.free(sock_path);
202 206
203 // The same 107-byte `sun_path` guard muxd applies (main.zig), for 207 // The same `sun_path` guard muxd applies (main.zig), for the
204 // the same reason and with the same constant. It sits before the 208 // same reason and off the same constant. It sits before the
205 // PATH search rather than at the connect because auto-start would 209 // PATH search rather than at the connect because auto-start would
206 // otherwise reach it first: mux finds muxd, spawns a child that 210 // otherwise reach it first: mux finds muxd, spawns a child that
207 // refuses the path instantly, and polls the full 2s into "daemon 211 // refuses the path instantly, and polls the full 2s into "daemon
208 // did not answer" — a timeout story about a path that was doomed 212 // did not answer" — a timeout story about a path that was doomed
209 // at parse. Refusing here costs nothing and says the real thing. 213 // at parse. Refusing here costs nothing and says the real thing.
210 if (sock_path.len > 107) { 214 if (sock_path.len > sockpath.max_sun_path) {
211 std.debug.print( 215 std.debug.print(
212 "mux: socket path too long ({d} bytes, max 107): {s}\n", 216 "mux: socket path too long ({d} bytes, max {d}): {s}\n",
213 .{ sock_path.len, sock_path }, 217 .{ sock_path.len, sockpath.max_sun_path, sock_path },
214 ); 218 );
215 return 1; 219 return 1;
216 } 220 }
@@ -271,14 +275,18 @@ test "parseArgs: --sock and --via each name their transport" {
271 test "parseArgs: a bare word is a host to hop to" { 275 test "parseArgs: a bare word is a host to hop to" {
272 const h = parse(&.{ "mux", "vm1" }); 276 const h = parse(&.{ "mux", "vm1" });
273 try std.testing.expect(h == .host); 277 try std.testing.expect(h == .host);
274 try std.testing.expectEqualStrings("vm1", h.host); 278 try std.testing.expectEqualStrings("vm1", h.host.name);
279 // Spelled out rather than written `client.quic_idle_ms_default` — see
280 // the quic:// test for why asserting against the parser's own constant
281 // could never catch the number changing.
282 try std.testing.expectEqual(@as(u32, 15_000), h.host.idle_ms);
275 283
276 // The user@host form is just as bare a word; nothing parses inside it, 284 // The user@host form is just as bare a word; nothing parses inside it,
277 // which is what lets ssh's own config (aliases, ports, ProxyJump) keep 285 // which is what lets ssh's own config (aliases, ports, ProxyJump) keep
278 // working untouched. 286 // working untouched.
279 const u = parse(&.{ "mux", "ubuntu@sandbox-9b70e9" }); 287 const u = parse(&.{ "mux", "ubuntu@sandbox-9b70e9" });
280 try std.testing.expect(u == .host); 288 try std.testing.expect(u == .host);
281 try std.testing.expectEqualStrings("ubuntu@sandbox-9b70e9", u.host); 289 try std.testing.expectEqualStrings("ubuntu@sandbox-9b70e9", u.host.name);
282 } 290 }
283 291
284 test "parseArgs: naming two transports is a conflict, however it is spelled" { 292 test "parseArgs: naming two transports is a conflict, however it is spelled" {
@@ -354,6 +362,15 @@ test "parseArgs: --quic-idle-ms parses, and refuses what ngtcp2 would invert" {
354 const t = parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "1500" }); 362 const t = parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "1500" });
355 try std.testing.expectEqual(@as(u32, 1500), t.quic.idle_ms); 363 try std.testing.expectEqual(@as(u32, 1500), t.quic.idle_ms);
356 364
365 // A bare HOST ends in a QUIC link too, so the flag has to reach it —
366 // the .host result carried no idle_ms at all and the flag was accepted
367 // and then dropped, which is worse than refusing it. muxweb's HOST
368 // tiles were already right; this is mux catching up.
369 const h = parse(&.{ "mux", "vm1", "--quic-idle-ms", "1500" });
370 try std.testing.expect(h == .host);
371 try std.testing.expectEqual(@as(u32, 1500), h.host.idle_ms);
372 try std.testing.expect(parse(&.{ "mux", "vm1", "--quic-idle-ms", "0" }) == .usage_error);
373
357 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "0" }) == .usage_error); 374 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "0" }) == .usage_error);
358 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "soon" }) == .usage_error); 375 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "soon" }) == .usage_error);
359 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "99999999999" }) == .usage_error); 376 try std.testing.expect(parse(&.{ "mux", "quic://a:1", "--key", "/k", "--quic-idle-ms", "99999999999" }) == .usage_error);
src/sockpath.zig
Old New
@@ -9,6 +9,12 @@
9 //! takes. 9 //! takes.
10 const std = @import("std"); 10 const std = @import("std");
11 11
12 /// The usable bytes of `sockaddr_un.sun_path`: the field is 108 and the
13 /// last one belongs to the NUL. All three binaries refuse a longer path
14 /// by name before acting on it, each in its own words — the number is
15 /// the kernel's and belongs in one place, the wording is theirs.
16 pub const max_sun_path = 107;
17
12 /// A socket file's identity at the moment it was bound, so teardown can 18 /// A socket file's identity at the moment it was bound, so teardown can
13 /// tell our socket from one that replaced it. 19 /// tell our socket from one that replaced it.
14 /// 20 ///
src/webhub_main.zig
Old New
@@ -12,6 +12,7 @@ const webhub = @import("webhub");
12 const build_options = @import("build_options"); 12 const build_options = @import("build_options");
13 const xdg = @import("xdg"); 13 const xdg = @import("xdg");
14 const handoff = @import("handoff"); 14 const handoff = @import("handoff");
15 const sockpath = @import("sockpath");
15 16
16 const usage = 17 const usage =
17 \\usage: muxweb TARGET [TARGET ...] [--port N] 18 \\usage: muxweb TARGET [TARGET ...] [--port N]
@@ -48,14 +49,20 @@ const Parsed = struct {
48 const ParseResult = union(enum) { 49 const ParseResult = union(enum) {
49 serve: Parsed, 50 serve: Parsed,
50 version, 51 version,
51 usage_error,
52 }; 52 };
53 53
54 /// A usage mistake is an ERROR, not a third result: it is the one outcome
55 /// with nothing to hand back, and saying so lets the single errdefer own
56 /// the tile list on every refusing path. Spelling it as a value meant a
57 /// `tiles.deinit` beside each of the seven `return .usage_error`s, which
58 /// is seven chances to forget one.
59 const ParseError = error{Usage} || std.mem.Allocator.Error;
60
54 fn parseArgs( 61 fn parseArgs(
55 alloc: std.mem.Allocator, 62 alloc: std.mem.Allocator,
56 args: []const [:0]const u8, 63 args: []const [:0]const u8,
57 env_key: ?[]const u8, 64 env_key: ?[]const u8,
58 ) !ParseResult { 65 ) ParseError!ParseResult {
59 var p = Parsed{ .tiles = .empty }; 66 var p = Parsed{ .tiles = .empty };
60 errdefer p.tiles.deinit(alloc); 67 errdefer p.tiles.deinit(alloc);
61 var key: ?[]const u8 = null; 68 var key: ?[]const u8 = null;
@@ -64,6 +71,9 @@ fn parseArgs(
64 while (i < args.len) : (i += 1) { 71 while (i < args.len) : (i += 1) {
65 const a = args[i]; 72 const a = args[i];
66 if (std.mem.eql(u8, a, "--version")) { 73 if (std.mem.eql(u8, a, "--version")) {
74 // The one non-error early return, so the one that still frees
75 // for itself: errdefer does not run on the way out with a
76 // result in hand.
67 p.tiles.deinit(alloc); 77 p.tiles.deinit(alloc);
68 return .version; 78 return .version;
69 } else if (std.mem.eql(u8, a, "--sock") and i + 1 < args.len) { 79 } else if (std.mem.eql(u8, a, "--sock") and i + 1 < args.len) {
@@ -71,50 +81,33 @@ fn parseArgs(
71 try p.tiles.append(alloc, .{ .sock = args[i] }); 81 try p.tiles.append(alloc, .{ .sock = args[i] });
72 } else if (std.mem.eql(u8, a, "--port") and i + 1 < args.len) { 82 } else if (std.mem.eql(u8, a, "--port") and i + 1 < args.len) {
73 i += 1; 83 i += 1;
74 p.port = std.fmt.parseInt(u16, args[i], 10) catch { 84 p.port = std.fmt.parseInt(u16, args[i], 10) catch return error.Usage;
75 p.tiles.deinit(alloc); 85 // Port 0 asks the kernel to choose, and the hub prints the port
76 return .usage_error; 86 // it was asked for as the door to open — a door nobody could
77 }; 87 // find. Refused like `--quic-idle-ms 0` and for the same
88 // reason: the number inverts what typing it means.
89 if (p.port == 0) return error.Usage;
78 } else if (std.mem.eql(u8, a, "--key") and i + 1 < args.len) { 90 } else if (std.mem.eql(u8, a, "--key") and i + 1 < args.len) {
79 i += 1; 91 i += 1;
80 key = args[i]; 92 key = args[i];
81 } else if (std.mem.eql(u8, a, "--quic-idle-ms") and i + 1 < args.len) { 93 } else if (std.mem.eql(u8, a, "--quic-idle-ms") and i + 1 < args.len) {
82 i += 1; 94 i += 1;
83 const n = std.fmt.parseInt(u32, args[i], 10) catch { 95 const n = std.fmt.parseInt(u32, args[i], 10) catch return error.Usage;
84 p.tiles.deinit(alloc); 96 if (n == 0) return error.Usage;
85 return .usage_error;
86 };
87 if (n == 0) {
88 p.tiles.deinit(alloc);
89 return .usage_error;
90 }
91 p.idle_ms = n; 97 p.idle_ms = n;
92 } else if (std.mem.startsWith(u8, a, "quic://")) { 98 } else if (std.mem.startsWith(u8, a, "quic://")) {
93 const hp = a["quic://".len..]; 99 const hp = a["quic://".len..];
94 if (hp.len == 0) { 100 if (hp.len == 0) return error.Usage;
95 p.tiles.deinit(alloc);
96 return .usage_error;
97 }
98 try p.tiles.append(alloc, .{ .quic = hp }); 101 try p.tiles.append(alloc, .{ .quic = hp });
99 } else if (a.len > 0 and a[0] != '-') { 102 } else if (a.len > 0 and a[0] != '-') {
100 try p.tiles.append(alloc, .{ .host = a }); 103 try p.tiles.append(alloc, .{ .host = a });
101 } else { 104 } else {
102 p.tiles.deinit(alloc); 105 return error.Usage;
103 return .usage_error;
104 } 106 }
105 } 107 }
106 108
107 if (p.tiles.items.len == 0) { 109 if (p.tiles.items.len == 0) return error.Usage;
108 p.tiles.deinit(alloc); 110 p.key = xdg.pickKey(key, env_key);
109 return .usage_error;
110 }
111 // --key wins over the environment, exactly mux_main's rule; empty
112 // means unset either way.
113 var k = key orelse env_key;
114 if (k) |kk| {
115 if (kk.len == 0) k = null;
116 }
117 p.key = k;
118 return .{ .serve = p }; 111 return .{ .serve = p };
119 } 112 }
120 113
@@ -126,17 +119,20 @@ pub fn main() !u8 {
126 const args = try std.process.argsAlloc(alloc); 119 const args = try std.process.argsAlloc(alloc);
127 defer std.process.argsFree(alloc, args); 120 defer std.process.argsFree(alloc, args);
128 121
129 var parsed = switch (try parseArgs(alloc, args, std.posix.getenv(client_key_env))) { 122 const result = parseArgs(alloc, args, std.posix.getenv(client_key_env)) catch |err| switch (err) {
123 error.Usage => {
124 std.debug.print("{s}", .{usage});
125 return 2;
126 },
127 else => |e| return e,
128 };
129 var parsed = switch (result) {
130 .version => { 130 .version => {
131 var vbuf: [64]u8 = undefined; 131 var vbuf: [64]u8 = undefined;
132 const s = std.fmt.bufPrint(&vbuf, "muxweb {s}\n", .{build_options.version}) catch unreachable; 132 const s = std.fmt.bufPrint(&vbuf, "muxweb {s}\n", .{build_options.version}) catch unreachable;
133 _ = std.posix.write(std.posix.STDOUT_FILENO, s) catch {}; 133 _ = std.posix.write(std.posix.STDOUT_FILENO, s) catch {};
134 return 0; 134 return 0;
135 }, 135 },
136 .usage_error => {
137 std.debug.print("{s}", .{usage});
138 return 2;
139 },
140 .serve => |p| p, 136 .serve => |p| p,
141 }; 137 };
142 defer parsed.deinit(alloc); 138 defer parsed.deinit(alloc);
@@ -145,46 +141,45 @@ pub fn main() !u8 {
145 // mux_main uses — handoff.recipeFor and xdg.resolveKeyPath — so the 141 // mux_main uses — handoff.recipeFor and xdg.resolveKeyPath — so the
146 // two binaries cannot drift on what a bare HOST or a `quic://` means. 142 // two binaries cannot drift on what a bare HOST or a `quic://` means.
147 // Labels are the argv spellings verbatim. 143 // Labels are the argv spellings verbatim.
144 //
145 // An arena, because every string built here lives exactly as long as
146 // the hub does: the Targets are handed to every tile thread and the
147 // labels to every /tiles response, so nothing is ever freed early and
148 // the hand-rolled list of pointers-to-free was a lifetime nobody
149 // needed to track. Which resolutions borrow from argv and which
150 // allocate stops mattering: both end at the same deinit.
151 var arena_state = std.heap.ArenaAllocator.init(alloc);
152 defer arena_state.deinit();
153 const arena = arena_state.allocator();
154
148 var targets: std.ArrayList(client.Target) = .empty; 155 var targets: std.ArrayList(client.Target) = .empty;
149 defer targets.deinit(alloc);
150 var labels: std.ArrayList([]const u8) = .empty; 156 var labels: std.ArrayList([]const u8) = .empty;
151 defer labels.deinit(alloc);
152 var owned: std.ArrayList([]const u8) = .empty;
153 defer {
154 for (owned.items) |s| alloc.free(s);
155 owned.deinit(alloc);
156 }
157 157
158 for (parsed.tiles.items) |spec| switch (spec) { 158 for (parsed.tiles.items) |spec| switch (spec) {
159 .sock => |path| { 159 .sock => |path| {
160 if (path.len > 107) { 160 if (path.len > sockpath.max_sun_path) {
161 std.debug.print("muxweb: socket path too long ({d} bytes, max 107): {s}\n", .{ path.len, path }); 161 std.debug.print("muxweb: socket path too long ({d} bytes, max {d}): {s}\n", .{
162 path.len, sockpath.max_sun_path, path,
163 });
162 return 2; 164 return 2;
163 } 165 }
164 try targets.append(alloc, .{ .sock = path }); 166 try targets.append(arena, .{ .sock = path });
165 try labels.append(alloc, path); 167 try labels.append(arena, path);
166 }, 168 },
167 .host => |h| { 169 .host => |h| {
168 const r = try handoff.recipeFor(alloc, h); 170 const r = try handoff.recipeFor(arena, h);
169 try owned.append(alloc, r.ssh_cmd); 171 try targets.append(arena, .{ .hand = .{
170 if (r.cache_path) |c| try owned.append(alloc, c);
171 try targets.append(alloc, .{ .hand = .{
172 .host = h, 172 .host = h,
173 .ssh_cmd = r.ssh_cmd, 173 .ssh_cmd = r.ssh_cmd,
174 .cache_path = r.cache_path, 174 .cache_path = r.cache_path,
175 .idle_ms = parsed.idle_ms, 175 .idle_ms = parsed.idle_ms,
176 } }); 176 } });
177 try labels.append(alloc, h); 177 try labels.append(arena, h);
178 }, 178 },
179 .quic => |hp| { 179 .quic => |hp| {
180 const key_path = switch (try xdg.resolveKeyPath(alloc, parsed.key)) { 180 const key_path = switch (try xdg.resolveKeyPath(arena, parsed.key)) {
181 .given => |p| p, 181 .given, .default => |p| p,
182 .default => |p| blk: {
183 try owned.append(alloc, p);
184 break :blk p;
185 },
186 .missing => |p| { 182 .missing => |p| {
187 defer alloc.free(p);
188 std.debug.print( 183 std.debug.print(
189 "muxweb: no key for quic://{s}: pass --key, set MUX_KEY_FILE, or run `muxd keygen` (default {s})\n", 184 "muxweb: no key for quic://{s}: pass --key, set MUX_KEY_FILE, or run `muxd keygen` (default {s})\n",
190 .{ hp, p }, 185 .{ hp, p },
@@ -192,14 +187,13 @@ pub fn main() !u8 {
192 return 2; 187 return 2;
193 }, 188 },
194 }; 189 };
195 try targets.append(alloc, .{ .quic = .{ 190 try targets.append(arena, .{ .quic = .{
196 .host_port = hp, 191 .host_port = hp,
197 .key_path = key_path, 192 .key_path = key_path,
198 .idle_ms = parsed.idle_ms, 193 .idle_ms = parsed.idle_ms,
199 } }); 194 } });
200 const label = try std.fmt.allocPrint(alloc, "quic://{s}", .{hp}); 195 const label = try std.fmt.allocPrint(arena, "quic://{s}", .{hp});
201 try owned.append(alloc, label); 196 try labels.append(arena, label);
202 try labels.append(alloc, label);
203 }, 197 },
204 }; 198 };
205 199
@@ -262,11 +256,31 @@ test "parse: three spellings become three tiles in argv order, port and key bind
262 256
263 test "parse: zero targets, bad flags, and flag-beats-env" { 257 test "parse: zero targets, bad flags, and flag-beats-env" {
264 const alloc = std.testing.allocator; 258 const alloc = std.testing.allocator;
259 // Every refusal arrives as error.Usage — and the testing allocator is
260 // the other half of this pin: a refusal that leaked the tile list
261 // would fail the test that provoked it, which is what the single
262 // errdefer now guarantees on all seven paths.
263 //
265 // No targets is a usage error, not an empty wall. 264 // No targets is a usage error, not an empty wall.
266 try std.testing.expect((try parseArgs(alloc, &[_][:0]const u8{"muxweb"}, null)) == .usage_error); 265 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{"muxweb"}, null));
267 // A flag with no value is a usage mistake, not a transport. 266 // A flag with no value is a usage mistake, not a transport.
268 try std.testing.expect((try parseArgs(alloc, &[_][:0]const u8{ "muxweb", "--sock" }, null)) == .usage_error); 267 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "--sock" }, null));
269 try std.testing.expect((try parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--port", "x" }, null)) == .usage_error); 268 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--port", "x" }, null));
269 // The refusals that had a tile on the list already, so the cleanup is
270 // load-bearing rather than theoretical.
271 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--wat" }, null));
272 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "quic://" }, null));
273 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--quic-idle-ms", "0" }, null));
274 // Port 0 means "kernel, you pick" — but the hub announces the port it
275 // was asked for, so the door it prints is not the door it opened.
276 try std.testing.expectError(error.Usage, parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--port", "0" }, null));
277 // ...and an ordinary port still binds, so the refusal is the zero and
278 // not the flag.
279 {
280 var r = (try parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h", "--port", "1" }, null)).serve;
281 defer r.deinit(alloc);
282 try std.testing.expectEqual(@as(u16, 1), r.port);
283 }
270 // Env fills in when --key is absent; --key wins when both are set. 284 // Env fills in when --key is absent; --key wins when both are set.
271 { 285 {
272 var r = (try parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h" }, "/env-key")).serve; 286 var r = (try parseArgs(alloc, &[_][:0]const u8{ "muxweb", "h" }, "/env-key")).serve;
src/xdg.zig
Old New
@@ -41,6 +41,17 @@ pub fn resolveKeyPath(alloc: std.mem.Allocator, given: ?[]const u8) !KeyResoluti
41 return .{ .default = p }; 41 return .{ .default = p };
42 } 42 }
43 43
44 /// Which spelling of the key path a command line meant: `--key` beats
45 /// `$MUX_KEY_FILE`, because the flag is the more specific statement of
46 /// intent, and an empty spelling of either means unset rather than a key
47 /// at the empty path. Both binaries' parsers call this — it lives beside
48 /// resolveKeyPath because it is the step before it, and a drift between
49 /// two copies would make them authenticate with different keys.
50 pub fn pickKey(flag: ?[]const u8, env: ?[]const u8) ?[]const u8 {
51 const k = flag orelse env orelse return null;
52 return if (k.len == 0) null else k;
53 }
54
44 pub fn keyPathFrom( 55 pub fn keyPathFrom(
45 alloc: std.mem.Allocator, 56 alloc: std.mem.Allocator,
46 xdg_config_home: ?[]const u8, 57 xdg_config_home: ?[]const u8,
@@ -153,6 +164,19 @@ test "keyPathFrom: XDG_CONFIG_HOME wins, HOME is the fallback, empty is unset" {
153 try std.testing.expectError(error.NoHome, keyPathFrom(a, null, null)); 164 try std.testing.expectError(error.NoHome, keyPathFrom(a, null, null));
154 } 165 }
155 166
167 test "pickKey: the flag wins, and empty is unset either way" {
168 try std.testing.expectEqualStrings("/flag", pickKey("/flag", "/env").?);
169 try std.testing.expectEqualStrings("/env", pickKey(null, "/env").?);
170 try std.testing.expectEqualStrings("/flag", pickKey("/flag", null).?);
171 try std.testing.expectEqual(@as(?[]const u8, null), pickKey(null, null));
172 // An empty environment variable is unset, per the basedir spec's rule
173 // and common sense.
174 try std.testing.expectEqual(@as(?[]const u8, null), pickKey(null, ""));
175 // An empty --key does NOT fall through to the environment: the flag
176 // was named, so it is the answer, and the answer is nothing.
177 try std.testing.expectEqual(@as(?[]const u8, null), pickKey("", "/env"));
178 }
179
156 test "logPathFrom: same shape against XDG_STATE_HOME" { 180 test "logPathFrom: same shape against XDG_STATE_HOME" {
157 const a = std.testing.allocator; 181 const a = std.testing.allocator;
158 const explicit = try logPathFrom(a, "/tmp/state", "/home/u"); 182 const explicit = try logPathFrom(a, "/tmp/state", "/home/u");