Skip to content

test(dns): pin zero through the wire format and the zone file - #233

Merged
Meldiron merged 1 commit into
mainfrom
test/dns-zero-priority-weight-port
Sep 8, 2026
Merged

test(dns): pin zero through the wire format and the zone file#233
Meldiron merged 1 commit into
mainfrom
test/dns-zero-priority-weight-port

Conversation

@Meldiron

@Meldiron Meldiron commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Reopened here from utopia-php/dns#51, which the mirror bot closed — development for packages/dns happens in this repository. Rewritten against the package's current conventions rather than cherry-picked, since the mirror has drifted (declare(strict_types=1), trailing commas, \strlen, assertInstanceOf).

Why

Traced from a report that an MX record with priority 0 could not be added to Appwrite Cloud. The defect was in the console, and this library takes zero correctly at every layer — but nothing proved it.

priority, weight and port are nullable because most record types do not carry them. Every place that reads them already uses ?? 0, not ?: 0 — correctly, since a zero the caller set is not a zero the caller omitted. MX priority 0 is the highest priority a mail exchange can be given, and RFC 2782 gives all three SRV fields a meaning at zero: highest priority, no share of the weighted draw, and the port that marks the service unavailable on that target.

The existing coverage uses 10 for MX, 5/10/5060 for SRV, and 10, 20 and 50 in the zone fixtures. So ?? 0 could become ?: '' in File::formatRdata and only the zone export would break — silently, into a line File::parseRdata then refuses, which fails the import of the entire zone rather than the one record.

What this adds

Four cases in RecordTest — MX priority 0 and SRV 0 0 0, encode and decode — and two in FileTest for the export/import round trip.

OK (181 tests, 596 assertions)

bin/monorepo validate passes, Pint passes.

Seen red: rewriting the MX branch of File::formatRdata to $pri = $priority ?: '' exports mail\t300\tIN\tMX\t mail and fails testExportAndImportKeepZeroMxPriority.

Related

  • appwrite/vibes#293 — the actual bug. parseInt(priority) || undefined read a typed 0 as "not supplied", so the console refused before a request was sent.
  • appwrite-labs/cloud#5710 — the API accepted priority 0, but validated priority/weight/port with a bare Integer(), so a negative priority wrote a zone line File::parseRdata refuses. Now Range(0, 65535).
  • appwrite-labs/dns#91 — the same coverage one level up, over the resolver that consumes this library.

🤖 Generated with Claude Code

`priority`, `weight` and `port` are nullable because most record types do not
carry them, and every place that reads them uses `?? 0` rather than `?: 0` --
correctly, since a zero the caller set is not a zero the caller omitted. MX
priority 0 is the highest priority a mail exchange can be given, and RFC 2782
gives all three SRV fields a meaning at zero.

Nothing held that. The MX and SRV cases in `RecordTest` use 10 and 5/10/5060,
and the zone fixtures use 10, 20 and 50, so `?? 0` could become `?: ''` and
only the zone export broke -- silently, into a line `File::parseRdata` then
refuses, which fails the import of the whole zone rather than the one record.

Covers encode, decode and the zone export/import round trip for MX priority 0
and SRV 0 0 0. Verified red: rewriting the MX branch of `File::formatRdata` to
`$pri = $priority ?: ''` exports `mail 300 IN MX  mail` and fails the round
trip test.

Found while tracing a report that an MX record with priority 0 could not be
added to Appwrite Cloud. The defect was in the console, and this library
already handled zero correctly at every layer -- but nothing proved it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Benchmark results

dns — transport throughput (4 cores, 250 repeats x 20 workers x 3 domains per transport)

transport req/s ok p50 ms p95 p99 max
udp 17460.37 15000/15000 1 1 1 3
tcp 17622.04 15000/15000 1 1 1 3
doh 10556.08 15000/15000 1 3 4 8

Shared CI runners — treat absolute numbers as rough, compare modes within a run. Commit 6fe4f12.

@Meldiron Meldiron left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just tests

@Meldiron
Meldiron merged commit 2e0a93d into main Sep 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant