filter: separate the compiled bodies in the checksum, a name moving between them was invisible
This commit is contained in:
@@ -98,7 +98,7 @@ Two SQLite files with opposite write profiles, isolated from each other:
|
||||
|
||||
Blocklist domains are **not** stored in SQLite — they are a cache of re-downloadable remote artifacts, not config or state:
|
||||
|
||||
- Each source compiles to `/var/lib/nxdns/blocklists/<source_id>.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `<source_id>.wild`, ABP exceptions (`@@||name^`) to `<source_id>.allow`; regex lines and browser-syntax lines nxdns cannot translate into a DNS decision are counted + skipped, both counts in metadata → UI. The checksum covers the three bodies in that order, so a source with no exceptions keeps the digest it had when only two existed and no upgrade forces a refetch.
|
||||
- Each source compiles to `/var/lib/nxdns/blocklists/<source_id>.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `<source_id>.wild`, ABP exceptions (`@@||name^`) to `<source_id>.allow`; regex lines and browser-syntax lines nxdns cannot translate into a DNS decision are counted + skipped, both counts in metadata → UI. The checksum covers the three bodies in that order, each followed by a separator byte so that moving a name between bodies — an upstream switching `a.example` to `*.a.example` — changes the digest and forces a republish.
|
||||
- `config.db` keeps source **metadata only** (`blocklist_sources`).
|
||||
- Startup + post-update: parse files into the immutable in-memory matcher (RCU swap, §9.5).
|
||||
- Corruption recovery is per-file: checksum mismatch → re-download one list.
|
||||
|
||||
+86
-2
@@ -118,9 +118,13 @@ the only thing a restart reads — rehydration never reparses headers.
|
||||
Both skip counters take `compiled.result.counts` on this path:
|
||||
`.skipped_regex_count = compiled.result.counts.skipped_regex` and
|
||||
`.skipped_unsupported_count = compiled.result.counts.skipped_unsupported`.
|
||||
`domain_count`, `wildcard_count` and `exception_count` keep reading from
|
||||
~~`domain_count`, `wildcard_count` and `exception_count` keep reading from
|
||||
`row` — they count written entries, so an unchanged checksum does mean an
|
||||
unchanged value for them.
|
||||
unchanged value for them.~~ **Corrected post-commit:** that claim was false.
|
||||
The unframed digest could not distinguish an entry in `.list` from the same
|
||||
entry in `.wild`, so an unchanged checksum did not vouch for the entry
|
||||
counts either. The addendum below frames the hash and makes all five stat
|
||||
fields read fresh on this path.
|
||||
|
||||
This needs a test the author has watched fail with the fix reverted: two
|
||||
compiles of bodies whose written entries are identical but whose skipped
|
||||
@@ -495,3 +499,83 @@ New files: none. Deleted surface: none.
|
||||
- No new columns beyond the one. See ruling 7 for what that leaves open and
|
||||
why — the reason is a scope decision, not a claim that the other counters
|
||||
are already visible.
|
||||
|
||||
## Addendum (post-`1bce81e`): the body checksum is framed
|
||||
|
||||
A filtering correctness bug found by the implementation review, predating this
|
||||
milestone. Fixed as a follow-up commit; this addendum is its design record —
|
||||
the user ruled it a follow-up, not a milestone 25.
|
||||
|
||||
### The defect
|
||||
|
||||
`bodyChecksum` (manager.zig:1660) and the compiler's incremental hashing
|
||||
(compiler.zig:105-108) both digest the unframed concatenation
|
||||
`list ++ wild ++ allow`. The compiler strips `*.` from a wildcard candidate
|
||||
(compiler.zig:130-133), so upstream `a.example` (list `a.example\n`, wild
|
||||
empty) and upstream `*.a.example` (list empty, wild `a.example\n`) hash the
|
||||
same bytes. `diskBodiesMatch` (manager.zig:1085) recomputes with the same
|
||||
function, so the refresh takes the unchanged-checksum branch and a list that
|
||||
switches an exact block to a wildcard block never takes effect. Ruling 3's
|
||||
entry-count argument rested on the digest distinguishing bodies; it does not,
|
||||
and the strikethrough above records that.
|
||||
|
||||
### The fix: a `0x00` separator after each body
|
||||
|
||||
The digest becomes `SHA-256(list ‖ 00 ‖ wild ‖ 00 ‖ allow ‖ 00)` — one zero
|
||||
byte fed to the hasher **after each of the three bodies**, same order as
|
||||
today. Soundness: a compiled body holds only validated name bytes and `\n`;
|
||||
`addCandidate` rejects any byte ≥ `0x80` or control byte
|
||||
(compiler.zig:151-155), so `0x00` cannot occur in a body and the three
|
||||
boundaries are unambiguous. Two distinct `(list, wild, allow)` triples cannot
|
||||
produce one digest short of SHA-256 itself.
|
||||
|
||||
A separator, not a length prefix, because the compiler hashes while it emits
|
||||
and does not know a body's length up front; a trailing byte needs no pre-pass.
|
||||
|
||||
Both producers move together or every refresh republishes forever:
|
||||
`compiler.compile` feeds the byte after each `emit` call, and
|
||||
`manager.bodyChecksum` feeds it after each body slice. Export the separator as
|
||||
a `pub const` from `compiler.zig` and have `bodyChecksum` use it — two literal
|
||||
`0`s in two files is how the next drift starts.
|
||||
|
||||
### Consequences, accepted
|
||||
|
||||
- Every stored checksum changes once. Zero installs; a development data
|
||||
directory fails its startup checksum verification and the startup refresh
|
||||
pass re-downloads and repairs it (or delete the data directory — the ruling
|
||||
1 stance).
|
||||
- Checksum compatibility with pre-exception digests — milestone 21 ruling 3's
|
||||
"empty allow body reproduces the old digest" property — is dead, and its
|
||||
rationale comments go with it: `Result.checksum` (compiler.zig:44-53), the
|
||||
`Header` doc and `bodyChecksum` doc (manager.zig:218-225, 1645-1648),
|
||||
`SourceStats.checksum` (sources_repo.zig), and the PLAN §3.8 sentence
|
||||
"keeps the digest it had when only two existed" (PLAN.md:101). All are
|
||||
rewritten to state the framed digest. The body order stays list, wild,
|
||||
allow.
|
||||
|
||||
### The branch reads all five fresh
|
||||
|
||||
On the checksum-unchanged path, all five stat fields —
|
||||
`domain_count`, `wildcard_count`, `exception_count`, `skipped_regex_count`,
|
||||
`skipped_unsupported_count` — now read from `compiled.result.counts`;
|
||||
`checksum` keeps passing `stored`. With framing, fresh and stored entry counts
|
||||
are provably equal, so this is not a correctness requirement — it removes the
|
||||
per-field vouching argument from the code entirely, and any future digest
|
||||
weakness then degrades to consistent stats rather than a split between
|
||||
database and status table.
|
||||
|
||||
### Regression tests (each watched failing with the framing reverted)
|
||||
|
||||
- [ ] Compiler: compiling `a.example` and compiling `*.a.example` produce
|
||||
**different** checksums. This is the collision itself.
|
||||
- [ ] Manager: publish a source whose body is `a.example`; refresh it with
|
||||
upstream bytes `*.a.example`. Assert the unchanged-checksum branch is
|
||||
**not** taken: the on-disk `.wild` stripped body is `a.example\n`, the
|
||||
`.list` body is empty, and the database row reads `domain_count = 0`,
|
||||
`wildcard_count = 1`.
|
||||
- [ ] Agreement: `bodyChecksum` over the three stripped on-disk bodies equals
|
||||
`compile`'s reported checksum for a fixture where **all three bodies are
|
||||
non-empty** (extend the existing agreement coverage if it exists; the
|
||||
empty-allow case no longer exercises the third frame).
|
||||
|
||||
Report the observed failure output for each, per the ruling 3 convention.
|
||||
|
||||
+48
-25
@@ -43,16 +43,30 @@ pub const Counts = struct {
|
||||
|
||||
pub const Result = struct {
|
||||
counts: Counts,
|
||||
/// Lowercase hex sha256 over the `.list` body, then the `.wild` body, then
|
||||
/// the `.allow` body.
|
||||
/// Lowercase hex sha256 over the `.list` body, the `.wild` body and the
|
||||
/// `.allow` body in that order, each followed by `body_separator`.
|
||||
///
|
||||
/// The `.allow` body came last for a reason: hashing an empty one adds
|
||||
/// nothing, so a list with no exceptions keeps the digest it had when only
|
||||
/// two bodies existed. Every checksum published before exceptions were
|
||||
/// honoured therefore stays valid, and upgrading forces no refetch.
|
||||
/// The separator is what makes the digest identify a compile. Without it
|
||||
/// the three bodies concatenate ambiguously: a wildcard is stored with its
|
||||
/// `*.` stripped, so an upstream that changes `a.example` to `*.a.example`
|
||||
/// moves the same bytes from the `.list` body to the `.wild` body and
|
||||
/// hashes to the same digest. `Manager.diskBodiesMatch` would then accept
|
||||
/// the stale files, the refresh would keep them, and the wildcard would
|
||||
/// never take effect.
|
||||
checksum: [64]u8,
|
||||
};
|
||||
|
||||
/// Fed to the checksum hasher after each of the three bodies, so the digest
|
||||
/// reads them as three fields rather than one run of bytes.
|
||||
///
|
||||
/// `0x00` is sound as a separator because it can never occur inside a body:
|
||||
/// `addCandidate` rejects every control byte and every byte at or above `0x80`,
|
||||
/// so a body holds none. Any producer of this digest must use this constant —
|
||||
/// `compiler.compile` hashes while it emits and `Manager.bodyChecksum` hashes
|
||||
/// three finished buffers, and a one-byte disagreement between them would make
|
||||
/// every refresh republish for ever.
|
||||
pub const body_separator = "\x00";
|
||||
|
||||
pub const Error = error{ OutOfMemory, TooManyDomains, ReadFailed, WriteFailed };
|
||||
|
||||
/// Reads `r` to end of stream and writes the three compiled bodies.
|
||||
@@ -100,12 +114,15 @@ pub fn compile(
|
||||
}
|
||||
}
|
||||
|
||||
// The `.allow` body is hashed last so that a list with no exceptions
|
||||
// reproduces the digest a two-body compile of the same bytes produced.
|
||||
// Each body is followed by `body_separator`, which is what keeps the digest
|
||||
// from confusing a name in one body with the same name in another.
|
||||
var hasher = Sha256.init(.{});
|
||||
counts.domains = try emit(&bodies.list, list_w, &hasher, &counts.duplicates);
|
||||
hasher.update(body_separator);
|
||||
counts.wildcards = try emit(&bodies.wild, wild_w, &hasher, &counts.duplicates);
|
||||
hasher.update(body_separator);
|
||||
counts.exceptions = try emit(&bodies.allow, allow_w, &hasher, &counts.duplicates);
|
||||
hasher.update(body_separator);
|
||||
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
@@ -433,22 +450,23 @@ test "the allow body is sorted, deduplicated and normalized like the others" {
|
||||
try testing.expectEqualStrings("ads.example\n", c.wild());
|
||||
}
|
||||
|
||||
test "an empty allow body reproduces the checksum of a two-body compile" {
|
||||
var c = try compileText(testing.allocator, hosts_fixture, .hosts);
|
||||
defer c.deinit();
|
||||
try testing.expectEqualStrings("", c.allow());
|
||||
test "moving a name between bodies changes the checksum" {
|
||||
// The collision the separator exists to prevent. A wildcard is stored with
|
||||
// its `*.` stripped, so both compiles write the bytes `a.example\n` — one
|
||||
// into the `.list` body, one into the `.wild` body. Unframed, the two hash
|
||||
// identically, `diskBodiesMatch` accepts the stale files, and an upstream
|
||||
// that switched a name to a wildcard never takes effect.
|
||||
var exact = try compileText(testing.allocator, "a.example\n", .domains);
|
||||
defer exact.deinit();
|
||||
var wild = try compileText(testing.allocator, "*.a.example\n", .domains);
|
||||
defer wild.deinit();
|
||||
|
||||
// What the digest was before the `.allow` body existed: the `.list` body
|
||||
// followed by the `.wild` body and nothing else. Every checksum stored by an
|
||||
// older nxdns was taken this way, and this is the equality that keeps them
|
||||
// valid — without it, every source on every installation would report
|
||||
// `ChecksumMismatch` at the first reload after the upgrade and re-download.
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(c.list());
|
||||
hasher.update(c.wild());
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &c.result.checksum);
|
||||
try testing.expectEqualStrings("a.example\n", exact.list());
|
||||
try testing.expectEqualStrings("", exact.wild());
|
||||
try testing.expectEqualStrings("", wild.list());
|
||||
try testing.expectEqualStrings("a.example\n", wild.wild());
|
||||
|
||||
try testing.expect(!std.mem.eql(u8, &exact.result.checksum, &wild.result.checksum));
|
||||
}
|
||||
|
||||
test "the checksum of a source with exceptions covers all three bodies in order" {
|
||||
@@ -461,8 +479,11 @@ test "the checksum of a source with exceptions covers all three bodies in order"
|
||||
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(c.list());
|
||||
hasher.update(body_separator);
|
||||
hasher.update(c.wild());
|
||||
hasher.update(body_separator);
|
||||
hasher.update(c.allow());
|
||||
hasher.update(body_separator);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &c.result.checksum);
|
||||
@@ -620,15 +641,17 @@ test "carriage returns are stripped" {
|
||||
try testing.expectEqualStrings("a.example.com\nb.example.com\n", c.list());
|
||||
}
|
||||
|
||||
test "empty input produces empty bodies and the sha256 of the empty string" {
|
||||
test "empty input produces empty bodies and the digest of three separators" {
|
||||
var c = try compileText(testing.allocator, "", .domains);
|
||||
defer c.deinit();
|
||||
|
||||
try testing.expectEqualStrings("", c.list());
|
||||
try testing.expectEqualStrings("", c.wild());
|
||||
try testing.expectEqualStrings("", c.allow());
|
||||
// Three empty bodies still hash their three separators, so an empty compile
|
||||
// has a digest of its own rather than the sha256 of the empty string.
|
||||
try testing.expectEqualStrings(
|
||||
"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
|
||||
"709e80c88487a2411e1ee4dfb9f22a861492d20c4765150c0c794abd70f8147c",
|
||||
&c.result.checksum,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -258,11 +258,17 @@ fn snapshotOver(
|
||||
});
|
||||
}
|
||||
|
||||
/// The third producer of this digest, beside `compiler.compile` and
|
||||
/// `Manager.bodyChecksum`. All three must frame the bodies the same way or the
|
||||
/// manager reads its own files as damaged.
|
||||
fn bodyChecksum(list_body: []const u8, wild_body: []const u8, allow_body: []const u8) [64]u8 {
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(list_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(wild_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(allow_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
return std.fmt.bytesToHex(digest, .lower);
|
||||
@@ -2149,7 +2155,7 @@ test "19: a downloaded list's exceptions lift its own blocks and nothing else" {
|
||||
}
|
||||
}
|
||||
|
||||
test "20: a data directory written before exceptions existed loads with no checksum mismatch" {
|
||||
test "20: a data directory with no .allow file loads, and an unframed checksum does not" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
const gpa = testing.allocator;
|
||||
@@ -2159,8 +2165,8 @@ test "20: a data directory written before exceptions existed loads with no check
|
||||
|
||||
const id = try seedSource(&env.database, source_url);
|
||||
|
||||
// Exactly what an older nxdns left behind: two compiled files, no `.allow`
|
||||
// file, and a checksum taken over the two bodies alone.
|
||||
// Two compiled files and no `.allow` file, which is what a source that
|
||||
// published before exceptions existed left on disk.
|
||||
const list_body = "aaa.example.com\nbbb.example.com\n";
|
||||
const wild_body = "ccc.example.com\n";
|
||||
|
||||
@@ -2177,33 +2183,55 @@ test "20: a data directory written before exceptions existed loads with no check
|
||||
.data = wild_body,
|
||||
});
|
||||
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(list_body);
|
||||
hasher.update(wild_body);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
const legacy_checksum = std.fmt.bytesToHex(digest, .lower);
|
||||
|
||||
try sources_repo.updateSourceStats(&env.database, id, .{
|
||||
var stats: sources_repo.SourceStats = .{
|
||||
.last_updated = 1_700_000_000,
|
||||
.domain_count = 2,
|
||||
.wildcard_count = 1,
|
||||
.exception_count = 0,
|
||||
.skipped_regex_count = 0,
|
||||
.skipped_unsupported_count = 0,
|
||||
.checksum = &legacy_checksum,
|
||||
});
|
||||
.checksum = undefined,
|
||||
};
|
||||
|
||||
try env.mgr.reload(io);
|
||||
// The digest those two files carried before the bodies were framed. It is
|
||||
// not the digest of any body layout the compiler produces now, so the load
|
||||
// must report the mismatch instead of serving the files.
|
||||
{
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(list_body);
|
||||
hasher.update(wild_body);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
const unframed = std.fmt.bytesToHex(digest, .lower);
|
||||
|
||||
const status = try env.status(id);
|
||||
try testing.expectEqual(manager.State.ok, status.state);
|
||||
try testing.expect(status.loaded);
|
||||
try testing.expectEqualStrings("", status.errorText());
|
||||
stats.checksum = &unframed;
|
||||
try sources_repo.updateSourceStats(&env.database, id, stats);
|
||||
try env.mgr.reload(io);
|
||||
|
||||
const decision, _ = try env.evaluate("aaa.example.com");
|
||||
try testing.expect(decision.blocked);
|
||||
try testing.expect((try env.evaluate("x.ccc.example.com"))[0].blocked);
|
||||
const status = try env.status(id);
|
||||
try testing.expectEqual(manager.State.load_failed, status.state);
|
||||
try testing.expect(!status.loaded);
|
||||
try testing.expectEqualStrings("ChecksumMismatch", status.errorText());
|
||||
}
|
||||
|
||||
// The same two files under the framed digest of three bodies, the third of
|
||||
// them empty. The absent `.allow` file is that empty body, so the source
|
||||
// loads and both entries filter.
|
||||
{
|
||||
const framed = bodyChecksum(list_body, wild_body, "");
|
||||
stats.checksum = &framed;
|
||||
try sources_repo.updateSourceStats(&env.database, id, stats);
|
||||
try env.mgr.reload(io);
|
||||
|
||||
const status = try env.status(id);
|
||||
try testing.expectEqual(manager.State.ok, status.state);
|
||||
try testing.expect(status.loaded);
|
||||
try testing.expectEqualStrings("", status.errorText());
|
||||
|
||||
const decision, _ = try env.evaluate("aaa.example.com");
|
||||
try testing.expect(decision.blocked);
|
||||
try testing.expect((try env.evaluate("x.ccc.example.com"))[0].blocked);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -2267,6 +2295,42 @@ const churn_after =
|
||||
"||blocked.example^\n" ++
|
||||
"tracker.example\n";
|
||||
|
||||
const boundary_before = "a.example\n";
|
||||
const boundary_after = "*.a.example\n";
|
||||
|
||||
test "23: a name moving from the list body to the wild body forces a republish" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
const gpa = testing.allocator;
|
||||
const env = try Env.create(gpa);
|
||||
defer env.destroy();
|
||||
const io = env.io();
|
||||
|
||||
var fixture = try HttpFixture.init(io, boundary_before);
|
||||
fixture.changed_body = boundary_after;
|
||||
defer fixture.deinit(io);
|
||||
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);
|
||||
_ = try seedSource(&env.database, url);
|
||||
|
||||
try testing.expect(try refreshOnce(env, url));
|
||||
|
||||
var first_checksum: [64]u8 = undefined;
|
||||
{
|
||||
var rows = try listRows(&env.database);
|
||||
defer rows.deinit();
|
||||
const row = try rows.byUrl(url);
|
||||
try testing.expectEqual(@as(i64, 1), row.domain_count);
|
||||
try testing.expectEqual(@as(i64, 0), row.wildcard_count);
|
||||
@memcpy(&first_checksum, row.checksum orelse return error.TestNoChecksum);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
test "22: a list that changed only its skipped lines still updates both skip counters" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
|
||||
+52
-35
@@ -220,9 +220,8 @@ pub const SourceStatus = struct {
|
||||
/// **not** the header, so it stays stable across a refetch of unchanged content
|
||||
/// while `fetched_at` moves.
|
||||
///
|
||||
/// The `.allow` body is hashed last so that a source with no exceptions keeps
|
||||
/// the digest it had when only two bodies existed: every checksum written before
|
||||
/// exceptions were honoured stays valid, and no upgrade forces a refetch.
|
||||
/// Each body is followed by a separator byte, so the digest identifies which
|
||||
/// body a name sits in rather than only which names were written.
|
||||
pub const Header = struct {
|
||||
url: []const u8,
|
||||
format: parsers.Format,
|
||||
@@ -591,11 +590,10 @@ pub const Manager = struct {
|
||||
};
|
||||
bodies.appendAssumeCapacity(wild_bytes);
|
||||
|
||||
// A missing `.allow` file is an empty allow body, not a failure. Two
|
||||
// sources are in that state and both are ordinary: one compiled before
|
||||
// exceptions were honoured, and one whose list carries no `@@` line.
|
||||
// Because the empty body contributes nothing to the checksum, the
|
||||
// stored digest of either still matches.
|
||||
// A missing `.allow` file is an empty allow body, not a failure. The
|
||||
// digest still covers three bodies, the third of them empty, so a
|
||||
// source whose list carries no `@@` line matches whether its empty
|
||||
// `.allow` file survived or not.
|
||||
const allow_bytes: []const u8 = blk: {
|
||||
const read = dir.readFileAlloc(io, allow_name, self.gpa, .limited(max_compiled_bytes)) catch |err| {
|
||||
if (err == error.OutOfMemory) return error.OutOfMemory;
|
||||
@@ -822,19 +820,19 @@ pub const Manager = struct {
|
||||
if (std.mem.eql(u8, stored, &compiled.result.checksum) and
|
||||
self.diskBodiesMatch(io, dir, row.id, stored))
|
||||
{
|
||||
// The three written counts come from the row: the checksum
|
||||
// covers the three bodies, so an unchanged checksum means an
|
||||
// unchanged number of entries in each. The two skip counts do
|
||||
// not: a skipped line lands in no body, so a list that changed
|
||||
// only its regex or browser-syntax lines arrives here with a
|
||||
// stale row and a fresh compile. Taking them from the row would
|
||||
// show the operator one number and restore another after a
|
||||
// restart.
|
||||
// Every count comes from the compile that just ran, not from
|
||||
// the row. The two skip counts have to: a skipped line lands in
|
||||
// no body, so a list that changed only its regex or
|
||||
// browser-syntax lines reaches here with a stale row. The three
|
||||
// written counts equal the row's anyway once the digest is
|
||||
// framed, so reading them from the compile costs nothing and
|
||||
// leaves no field whose freshness rests on an argument about
|
||||
// what the checksum covers.
|
||||
try sources_repo.updateSourceStats(self.database, row.id, .{
|
||||
.last_updated = now,
|
||||
.domain_count = row.domain_count,
|
||||
.wildcard_count = row.wildcard_count,
|
||||
.exception_count = row.exception_count,
|
||||
.domain_count = compiled.result.counts.domains,
|
||||
.wildcard_count = compiled.result.counts.wildcards,
|
||||
.exception_count = compiled.result.counts.exceptions,
|
||||
.skipped_regex_count = compiled.result.counts.skipped_regex,
|
||||
.skipped_unsupported_count = compiled.result.counts.skipped_unsupported,
|
||||
.checksum = stored,
|
||||
@@ -1078,10 +1076,9 @@ pub const Manager = struct {
|
||||
/// the rewrite path — the only path that can repair it.
|
||||
///
|
||||
/// A missing `.allow` file is the one exception, and it is the same one
|
||||
/// `loadSource` makes: a source compiled before exceptions were honoured has
|
||||
/// no such file, and its stored checksum was taken over an empty allow body.
|
||||
/// Answering false there would rewrite every list on the first refresh after
|
||||
/// an upgrade for no change in content.
|
||||
/// `loadSource` makes: it reads as an empty allow body, which is what a list
|
||||
/// with no `@@` line compiles to anyway. Answering false there would rewrite
|
||||
/// such a list on every refresh for no change in content.
|
||||
fn diskBodiesMatch(self: *Manager, io: std.Io, dir: std.Io.Dir, id: i64, expected: []const u8) bool {
|
||||
var list_buf: [name_buf_len]u8 = undefined;
|
||||
var wild_buf: [name_buf_len]u8 = undefined;
|
||||
@@ -1653,15 +1650,20 @@ fn rejectedWithoutEntries(counts: compiler.Counts) bool {
|
||||
return counts.invalid != 0 or counts.skipped_unsupported != 0 or counts.long_lines != 0;
|
||||
}
|
||||
|
||||
/// The digest the `.list`, `.wild` and `.allow` bodies share, in that order.
|
||||
/// The allow body comes last so that hashing an empty one leaves the digest of
|
||||
/// the two-body form untouched, which is what keeps every checksum stored before
|
||||
/// exceptions were honoured valid.
|
||||
/// The digest the `.list`, `.wild` and `.allow` bodies share, in that order,
|
||||
/// each followed by `compiler.body_separator`.
|
||||
///
|
||||
/// Must stay byte-for-byte what `compiler.compile` produces, separators
|
||||
/// included: this is the other half of the same digest, and the two are
|
||||
/// compared against each other on every refresh.
|
||||
fn bodyChecksum(list_body: []const u8, wild_body: []const u8, allow_body: []const u8) [64]u8 {
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(list_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(wild_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(allow_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
return std.fmt.bytesToHex(digest, .lower);
|
||||
@@ -2412,23 +2414,33 @@ test "compiledBodiesMatch verifies the bodies, not the presence of the files" {
|
||||
try testing.expect(!compiledBodiesMatch("", "", "", &expected));
|
||||
}
|
||||
|
||||
test "a source with no exceptions keeps the checksum it had before the allow body existed" {
|
||||
test "bodyChecksum separates the three bodies" {
|
||||
const list_body = "a.example.com\nb.example.com\n";
|
||||
const wild_body = "c.example.com\n";
|
||||
|
||||
// What an older nxdns stored: the digest of the two bodies alone. It is what
|
||||
// sits in `blocklist_sources.checksum` on every installation being upgraded,
|
||||
// and the files on disk are the two it was taken over.
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update(list_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(wild_body);
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update(compiler.body_separator);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
const stored = std.fmt.bytesToHex(digest, .lower);
|
||||
const expected = std.fmt.bytesToHex(digest, .lower);
|
||||
|
||||
try testing.expectEqualStrings(&stored, &bodyChecksum(list_body, wild_body, ""));
|
||||
try testing.expectEqualStrings(&expected, &bodyChecksum(list_body, wild_body, ""));
|
||||
// No `.allow` file: what `loadSource` and `diskBodiesMatch` pass for one.
|
||||
try testing.expect(compiledBodiesMatch(list_body, wild_body, "", &stored));
|
||||
// It is an empty body, and an empty body still gets its separator.
|
||||
try testing.expect(compiledBodiesMatch(list_body, wild_body, "", &expected));
|
||||
|
||||
// The framing itself: the same bytes in a different body is a different
|
||||
// digest. Unframed these two are equal, and a stale `.list` survives an
|
||||
// upstream that switched the name to a wildcard.
|
||||
try testing.expect(!std.mem.eql(
|
||||
u8,
|
||||
&bodyChecksum("a.example\n", "", ""),
|
||||
&bodyChecksum("", "a.example\n", ""),
|
||||
));
|
||||
}
|
||||
|
||||
test "rejectedWithoutEntries fails a compile that produced nothing usable" {
|
||||
@@ -2552,7 +2564,12 @@ test "collectSample steps over a line that does not fit the reader buffer" {
|
||||
test "bodyChecksum covers the list body, then the wild body, then the allow body" {
|
||||
const all = bodyChecksum("a.example.com\n", "b.example.com\n", "c.example.com\n");
|
||||
var hasher = Sha256.init(.{});
|
||||
hasher.update("a.example.com\nb.example.com\nc.example.com\n");
|
||||
hasher.update("a.example.com\n");
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update("b.example.com\n");
|
||||
hasher.update(compiler.body_separator);
|
||||
hasher.update("c.example.com\n");
|
||||
hasher.update(compiler.body_separator);
|
||||
var digest: [Sha256.digest_length]u8 = undefined;
|
||||
hasher.final(&digest);
|
||||
try testing.expectEqualStrings(&std.fmt.bytesToHex(digest, .lower), &all);
|
||||
|
||||
@@ -111,9 +111,8 @@ pub const SourceStats = struct {
|
||||
skipped_regex_count: i64,
|
||||
skipped_unsupported_count: i64,
|
||||
/// Lowercase hex sha256 over the `.list` body, then the `.wild` body, then
|
||||
/// the `.allow` body. The allow body is hashed last so an empty one leaves
|
||||
/// the digest of a two-body compile unchanged, which is what keeps a
|
||||
/// checksum written before exceptions were honoured valid.
|
||||
/// the `.allow` body, each followed by `compiler.body_separator` so the
|
||||
/// digest cannot confuse a name in one body with the same name in another.
|
||||
checksum: []const u8,
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user