diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index c1b8966..cd7bf63 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [main] + branches: [master] pull_request: - branches: [main] + branches: [master] env: ZIG_VERSION: "0.16.0" diff --git a/.gitea/workflows/live-tls.yml b/.gitea/workflows/live-tls.yml index 255cc9c..985349f 100644 --- a/.gitea/workflows/live-tls.yml +++ b/.gitea/workflows/live-tls.yml @@ -1,9 +1,11 @@ name: Live TLS -# Manual only. This workflow reaches public DoT resolvers, so it is +# Manual and weekly. This workflow reaches public DoT resolvers, so it is # non-blocking by construction and never runs on push or pull_request. on: workflow_dispatch: + schedule: + - cron: "0 5 * * 1" env: ZIG_VERSION: "0.16.0" diff --git a/AGENTS.md b/AGENTS.md index 34cb524..67b62f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,3 +35,32 @@ What that means in practice: commented-out code. - Git: GPG-signed commits (`git commit -S`), simple lowercase messages, no generated-by footers. + +## Reading `zig build test` output + +A fully passing `zig build test` still prints a line like `failed command: +.../test --cache-dir=... --seed=... --listen=-`, and still exits 0. That line +is a known upstream zig 0.16.0 labelling defect. It does not mean a test +failed, and no test binary crashed. + +The build runner sets a step's `result_failed_command` on every spawn +(`std/Build/Step/Run.zig:1540`) and never clears it on success. It then prints +a step's diagnostics whenever the step wrote anything to stderr, explicitly "no +matter the result" (`compiler/build_runner.zig:1381`), and that printer emits +the `failed command: ` label unconditionally when the field is set +(`compiler/build_runner.zig:1515`). Our suite writes to stderr on every run, +because the tests that cover the warning paths log through the real sink. A +minimal reproducer with no mbedTLS and no C — one passing test whose body is a +`std.debug.print` — prints the same label and reports "3/3 steps succeeded; +1/1 tests passed"; deleting the print removes the label. No upstream issue +matched a search, so the reference is the 0.16.0 source lines above. + +Any *other* failure text is real. Trust the summary line: `zig build test` +exiting non-zero, a `N failed` count, or a panic backtrace all mean a genuine +failure. Do not filter, wrap, or suppress the runner's output to hide the +label — that would hide real failures with it. + +One trap: running a cached test binary by hand with `--listen=-` aborts with +`internal test runner failure: EndOfStream`. That is not a teardown bug; the +IPC runner is talking to a closed stdin because no build runner is on the other +end. Run the binary with no arguments to get the plain stdio report. diff --git a/build.zig b/build.zig index 31cecf5..5f2d0f4 100644 --- a/build.zig +++ b/build.zig @@ -21,14 +21,34 @@ pub fn build(b: *std.Build) void { const fuzz = b.option(bool, "fuzz", "Build the fuzz targets with the LLVM backend (required for --fuzz)") orelse false; const version_string = b.option([]const u8, "version-string", "Version reported by `nxdns version`") orelse "0.1.0-dev"; const git_commit = b.option([]const u8, "git-commit", "Git commit reported by `nxdns version`") orelse "unknown"; - const web_dist = b.option([]const u8, "web-dist", "Built web UI directory to embed (default: the placeholder page)") orelse "web/dist-placeholder"; + const web_dist = b.option( + []const u8, + "web-dist", + "Built web UI directory to embed (default: the placeholder page). " ++ + "Only the exact value `web/dist` gets the freshness check; " ++ + "the placeholder and any other path skip it.", + ) orelse "web/dist-placeholder"; // `b.path` panics on absolute paths, and a CI artifact directory is one. const web_dist_path: std.Build.LazyPath = if (std.fs.path.isAbsolute(web_dist)) .{ .cwd_relative = web_dist } else b.path(web_dist); - const web_assets = webAssetsIndex(b, web_dist_path); + + // A stale `web/dist` shipped a crashing settings page once (milestone-15 + // ruling 5). The stamp is checked only for the real dist tree: the + // placeholder has no sources to be stale against, and an explicit path is a + // CI artifact that was built elsewhere. + const web_dist_check: ?*std.Build.Step = if (std.mem.eql(u8, web_dist, "web/dist")) check: { + const run_check = b.addSystemCommand(&.{"node"}); + // The script path goes through `b.path` so it resolves against the + // build root: `zig build` run from any other directory would not find + // a cwd-relative one. + run_check.addFileArg(b.path("web/scripts/stamp-dist.mjs")); + run_check.addArg("--check"); + break :check &run_check.step; + } else null; + const web_assets = webAssetsIndex(b, web_dist_path, web_dist_check); const options = b.addOptions(); options.addOption(bool, "integration", integration); @@ -45,6 +65,40 @@ pub fn build(b: *std.Build) void { if (b.args) |args| run.addArgs(args); b.step("run", "Run nxdns").dependOn(&run.step); + // Zig collects tests only from the root module, so a file missing from + // src/tests.zig silently contributes no tests. The list stays hand-written + // (generating it from a staged copy would point diagnostics at cache + // paths); this makes it complete by construction instead. + checkTestImports(b); + + // A successful `zig build test` still prints `failed command: .../test + // ... --listen=-` as its last line. This is an upstream zig 0.16.0 + // build-runner labelling defect, not a failure here, and not something + // this build script can suppress without hiding real failures. + // + // Mechanism, verified against the 0.16.0 sources on 2026-08-07: + // - std/Build/Step/Run.zig:1540 sets `result_failed_command` for every + // spawn, unconditionally ("if an error occurs, it's caused by this + // command"). Nothing clears it when the child succeeds. + // - compiler/build_runner.zig:1381 prints a step's diagnostics whenever + // `result_stderr` is non-empty, explicitly "no matter the result". + // - compiler/build_runner.zig:1515, reached from there, emits the + // `failed command: ` line because `result_failed_command` is non-null. + // So any Run step that both succeeds and writes one byte to stderr gets + // the label. Our suite writes plenty: the tests that exercise the warning + // paths log through the real sink. + // + // Minimal reproducer, no mbedTLS and no C: one passing test whose body is + // `std.debug.print` plus `try expect(true)`, in a build.zig with nothing + // but `addTest` + `addRunArtifact`. It prints the label and reports + // "3/3 steps succeeded; 1/1 tests passed". Deleting the print removes the + // label. The test child does not crash and does not abort in teardown. + // (Running the cached test binary by hand with `--listen=-` does abort, + // but only because stdin is then closed and the IPC runner panics on + // `EndOfStream`; that is an artifact of the manual invocation.) + // + // No upstream issue matched a search of ziglang/zig for this behaviour; + // the reference is the 0.16.0 source lines above. See AGENTS.md. const tests = b.addTest(.{ .root_module = b.createModule(.{ .root_source_file = b.path("src/tests.zig"), @@ -109,6 +163,26 @@ pub fn build(b: *std.Build) void { }); test_step.dependOn(&b.addRunArtifact(blocklist_fuzz_tests).step); + // `src/web/http_util.zig` imports only std, so its fuzz module roots + // directly at the file — no aggregator needed (milestone-15 ruling 6c). + const http_util_mod = b.createModule(.{ + .root_source_file = b.path("src/web/http_util.zig"), + .target = target, + .optimize = optimize, + }); + const http_util_fuzz_mod = b.createModule(.{ + .root_source_file = b.path("tests/fuzz/http_util_fuzz.zig"), + .target = target, + .optimize = optimize, + }); + http_util_fuzz_mod.addImport("http_util", http_util_mod); + const http_util_fuzz_tests = b.addTest(.{ + .name = "http-util-fuzz", + .use_llvm = if (fuzz) true else null, + .root_module = http_util_fuzz_mod, + }); + test_step.dependOn(&b.addRunArtifact(http_util_fuzz_tests).step); + // The bench harness (milestone-12 ruling 1). The measured roots // (matcher.zig, dns_cache.zig, compiler.zig) share files in their relative // import closures (model.zig, types.zig, ...), and a file may belong to @@ -116,6 +190,10 @@ pub fn build(b: *std.Build) void { // into one executable. So one staged module: a copy of src/ plus a // generated aggregator root, imported by the bench as `core`. No sqlite, // no mbedTLS: the closure is pure Zig. + // + // The compiler fuzz target reuses the same staged tree (milestone-15 + // ruling 6b): `compiler.zig` imports `../dns/`, so a module rooted under + // `src/filter/` fails with ImportOutsideModulePath. const bench_stage = b.addWriteFiles(); _ = bench_stage.addCopyDirectory(b.path("src"), "src", .{}); const bench_core = bench_stage.add("bench_core.zig", @@ -133,6 +211,19 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, }); + const compiler_fuzz_mod = b.createModule(.{ + .root_source_file = b.path("tests/fuzz/compiler_fuzz.zig"), + .target = target, + .optimize = optimize, + }); + compiler_fuzz_mod.addImport("core", bench_core_mod); + const compiler_fuzz_tests = b.addTest(.{ + .name = "compiler-fuzz", + .use_llvm = if (fuzz) true else null, + .root_module = compiler_fuzz_mod, + }); + test_step.dependOn(&b.addRunArtifact(compiler_fuzz_tests).step); + const bench_mod = b.createModule(.{ .root_source_file = b.path("tools/bench.zig"), .target = target, @@ -192,6 +283,53 @@ pub fn build(b: *std.Build) void { } } +/// Milestone-15 ruling 4: every `*.zig` under `src/` must appear in +/// `src/tests.zig` as a line that trims to exactly `_ = @import("");`, +/// where `` is relative to `src/`. Whole-line equality, not a substring +/// search: a commented-out import trims to a line starting with `//` and does +/// not match, and `db.zig` cannot satisfy the requirement for `db2.zig`. +/// Duplicate lines are an error too — they hide a botched merge. No allowlist: +/// a file with no tests still gets imported, because the import is free and an +/// exception is the thing that lets a real gap through. +fn checkTestImports(b: *std.Build) void { + const gpa = b.allocator; + const io = b.graph.io; + const root = b.build_root.handle; + + const tests_src = root.readFileAlloc(io, "src/tests.zig", gpa, .limited(4 << 20)) catch |err| { + std.process.fatal("cannot read src/tests.zig: {t}", .{err}); + }; + + var src_dir = root.openDir(io, "src", .{ .iterate = true }) catch |err| { + std.process.fatal("cannot open src/: {t}", .{err}); + }; + defer src_dir.close(io); + + var walker = src_dir.walk(gpa) catch @panic("OOM"); + defer walker.deinit(); + + while (walker.next(io) catch |err| { + std.process.fatal("cannot walk src/: {t}", .{err}); + }) |entry| { + if (entry.kind != .file) continue; + if (!std.mem.endsWith(u8, entry.path, ".zig")) continue; + if (std.mem.eql(u8, entry.path, "tests.zig")) continue; + + const needle = b.fmt("_ = @import(\"{s}\");", .{entry.path}); + var matches: usize = 0; + var lines = std.mem.splitScalar(u8, tests_src, '\n'); + while (lines.next()) |line| { + if (std.mem.eql(u8, std.mem.trim(u8, line, " \t\r"), needle)) matches += 1; + } + if (matches == 0) { + std.process.fatal("src/tests.zig is missing `{s}`", .{needle}); + } + if (matches > 1) { + std.process.fatal("src/tests.zig repeats `{s}` {d} times", .{ needle, matches }); + } + } +} + fn addExecutable( b: *std.Build, target: std.Build.ResolvedTarget, @@ -226,8 +364,14 @@ fn addExecutable( /// tool because a Run step hashes only the resolved path string of a directory /// argument, not its contents; the staged copy lives at a content-hashed path, /// so editing an asset re-runs the tool instead of replaying a stale cache. -fn webAssetsIndex(b: *std.Build, dist: std.Build.LazyPath) std.Build.LazyPath { - const staged = b.addWriteFiles().addCopyDirectory(dist, ".", .{}); +fn webAssetsIndex( + b: *std.Build, + dist: std.Build.LazyPath, + freshness_check: ?*std.Build.Step, +) std.Build.LazyPath { + const stage = b.addWriteFiles(); + if (freshness_check) |check| stage.step.dependOn(check); + const staged = stage.addCopyDirectory(dist, ".", .{}); const tool = b.addExecutable(.{ .name = "gen_web_assets", diff --git a/specs/milestone-15.md b/specs/milestone-15.md index dbafda3..ac0665d 100644 --- a/specs/milestone-15.md +++ b/specs/milestone-15.md @@ -85,6 +85,23 @@ The session must attempt a root cause, in this order: Either way: no check is loosened, no output is filtered, and the spec is updated afterward to record which outcome held (per the spec-update rule). +**Outcome recorded (implementation): B — upstream, and this ruling's own +diagnosis was wrong.** The label is not an abort and has nothing to do with +mbedTLS. In Zig 0.16.0, `std/Build/Step/Run.zig:1540` sets +`result_failed_command` unconditionally on every spawn and nothing clears it +on success; `compiler/build_runner.zig:1381` prints a step's diagnostics +whenever the child wrote to stderr, "no matter the result", and `:1515` then +emits the `failed command:` label because the field is non-null. Any Run +step that succeeds while writing one byte to stderr gets the label; our +warning-path tests log through the real sink. Minimal reproducer: one +passing test containing a `std.debug.print` shows the label; the same test +without the print shows nothing. The old "exits 134 under `--listen=-`" +observation was a manual-invocation artifact: the IPC runner panics with +`EndOfStream` (test_runner.zig:88) reading a closed stdin when no build +runner sits on the other end. Documented in the `b.addTest` comment in +build.zig and in AGENTS.md ("Reading `zig build test` output"). No matching +upstream issue found; the 0.16.0 source lines are the reference. + ### 4. The test import list gets a completeness guard `src/tests.zig:3-119` is a hand-maintained `comptime` block of 115 @@ -182,10 +199,17 @@ arm alongside the existing compiler tests. `src/web/http_util.zig` imports only `std`, so the fuzz module roots at a new file `tests/fuzz/http_util_fuzz.zig` with `addImport("http_util", )` — no aggregator. Targets: `parsePath`, -`decodeInPlace` (both `PlusRule` values), `queryValue`. Invariants to assert, -both already stated in the file: split-before-decode (a decoded segment never -gains a `/`), and decode-only-shrinks (result length ≤ input length; result is -a prefix-aliased slice of the buffer). `dnsParam`/`decodeDnsValue` stay +`decodeInPlace` (both `PlusRule` values), `queryValue`. Invariants to assert: +split-before-decode and decode-only-shrinks (result length ≤ input length; +result is a prefix-aliased slice of the buffer). **Corrected during +implementation:** the original phrasing "a decoded segment never gains a `/`" +is false — `%2F` legitimately decodes to a literal `/` inside its segment +(http_util.zig's own tests assert it), and the fuzz target caught that on its +first run. The property that holds is a count: segmentation is decided by the +raw bytes, so each decoded segment pairs with its raw `/`-delimited chunk in +order, the segment counts match, and each segment is no longer than its +chunk. A decode-then-split implementation still fails this. +`dnsParam`/`decodeDnsValue` stay private in `doh_server.zig` and stay unfuzzed — recorded, out of scope. ### 7. The multi-read fetch path gets a real fixture route @@ -208,7 +232,11 @@ forces the fixture to flush in parts. - Prove it can fail: during development, reintroduce the `readSliceShort(self.transfer_buf)` aliasing pattern locally and confirm the new test dies where the old suite stayed green. Record the proof in the - session notes; do not commit the revert. + session notes; do not commit the revert. **Proof recorded + (implementation):** with the aliasing reintroduced, the new test failed + (`expected 1500, found 1238`) while all 1378 other tests stayed green. On + this body the bug silently dropped 262 domains rather than crashing — the + count assertion does the real work, not the crash. ### 8. The rotation failure paths get tests diff --git a/src/cli.zig b/src/cli.zig index 50932a4..b886bd8 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -73,6 +73,29 @@ pub const Command = union(enum) { help, }; +/// The argv spelling of a command paired with its tag. The two differ for +/// `export` and `import`, whose tags carry a trailing underscore because both +/// words are Zig keywords. +pub const CommandName = struct { name: []const u8, tag: std.meta.Tag(Command) }; + +/// The one list of subcommands. `parseArgs` matches the command word against +/// it, `docs_drift_test.zig` derives its reference-heading needles from it, and +/// a test below holds `usage_text` to it. +pub const command_names = [_]CommandName{ + .{ .name = "run", .tag = .run }, + .{ .name = "check", .tag = .check }, + .{ .name = "export", .tag = .export_ }, + .{ .name = "import", .tag = .import_ }, + .{ .name = "version", .tag = .version }, + .{ .name = "help", .tag = .help }, +}; + +comptime { + // A tag added to `Command` without an entry here fails the compile rather + // than silently dropping out of the parser and the doc guard. + std.debug.assert(command_names.len == @typeInfo(Command).@"union".fields.len); +} + pub const ParseError = error{ UnknownCommand, UnknownFlag, @@ -88,18 +111,30 @@ pub fn parseArgs(argv: []const []const u8) ParseError!Command { const command = argv[0]; const rest = argv[1..]; - if (eql(command, "version")) { - if (rest.len != 0) return error.TooManyArguments; - return .version; - } - if (eql(command, "help") or eql(command, "--help") or eql(command, "-h")) { + // The two flag spellings of `help` are not subcommands, so they are not in + // `command_names` and are matched before it. + if (eql(command, "--help") or eql(command, "-h")) { if (rest.len != 0) return error.TooManyArguments; return .help; } - if (eql(command, "run")) return .{ .run = try parseRunArgs(rest) }; - if (eql(command, "check")) return .{ .check = try parseCheckArgs(rest) }; - if (eql(command, "export")) return .{ .export_ = try parseExportArgs(rest) }; - if (eql(command, "import")) return .{ .import_ = try parseImportArgs(rest) }; + + for (command_names) |entry| { + if (!eql(command, entry.name)) continue; + switch (entry.tag) { + .run => return .{ .run = try parseRunArgs(rest) }, + .check => return .{ .check = try parseCheckArgs(rest) }, + .export_ => return .{ .export_ = try parseExportArgs(rest) }, + .import_ => return .{ .import_ = try parseImportArgs(rest) }, + .version => { + if (rest.len != 0) return error.TooManyArguments; + return .version; + }, + .help => { + if (rest.len != 0) return error.TooManyArguments; + return .help; + }, + } + } return error.UnknownCommand; } @@ -1076,6 +1111,19 @@ test "parseArgs rejects an extra positional argument" { try testing.expectError(error.TooManyArguments, parseArgs(&.{ "-h", "extra" })); } +test "usage_text lists every command in command_names" { + // The commands block indents each entry by two spaces, so a command that + // survives only as a word inside an option description does not count. + for (command_names) |entry| { + var needle_buf: [32]u8 = undefined; + const needle = try std.fmt.bufPrint(&needle_buf, "\n {s} ", .{entry.name}); + if (std.mem.indexOf(u8, usage_text, needle) == null) { + std.debug.print("command missing from usage_text: {s}\n", .{entry.name}); + return error.CommandMissingFromUsage; + } + } +} + test "usage writes non-empty text" { var out: Writer.Allocating = .init(testing.allocator); defer out.deinit(); diff --git a/src/docs_drift_test.zig b/src/docs_drift_test.zig index 87d3a57..fc5d9ca 100644 --- a/src/docs_drift_test.zig +++ b/src/docs_drift_test.zig @@ -5,6 +5,7 @@ const std = @import("std"); const docs = @import("docs_files"); +const cli = @import("cli.zig"); const routes = @import("web/routes.zig"); const model = @import("config/model.zig"); @@ -48,12 +49,11 @@ test "every settings key appears in docs/reference/configuration.md" { test "every cli subcommand has its own reference heading in docs/reference/cli.md" { const gpa = std.testing.allocator; - const subcommands = [_][]const u8{ "run", "check", "export", "import", "version", "help" }; - for (subcommands) |name| { + for (cli.command_names) |entry| { // Anchors on the reference-section heading ("## `import FILE`" starts // with "## `import"), so prose mentions elsewhere cannot mask a removed // command section. - const needle = try std.fmt.allocPrint(gpa, "## `{s}", .{name}); + const needle = try std.fmt.allocPrint(gpa, "## `{s}", .{entry.name}); defer gpa.free(needle); if (std.mem.indexOf(u8, docs.reference_cli_md, needle) == null) { std.debug.print("subcommand heading missing from docs/reference/cli.md: {s}\n", .{needle}); diff --git a/src/filter/compiler.zig b/src/filter/compiler.zig index 06b777f..070cc38 100644 --- a/src/filter/compiler.zig +++ b/src/filter/compiler.zig @@ -466,6 +466,32 @@ test "an over-long line is skipped when the reader buffer is large" { try testing.expectEqualStrings("a.example.com\nb.example.com\n", c.list()); } +test "an over-long final line with no newline ends the stream inside the discard" { + const gpa = testing.allocator; + var text: std.ArrayList(u8) = .empty; + defer text.deinit(gpa); + + try text.appendSlice(gpa, "a.example.com\n"); + try text.appendNTimes(gpa, 'x', 5_000); + + // A reader buffer smaller than the trailing line makes `takeDelimiter` + // report `error.StreamTooLong`, and the discard that follows then runs out + // of input because nothing terminates that line. That is the `EndOfStream` + // arm: the loop must count the line and stop, not treat the exhausted + // stream as a read failure. + var backing: std.Io.Reader = .fixed(text.items); + var buf: [max_line_len]u8 = undefined; + var limited = backing.limited(.unlimited, &buf); + + var c = try compileReader(gpa, &limited.interface, .domains); + defer c.deinit(); + + try testing.expectEqual(@as(u32, 1), c.result.counts.long_lines); + try testing.expectEqual(@as(u32, 1), c.result.counts.domains); + try testing.expectEqualStrings("a.example.com\n", c.list()); + try testing.expect(std.mem.indexOf(u8, c.list(), "xxxx") == null); +} + test "carriage returns are stripped" { var c = try compileText(testing.allocator, "b.example.com\r\na.example.com\r\n", .domains); defer c.deinit(); diff --git a/src/filter/filter_integration_test.zig b/src/filter/filter_integration_test.zig index 4a52a73..b8d4381 100644 --- a/src/filter/filter_integration_test.zig +++ b/src/filter/filter_integration_test.zig @@ -351,10 +351,37 @@ const Env = struct { // fixtures: the loopback http server // --------------------------------------------------------------------------- -const Route = enum(u8) { body, redirect, not_found, oversize }; +const Route = enum(u8) { body, redirect, not_found, oversize, chunked }; const redirect_path = "/redirected.txt"; +const chunked_domains = 1_500; + +/// The body the `chunked` route serves (milestone-15 ruling 7). Two thresholds +/// matter here and they are different: a body over `fetcher.min_transfer_buf` +/// (16 KiB) makes the fetcher's `pumpBody` loop iterate instead of finishing in +/// one read, and a body over the fixture connection's 8192-byte write buffer +/// makes the reply reach the wire in parts. 1500 lines of 31 bytes clears both +/// with room to spare. +fn chunkedFixtureBody(gpa: std.mem.Allocator) ![]u8 { + var out: std.Io.Writer.Allocating = .init(gpa); + errdefer out.deinit(); + for (0..chunked_domains) |i| { + try out.writer.print("0.0.0.0 chunk{d:0>5}.example.com\n", .{i}); + } + return out.toOwnedSlice(); +} + +/// `body` cut into three equal parts, deliberately not at line boundaries: a +/// domain name then straddles every part boundary, so a byte the transfer loses +/// or repeats there corrupts a name and moves the compiled count, instead of +/// being absorbed by a spare newline. +fn thirds(body: []const u8) [3][]const u8 { + const first = body.len / 3; + const second = 2 * (body.len / 3); + return .{ body[0..first], body[first..second], body[second..] }; +} + /// Past `fetcher.max_body_bytes`. The cap is a constant, so the only way to /// reach it in a test is a declared length: the fetcher refuses on the response /// head, before a byte of body streams. @@ -364,6 +391,11 @@ const HttpFixture = struct { server: net.Server, body: []const u8, route: std.atomic.Value(u8), + /// How many parts the `chunked` route has flushed. The test reads it to + /// prove the reply really left this server in pieces, because a `Writer` + /// reports a buffered part as written and would otherwise hide a fixture + /// that sent everything in one go. + flushed_parts: std.atomic.Value(u32), fn init(io: std.Io, body: []const u8) !HttpFixture { const local: net.IpAddress = try .parse("127.0.0.1", 0); @@ -371,6 +403,7 @@ const HttpFixture = struct { .server = try local.listen(io, .{ .reuse_address = true }), .body = body, .route = .init(@intFromEnum(Route.body)), + .flushed_parts = .init(0), }; } @@ -429,8 +462,27 @@ const HttpFixture = struct { .transfer_encoding = .none, .extra_headers = &.{.{ .name = "content-length", .value = oversize_length }}, }), + .chunked => try self.respondChunked(request), } } + + /// Streams the body in three flushed parts instead of one `respond` + /// (milestone-15 ruling 7). Every other arm sends ~130 bytes in a single + /// write, which the fetcher consumes in one read: the loop that 35f2324 + /// killed the process in was never driven end to end by this suite. + fn respondChunked(self: *HttpFixture, request: *std.http.Server.Request) !void { + var send_buf: [4096]u8 = undefined; + var stream = try request.respondStreaming(&send_buf, .{ + .respond_options = .{ .keep_alive = false }, + }); + for (thirds(self.body)) |part| { + try stream.writer.writeAll(part); + // Without this the parts sit in `send_buf` and leave as one write. + try stream.flush(); + _ = self.flushed_parts.fetchAdd(1, .release); + } + try stream.end(); + } }; // --------------------------------------------------------------------------- @@ -1478,3 +1530,64 @@ test "17: each blocking mode synthesizes the documented blocked reply" { try testing.expect(packet.findOptRecord(nx_packet) != null); } } + +// --------------------------------------------------------------------------- +// 18: the multi-read download path +// --------------------------------------------------------------------------- + +test "18: a body streamed in flushed parts survives the fetcher's multi-read pump" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + const env = try Env.create(gpa); + defer env.destroy(); + const io = env.io(); + + const body = try chunkedFixtureBody(gpa); + defer gpa.free(body); + try testing.expect(body.len >= 24 * 1024); + try testing.expect(body.len > fetcher.min_transfer_buf); + + var fixture = try HttpFixture.init(io, body); + defer fixture.deinit(io); + fixture.setRoute(.chunked); + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, HttpFixture.serve, .{ &fixture, io }); + + var url_buf: [64]u8 = undefined; + const url = try fixture.url(&url_buf); + const id = try seedSource(&env.database, url); + + try testing.expect(try refreshOnce(env, url)); + try env.mgr.reload(io); + + try testing.expect(fixture.flushed_parts.load(.acquire) >= 3); + + var rows = try listRows(&env.database); + defer rows.deinit(); + const row = try rows.byUrl(url); + try testing.expectEqual(@as(i64, chunked_domains), row.domain_count); + try testing.expectEqual(@as(i64, 0), row.wildcard_count); + try testing.expectEqual(manager.State.ok, (try env.status(id)).state); + + // Every name arrived intact, including the two that straddled a flush + // boundary: a byte lost or repeated at a boundary corrupts a name, which + // moves the compiled count away from the number of lines sent. + var dir = try env.blocklistDir(); + defer dir.close(io); + var bodies = try Bodies.read(gpa, io, dir, "1"); + defer bodies.deinit(gpa); + try testing.expectEqual( + @as(usize, chunked_domains), + std.mem.count(u8, manager.stripHeader(bodies.list), "\n"), + ); + + var buf: [64]u8 = undefined; + for ([_]usize{ 0, chunked_domains / 3, 2 * chunked_domains / 3, chunked_domains - 1 }) |i| { + const domain = try std.fmt.bufPrint(&buf, "chunk{d:0>5}.example.com", .{i}); + const decision, _ = try env.evaluate(domain); + try testing.expect(decision.blocked); + try testing.expectEqual(matcher.Reason.blocklist_domain, decision.reason); + } +} diff --git a/src/platform/logging.zig b/src/platform/logging.zig index febb75f..c73c406 100644 --- a/src/platform/logging.zig +++ b/src/platform/logging.zig @@ -26,6 +26,7 @@ //! and a log line emitted from the sink would recurse. const std = @import("std"); +const builtin = @import("builtin"); const model = @import("../config/model.zig"); /// Two upstream failures with the same key inside this window produce one line. @@ -585,9 +586,25 @@ fn rotateStepsLocked() RotateError!void { try renameLocked(dir, p, first); } +const RotateFault = enum { none, fail_delete, fail_rename }; + +/// The two rotation steps fail only when the filesystem does, which no unit +/// test can arrange on demand, so the failure paths are driven through this +/// seam instead. The storage exists in a test build only, and +/// `rotateFaultTripped` reduces to `false` everywhere else. +const rotate_fault_seam = if (builtin.is_test) struct { + var fault: RotateFault = .none; +} else struct {}; + +fn rotateFaultTripped(comptime which: RotateFault) bool { + if (!builtin.is_test) return false; + return rotate_fault_seam.fault == which; +} + /// A generation that does not exist yet is not a failure: the first rotations /// of a fresh log directory find nothing to delete. fn deleteLocked(dir: std.Io.Dir, p: []const u8) RotateError!void { + if (rotateFaultTripped(.fail_delete)) return error.RotateFailed; dir.deleteFile(state.io, p) catch |err| switch (err) { error.FileNotFound => {}, else => return error.RotateFailed, @@ -595,6 +612,7 @@ fn deleteLocked(dir: std.Io.Dir, p: []const u8) RotateError!void { } fn renameLocked(dir: std.Io.Dir, from: []const u8, to: []const u8) RotateError!void { + if (rotateFaultTripped(.fail_rename)) return error.RotateFailed; dir.rename(from, dir, to, state.io) catch |err| switch (err) { error.FileNotFound => {}, else => return error.RotateFailed, @@ -605,10 +623,9 @@ fn renameLocked(dir: std.Io.Dir, from: []const u8, to: []const u8) RotateError!v // Tests // // The pure pieces only. `logFn` is never exercised: installing a sink under the -// test runner would eat the harness's own output. File behaviour is S8's, and -// the rotation failure paths (`rotateLocked` returning false, the pending -// rotation that keeps the oversized file closed) stay uncovered: they need a -// filesystem that fails a delete or a rename on demand. +// test runner would eat the harness's own output. File behaviour is S8's. The +// rotation failure paths need a filesystem that fails a delete or a rename on +// demand, which `rotate_fault_seam` supplies. // --------------------------------------------------------------------------- const testing = std.testing; @@ -908,6 +925,72 @@ test "a failed open counts exactly one sink error" { try testing.expectEqual(@as(u64, 0), state.stats.lines_written); } +/// The shared body of the two rotation-failure tests: they differ only in which +/// step is made to fail and in how many `max_files` it takes to reach it. +/// +/// `state.io` is normally installed by `install`, which no test calls, so a real +/// one is put in place for the duration: `rotateLocked` swaps cancel protection +/// on it before the first step runs, and the `fail_rename` case deletes the +/// oldest generation for real before it reaches the rename. That delete is why +/// the path points into a fresh tmp directory rather than at any real log. +fn expectRotationFailureCounted(fault: RotateFault, max_files: u8) !void { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [128]u8 = undefined; + const p = try std.fmt.bufPrint(&path_buf, ".zig-cache/tmp/{s}/nxdns.log", .{tmp.sub_path}); + + var stderr_buf: [64]u8 = undefined; + _ = std.debug.lockStderr(&stderr_buf); + const saved_stats = state.stats; + const saved_path_len = state.path_len; + const saved_max_files = state.max_files; + const saved_file = state.file; + const saved_pending = state.rotate_pending; + const saved_io = state.io; + defer { + rotate_fault_seam.fault = .none; + state.stats = saved_stats; + state.path_len = saved_path_len; + state.max_files = saved_max_files; + state.file = saved_file; + state.rotate_pending = saved_pending; + state.io = saved_io; + std.debug.unlockStderr(); + } + + state.stats = .{}; + state.io = threaded.io(); + state.path_len = p.len; + @memcpy(state.path_buf[0..p.len], p); + state.max_files = max_files; + state.file = null; + state.rotate_pending = true; + rotate_fault_seam.fault = fault; + + try testing.expect(!prepareFileLocked(64)); + try testing.expectEqual(@as(u64, 1), state.stats.sink_errors); + try testing.expectEqual(@as(u64, 0), state.stats.rotations); + // The oversized file stays closed and the rotation stays owed, so the next + // line retries the rotation instead of appending past `max_bytes`. + try testing.expect(state.rotate_pending); + try testing.expectEqual(@as(?std.Io.File, null), state.file); +} + +test "a failed rotation delete counts exactly one sink error" { + // `max_files` below 2 keeps no generations, so the whole rotation is the one + // delete of the live path. + try expectRotationFailureCounted(.fail_delete, 1); +} + +test "a failed rotation rename counts exactly one sink error" { + // `max_files` of 2 keeps generation 1, so the steps are one delete of the + // oldest generation followed by the rename of the live path onto it. + try expectRotationFailureCounted(.fail_rename, 2); +} + test "rotatedName appends the generation" { var buf: [max_rotated_path_bytes]u8 = undefined; try testing.expectEqualStrings( diff --git a/src/tests.zig b/src/tests.zig index 3f614a8..42975a5 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -12,6 +12,7 @@ comptime { _ = @import("dns/record.zig"); _ = @import("dns/edns.zig"); _ = @import("dns/packet.zig"); + _ = @import("dns/dns.zig"); _ = @import("platform/address.zig"); _ = @import("platform/tls_client.zig"); _ = @import("platform/tls_client_integration_test.zig"); diff --git a/tests/fuzz/compiler_fuzz.zig b/tests/fuzz/compiler_fuzz.zig new file mode 100644 index 0000000..c202b98 --- /dev/null +++ b/tests/fuzz/compiler_fuzz.zig @@ -0,0 +1,184 @@ +//! Fuzz target for the blocklist compiler's streaming line loop +//! (`src/filter/compiler.zig`). +//! +//! `blocklist_fuzz.zig` covers the line parsers; this file covers the loop that +//! drives them. That loop is where the reader and the parsers meet, and it holds +//! three arms no parser target can reach: `takeDelimiter` returning a line, +//! `error.StreamTooLong` followed by `discardDelimiterInclusive`, and that +//! discard hitting end of stream on a final over-long line with no newline. +//! +//! The contract: any byte string is a legal blocklist, so `compile` may classify +//! it however it likes but must return — never panic, never loop forever, never +//! read out of bounds. Where it returns the target then checks what the manager +//! is entitled to rely on: +//! +//! - every per-line counter is bounded by the number of lines in the input, +//! and every per-candidate counter by its length, so no line is counted +//! twice and the discard arm cannot re-read bytes it already consumed; +//! - the written counts never exceed `max_domains`; +//! - a compiled body is a pure function of (bytes, format): the same input +//! compiled twice gives the same counts and the same checksum. +//! +//! `compiler.zig` imports `../dns/`, so a module rooted under `src/filter/` +//! fails with `ImportOutsideModulePath`. The root here is the staged-copy +//! aggregator `build.zig` already builds for the bench harness, imported as +//! `core`. +//! +//! Runner semantics: under a plain `zig build test` the target runs once per +//! corpus entry plus once on empty input, which makes the corpus a regression +//! suite. `zig build test --fuzz=` gives it `n` generated inputs. + +const std = @import("std"); +const core = @import("core"); + +const compiler = core.compiler; +const Smith = std.testing.Smith; + +/// The aggregator exposes `compiler`, and `compiler.zig` keeps its own +/// `parsers` import private, so the format enum is read off the signature of the +/// function under test rather than imported. That also keeps the target honest +/// if a format is ever added: `formats` grows with the enum. +const Format = @typeInfo(@TypeOf(compiler.compile)).@"fn".params[2].type.?; +const formats = std.enums.values(Format); + +/// Comfortably past `compiler.max_line_len`, so a single generated input can +/// hold an over-long line and the lines around it. +const max_input = 16384; + +/// The upper bound on the reader buffer the target hands `compile`. A buffer +/// under `max_line_len` reports an over-long line as `error.StreamTooLong` and +/// takes the discard arm; a buffer over it reports the line whole and takes the +/// length check at compiler.zig:84. Both are reachable inside this range. +const max_reader_buf = 8192; + +const fuzz_options: std.testing.FuzzInputOptions = .{ .corpus = &corpus }; + +test "fuzz compiler.compile" { + try std.testing.fuzz({}, compileTarget, fuzz_options); +} + +fn compileTarget(_: void, smith: *Smith) anyerror!void { + var buf: [max_input]u8 = undefined; + const bytes = buf[0..smith.slice(&buf)]; + const format = formats[smith.index(formats.len)]; + const reader_buf_len = smith.valueRangeAtMost(u32, 64, max_reader_buf); + + const first = (try compileOnce(bytes, format, reader_buf_len)) orelse return; + + try expectConsistent(first.counts, bytes); + + // Determinism is the property the whole design rests on (compiler.zig's + // header comment): two runs over the same bytes agree byte for byte. + const second = (try compileOnce(bytes, format, reader_buf_len)) orelse + return error.TestSecondRunFailed; + try std.testing.expectEqual(first.counts, second.counts); + try std.testing.expectEqualSlices(u8, &first.checksum, &second.checksum); +} + +/// One compile into discarding writers, or null when the compiler rejected the +/// input. Every member of `compiler.Error` is a legitimate rejection: an +/// allocator that ran out, a list past `max_domains`, and the two stream +/// failures the fixed reader and the discarding writers cannot actually raise. +fn compileOnce( + bytes: []const u8, + format: Format, + reader_buf_len: u32, +) anyerror!?compiler.Result { + var backing: std.Io.Reader = .fixed(bytes); + var reader_buf: [max_reader_buf]u8 = undefined; + var limited = backing.limited(.unlimited, reader_buf[0..reader_buf_len]); + + var list_sink: [0]u8 = .{}; + var list_w: std.Io.Writer.Discarding = .init(&list_sink); + var wild_sink: [0]u8 = .{}; + var wild_w: std.Io.Writer.Discarding = .init(&wild_sink); + + return compiler.compile( + std.testing.allocator, + &limited.interface, + format, + &list_w.writer, + &wild_w.writer, + ) catch |err| switch (err) { + error.OutOfMemory, + error.TooManyDomains, + error.ReadFailed, + error.WriteFailed, + => null, + }; +} + +/// The counters have to add up against the input that produced them. +fn expectConsistent(counts: compiler.Counts, bytes: []const u8) !void { + // Every line the loop classifies ends at a newline or at the end of the + // input, so no per-line counter can exceed the number of lines. + const lines = std.mem.count(u8, bytes, "\n") + 1; + try std.testing.expect(counts.long_lines <= lines); + try std.testing.expect(counts.skipped_regex <= lines); + try std.testing.expect(counts.skipped_unsupported <= lines); + + // A candidate is a non-empty whitespace-separated field or a whole wildcard + // line, so every candidate consumes at least one byte of the input, and a + // written name is a candidate that survived. + const candidates = @as(u64, counts.domains) + counts.wildcards + + counts.duplicates + counts.invalid; + try std.testing.expect(candidates <= bytes.len + 1); + + try std.testing.expect(counts.domains <= compiler.max_domains); + try std.testing.expect(counts.wildcards <= compiler.max_domains); +} + +// --------------------------------------------------------------------------- +// corpus +// --------------------------------------------------------------------------- +// +// `Smith` does not consume a corpus entry as raw input. It reads a byte stream +// in which a slice is a little-endian `u32` length followed by that many bytes, +// so every entry below is length-prefixed. An entry that carries only the slice +// leaves the format index and the reader-buffer length at the low end of their +// ranges, which is the 64-byte buffer that makes `error.StreamTooLong` the +// common case. + +/// Past `compiler.max_line_len`, so the discard arm at compiler.zig:72 replays +/// from the corpus rather than waiting on a discovery. +const long_line = "a" ** 5000 ++ ".example.com"; + +/// The same line as the last line of the input, with nothing behind it: the +/// discard then hits end of stream, which is the `break` at compiler.zig:73. +const long_line_unterminated = "0.0.0.0 kept.example.com\n" ++ long_line; +const long_line_terminated = long_line_unterminated ++ "\n0.0.0.0 after.example.com\n"; + +/// Encodes `bytes` as a single `Smith.slice` value. +fn sliceInput(comptime bytes: []const u8) *const [4 + bytes.len]u8 { + return &struct { + const value: [4 + bytes.len]u8 = blk: { + var buf: [4 + bytes.len]u8 = undefined; + std.mem.writeInt(u32, buf[0..4], @intCast(bytes.len), .little); + buf[4..].* = bytes[0..bytes.len].*; + break :blk buf; + }; + }.value; +} + +const corpus = [_][]const u8{ + sliceInput(long_line_unterminated), + sliceInput(long_line_terminated), + sliceInput("# a hosts list\n0.0.0.0 ads.example.com # advertising\n"), + sliceInput("||ads.example.net^\n@@||allow.example.net^\n/re[0-9]+/\n"), + sliceInput("*.wild.example.org\nlocalhost\nAdS.Example.COM.\n"), +}; + +test "the unterminated corpus entry ends on an over-long line" { + try std.testing.expect(!std.mem.endsWith(u8, long_line_unterminated, "\n")); + const last = std.mem.findScalarLast(u8, long_line_unterminated, '\n').? + 1; + try std.testing.expect(long_line_unterminated.len - last > compiler.max_line_len); +} + +test "a corpus entry carries its own length" { + const encoded = sliceInput(long_line); + try std.testing.expectEqual( + @as(u32, long_line.len), + std.mem.readInt(u32, encoded[0..4], .little), + ); + try std.testing.expectEqualSlices(u8, long_line, encoded[4..]); +} diff --git a/tests/fuzz/dns_fuzz.zig b/tests/fuzz/dns_fuzz.zig index af66784..958af32 100644 --- a/tests/fuzz/dns_fuzz.zig +++ b/tests/fuzz/dns_fuzz.zig @@ -10,7 +10,10 @@ //! - a `Name` that `parse` accepted ends inside the packet and survives a //! round trip through presentation form; //! - a buffer that `decrementTtls` aged still parses, and no record it aged -//! holds a TTL below the minimum it reported. +//! holds a TTL below the minimum it reported; +//! - a query that `stripEcs` rewrote still parses, still carries a valid OPT +//! record, no longer carries an ECS option, and kept all four of its +//! section counts. //! //! The targets stay inside the documented safe entry points. `setId` is called //! only on a buffer long enough to hold a header, because it asserts that @@ -54,6 +57,10 @@ test "fuzz packet.decrementTtls" { try std.testing.fuzz({}, ttlTarget, fuzz_options); } +test "fuzz edns.stripEcs" { + try std.testing.fuzz({}, stripEcsTarget, fuzz_options); +} + fn parseTarget(_: void, smith: *Smith) anyerror!void { var buf: [max_input]u8 = undefined; const bytes = buf[0..smith.slice(&buf)]; @@ -136,6 +143,55 @@ fn ttlTarget(_: void, smith: *Smith) anyerror!void { } } +/// `stripEcs` is the only attacker-facing entry point that rewrites a packet, so +/// it is the only one where a finding can be a wrong output rather than a crash. +/// +/// The input is derived exactly as `parseTarget` derives it, because `stripEcs` +/// asserts its preconditions rather than returning an error: `query` must be the +/// same bytes `pkt` was parsed from, and `out` must not overlap them. `out` is a +/// separate stack buffer for that reason, and tripping either assertion from a +/// hand-built argument would report a fault no packet can cause. +fn stripEcsTarget(_: void, smith: *Smith) anyerror!void { + var buf: [max_input]u8 = undefined; + const bytes = buf[0..smith.slice(&buf)]; + + const p = packet.parse(bytes) catch return; + const opt_record = packet.findOptRecord(p) orelse return; + const opt = edns.parseOpt(bytes, opt_record) catch return; + + // Removing an option only ever shortens the query, so a buffer the size of + // the input always holds the rewrite. + var out: [max_input]u8 = undefined; + const result = edns.stripEcs(bytes, p, opt, &out) catch return; + const rewritten = switch (result) { + .unchanged => return, + .rewritten => |message| message, + }; + + try std.testing.expect(rewritten.len <= bytes.len); + + const stripped = try packet.parse(rewritten); + try std.testing.expectEqual(p.header.id, stripped.header.id); + try std.testing.expectEqual(p.header.qdcount, stripped.header.qdcount); + try std.testing.expectEqual(p.header.ancount, stripped.header.ancount); + try std.testing.expectEqual(p.header.nscount, stripped.header.nscount); + try std.testing.expectEqual(p.header.arcount, stripped.header.arcount); + + const stripped_record = packet.findOptRecord(stripped) orelse + return error.TestOptRecordLost; + const stripped_opt = try edns.parseOpt(rewritten, stripped_record); + try std.testing.expectEqual(opt.udp_payload_size, stripped_opt.udp_payload_size); + try std.testing.expectEqual(opt.do_bit, stripped_opt.do_bit); + + var options = edns.options(rewritten, stripped_opt); + while (try options.next()) |option| { + try std.testing.expect(option.code != edns.ecs_option_code); + } + try std.testing.expect( + (try edns.findOption(rewritten, stripped_opt, edns.ecs_option_code)) == null, + ); +} + /// Runs every typed RDATA accessor over a record. Each one rejects a record of /// the wrong type or a truncated RDATA, so only a panic is a finding here. fn sweepRdata(bytes: []const u8, rec: record.Record) void { diff --git a/tests/fuzz/http_util_fuzz.zig b/tests/fuzz/http_util_fuzz.zig new file mode 100644 index 0000000..8bb65b6 --- /dev/null +++ b/tests/fuzz/http_util_fuzz.zig @@ -0,0 +1,226 @@ +//! Fuzz targets for the HTTP request parsers (`src/web/http_util.zig`). +//! +//! These read the third untrusted-byte family nxdns accepts: the request line +//! and the query string a browser — or anything else on the LAN — sends. Every +//! target holds the same contract as the DNS and blocklist targets: rejecting +//! bytes with an error is correct, panicking or reading out of bounds is not. +//! +//! Two invariants the file itself states are what a success has to satisfy: +//! +//! - split before decode. `parsePath` cuts segments at `/` and only then +//! percent-decodes each one, so a `%2F` inside a segment stays inside it. +//! The decoded segment does hold a literal `/` — `http_util.zig`'s own test +//! asserts that — but it is one segment, not two. So the property is a +//! count: the segmentation is the one the *raw* bytes describe, and a +//! decoder that ran before the split would hand back more segments than +//! the raw bytes have. +//! - decode only shrinks. `decodeInPlace` writes behind its own read cursor, +//! so the result is never longer than its input and always aliases the front +//! of the same buffer. If either ever stopped holding, the write cursor +//! would have passed the read cursor and the decoder would be reading bytes +//! it had already overwritten. +//! +//! `http_util.zig` imports only `std`, so the module here roots straight at it; +//! no staged aggregator is involved. +//! +//! Runner semantics: under a plain `zig build test` each target runs once per +//! corpus entry plus once on empty input, which makes the corpus a regression +//! suite. `zig build test --fuzz=` gives each target `n` generated inputs. + +const std = @import("std"); +const http_util = @import("http_util"); + +const Smith = std.testing.Smith; + +/// A target longer than this is a 414 before it reaches any parser +/// (`http_util.max_target_len`), so a longer input buys no new code paths. +const max_input = 4096; + +/// `Smith` entity ids. The query target needs stable, distinct ids for its query +/// string and its key; the single-slice targets take the first. +const primary_hash: u32 = 1; +const secondary_hash: u32 = 2; + +const fuzz_options: std.testing.FuzzInputOptions = .{ .corpus = &corpus }; + +test "fuzz http_util.parsePath" { + try std.testing.fuzz({}, pathTarget, fuzz_options); +} + +test "fuzz http_util.decodeInPlace" { + try std.testing.fuzz({}, decodeTarget, fuzz_options); +} + +test "fuzz http_util.queryValue" { + try std.testing.fuzz({}, queryTarget, fuzz_options); +} + +fn pathTarget(_: void, smith: *Smith) anyerror!void { + var buf: [max_input]u8 = undefined; + const len = smith.sliceWithHash(&buf, primary_hash); + const input = buf[0..len]; + + // `parsePath` decodes in place, so the raw request line is kept: it is what + // the segmentation has to agree with. + var raw_buf: [max_input]u8 = undefined; + @memcpy(raw_buf[0..len], input); + const raw = raw_buf[0..len]; + + const path = http_util.parsePath(input) catch return; + + try std.testing.expect(path.len <= http_util.max_path_segments); + + // Every non-empty run between two `/` in the raw bytes is one segment, in + // order. A non-empty run always decodes to at least one byte, so the two + // sequences are the same length and pair up. + var chunks = std.mem.splitScalar(u8, raw, '/'); + var i: usize = 0; + while (chunks.next()) |chunk| { + if (chunk.len == 0) continue; + try std.testing.expect(i < path.len); + const segment = path.segments()[i]; + // Decode only shrinks, per segment. + try std.testing.expect(segment.len <= chunk.len); + try expectAliases(segment, input); + i += 1; + } + try std.testing.expectEqual(path.len, i); +} + +fn decodeTarget(_: void, smith: *Smith) anyerror!void { + var buf: [max_input]u8 = undefined; + const len = smith.sliceWithHash(&buf, primary_hash); + const input = buf[0..len]; + + // Both rules are fed the same bytes: `+` is a space in a query string and an + // ordinary character in a path, and neither reading may change the bound. + for ([_]http_util.PlusRule{ .literal_plus, .plus_is_space }) |rule| { + var scratch: [max_input]u8 = undefined; + @memcpy(scratch[0..len], input); + const decoded = http_util.decodeInPlace(scratch[0..len], rule) catch continue; + try std.testing.expect(decoded.len <= len); + try expectPrefixOf(decoded, scratch[0..len]); + } +} + +fn queryTarget(_: void, smith: *Smith) anyerror!void { + var query_buf: [max_input]u8 = undefined; + var key_buf: [max_input]u8 = undefined; + const query = query_buf[0..smith.sliceWithHash(&query_buf, primary_hash)]; + const key = key_buf[0..smith.sliceWithHash(&key_buf, secondary_hash)]; + + var out: [http_util.max_query_value_len]u8 = undefined; + const value = (http_util.queryValue(query, key, &out) catch return) orelse return; + + // The decoded value lives at the front of the caller's buffer, which is what + // lets a handler keep it for the length of the request. + try expectPrefixOf(value, &out); + + // Decode only shrinks, measured against the raw pair the walker found. + var it = http_util.queryPairs(query); + while (it.next()) |pair| { + if (!std.mem.eql(u8, pair.key, key)) continue; + try std.testing.expect(value.len <= pair.value.len); + return; + } + return error.TestValueWithoutPair; +} + +/// A result the parsers hand back is a window into the caller's buffer, never a +/// copy and never a pointer into a temporary. +fn expectAliases(result: []const u8, buffer: []const u8) !void { + if (result.len == 0) return; + const start = @intFromPtr(result.ptr); + const base = @intFromPtr(buffer.ptr); + try std.testing.expect(start >= base); + try std.testing.expect(start + result.len <= base + buffer.len); +} + +/// The stronger form the in-place decoders owe: the result starts where the +/// input started. A decoder that writes ahead of its read cursor cannot satisfy +/// this and a shorter-but-moved slice would slip past `expectAliases`. +fn expectPrefixOf(result: []const u8, buffer: []const u8) !void { + try std.testing.expect(result.len <= buffer.len); + if (buffer.len == 0) return; + try std.testing.expectEqual(@intFromPtr(buffer.ptr), @intFromPtr(result.ptr)); +} + +// --------------------------------------------------------------------------- +// corpus +// --------------------------------------------------------------------------- +// +// `Smith` does not consume a corpus entry as raw parser input. It reads a byte +// stream in which a slice is a little-endian `u32` length followed by that many +// bytes, so every entry below is length-prefixed. The three targets share one +// corpus: each starts with a slice, and the query target reads a second one that +// falls back to empty when an entry carries only the first. + +/// A route the router actually matches, with an id capture. +const api_path = "/api/groups/12"; + +/// The encoded slash: the byte pattern the split-before-decode rule exists for. +/// It decodes to `a/b/../etc` inside one segment and must stay one segment. +const encoded_slash = "/api/rules/a%2Fb%2F..%2Fetc"; + +/// One segment past `max_path_segments`. +const deep_path = "/1/2/3/4/5/6/7/8/9"; + +/// The three malformed escapes `decodeInPlace` refuses rather than passes +/// through, and a plus that means different things under the two rules. +const bad_escapes = "/%2/%/%zz/a+b"; + +/// Encodes `bytes` as a single `Smith.slice` value. +fn sliceInput(comptime bytes: []const u8) *const [4 + bytes.len]u8 { + return &struct { + const value: [4 + bytes.len]u8 = blk: { + var buf: [4 + bytes.len]u8 = undefined; + std.mem.writeInt(u32, buf[0..4], @intCast(bytes.len), .little); + buf[4..].* = bytes[0..bytes.len].*; + break :blk buf; + }; + }.value; +} + +/// Encodes two `Smith.slice` values back to back, which is what the query target +/// reads as its query string and its key. +fn pairInput(comptime a: []const u8, comptime b: []const u8) *const [8 + a.len + b.len]u8 { + return &struct { + const value: [8 + a.len + b.len]u8 = blk: { + var buf: [8 + a.len + b.len]u8 = undefined; + buf[0 .. 4 + a.len].* = sliceInput(a).*; + buf[4 + a.len ..].* = sliceInput(b).*; + break :blk buf; + }; + }.value; +} + +const corpus = [_][]const u8{ + sliceInput(api_path), + sliceInput(encoded_slash), + sliceInput(deep_path), + sliceInput(bad_escapes), + sliceInput("/api//groups/"), + // The query shapes the API defines, each with the key that reads it. + pairInput("domain=a+b&limit=250", "domain"), + pairInput("domain=%61%2Fb&blocked=1", "domain"), + pairInput("a=1&&b&c=", "c"), + pairInput("domain=" ++ "x" ** 1024, "domain"), + pairInput("domain=%zz", "domain"), +}; + +test "a corpus entry carries its own length" { + const encoded = sliceInput(api_path); + try std.testing.expectEqual( + @as(u32, api_path.len), + std.mem.readInt(u32, encoded[0..4], .little), + ); + try std.testing.expectEqualSlices(u8, api_path, encoded[4..]); +} + +test "a paired corpus entry carries both lengths" { + const encoded = pairInput("a=1", "a"); + try std.testing.expectEqual(@as(u32, 3), std.mem.readInt(u32, encoded[0..4], .little)); + try std.testing.expectEqualSlices(u8, "a=1", encoded[4..7]); + try std.testing.expectEqual(@as(u32, 1), std.mem.readInt(u32, encoded[7..11], .little)); + try std.testing.expectEqualSlices(u8, "a", encoded[11..]); +} diff --git a/web/package.json b/web/package.json index ab7e0b6..17185f8 100644 --- a/web/package.json +++ b/web/package.json @@ -8,7 +8,7 @@ }, "scripts": { "dev": "vite", - "build": "vite build", + "build": "vite build && node scripts/stamp-dist.mjs", "typecheck": "tsc -b", "lint": "oxlint src vite.config.ts", "format": "prettier --write .", diff --git a/web/scripts/stamp-dist.mjs b/web/scripts/stamp-dist.mjs new file mode 100644 index 0000000..783a3ca --- /dev/null +++ b/web/scripts/stamp-dist.mjs @@ -0,0 +1,99 @@ +#!/usr/bin/env node +// Freshness stamp for web/dist (milestone-15 ruling 5). A stale dist has +// already shipped a crashing settings page once. Write mode runs from web/ as +// part of `npm run build`; check mode runs from the repository root as a +// build.zig system command. Every path resolves from this file's own location +// so both working directories hash the same set. + +import { createHash } from "node:crypto"; +import { readdirSync, readFileSync, writeFileSync, statSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const webRoot = dirname(dirname(fileURLToPath(import.meta.url))); +const distDir = join(webRoot, "dist"); +const stampFile = join(distDir, ".src-hash"); +const stampRelative = "web/dist/.src-hash"; + +const inputDirs = ["src", "public"]; +const inputFiles = [ + "index.html", + "package.json", + "package-lock.json", + "vite.config.ts", + "tsconfig.json", + "tsconfig.app.json", + "tsconfig.node.json", +]; + +const staleMessage = "web/dist is stale: rebuild the frontend (npm run build)"; + +function fail(message) { + process.stderr.write(`${message}\n`); + process.exit(1); +} + +function walk(relativeDir) { + const absolute = join(webRoot, relativeDir); + let entries; + try { + entries = readdirSync(absolute, { withFileTypes: true }); + } catch (err) { + fail(`stamp-dist: cannot read web/${relativeDir}: ${err.message}`); + } + const found = []; + for (const entry of entries) { + const child = `${relativeDir}/${entry.name}`; + if (entry.isDirectory()) { + found.push(...walk(child)); + } else if (entry.isFile()) { + found.push(child); + } + } + return found; +} + +function inputSet() { + const paths = [...inputFiles, ...inputDirs.flatMap(walk)]; + for (const path of inputFiles) { + try { + if (!statSync(join(webRoot, path)).isFile()) fail(`stamp-dist: web/${path} is not a file`); + } catch (err) { + fail(`stamp-dist: cannot stat web/${path}: ${err.message}`); + } + } + // Sorted by path so the digest does not depend on directory order. + return paths.sort(); +} + +function digest() { + const hash = createHash("sha256"); + for (const path of inputSet()) { + hash.update(path); + hash.update("\0"); + hash.update(readFileSync(join(webRoot, path))); + hash.update("\0"); + } + return hash.digest("hex"); +} + +const check = process.argv.includes("--check"); +const computed = digest(); + +if (check) { + let recorded; + try { + recorded = readFileSync(stampFile, "utf8").trim(); + } catch { + fail(staleMessage); + } + if (recorded !== computed) fail(staleMessage); + process.exit(0); +} + +try { + writeFileSync(stampFile, `${computed}\n`); +} catch (err) { + fail(`stamp-dist: cannot write ${stampRelative}: ${err.message}`); +} +process.stdout.write(`${stampRelative} ${computed}\n`);