From d2c8de567ad3a7cff665750fecdb1734e624de68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 26 Mar 2026 17:33:12 +0100 Subject: [PATCH 1/3] Fix soa zone answer --- src/DNS/Zone/Resolver.php | 24 ++++++++++++++++ tests/unit/DNS/Zone/ResolverTest.php | 43 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/DNS/Zone/Resolver.php b/src/DNS/Zone/Resolver.php index ef1646b..cbcffd1 100644 --- a/src/DNS/Zone/Resolver.php +++ b/src/DNS/Zone/Resolver.php @@ -42,6 +42,18 @@ public static function lookup(Message $query, Zone $zone): Message $records = self::selectBestRecords($query, $zone); if (empty($records)) { + // SOA is stored separately; if querying SOA at the zone apex, return it + if ($question->type === Record::TYPE_SOA && $question->name === $zone->name) { + return Message::response( + header: $query->header, + responseCode: Message::RCODE_NOERROR, + questions: $query->questions, + answers: [$zone->soa], + authoritative: true, + recursionAvailable: false + ); + } + return Message::response( header: $query->header, responseCode: Message::RCODE_NXDOMAIN, @@ -157,6 +169,18 @@ private static function handleExactMatch(array $records, Message $query, Zone $z $isAuthoritative = $zone->isAuthoritative($question->name); if ($isAuthoritative) { + // SOA is stored separately in Zone; handle SOA queries at the zone apex + if ($question->type === Record::TYPE_SOA && $question->name === $zone->name) { + return Message::response( + header: $query->header, + responseCode: Message::RCODE_NOERROR, + questions: $query->questions, + answers: [$zone->soa], + authoritative: true, + recursionAvailable: false + ); + } + // Path E1: Exact match of type $exactTypeRecords = array_filter( $records, diff --git a/tests/unit/DNS/Zone/ResolverTest.php b/tests/unit/DNS/Zone/ResolverTest.php index 0146a09..3cf5c2e 100644 --- a/tests/unit/DNS/Zone/ResolverTest.php +++ b/tests/unit/DNS/Zone/ResolverTest.php @@ -465,6 +465,49 @@ public function testLookupReturnsApexAAAARecord(): void $this->assertSame($aaaaRecord, $response->answers[0]); } + public function testLookupReturnsSoaAnswerForApexSoaQueryWithRecords(): void + { + $soa = new Record( + 'example.com', + Record::TYPE_SOA, + ttl: 300, + rdata: 'ns1.appwrite.zone. team@appwrite.io. 1761705275 3600 600 86400 300' + ); + $aRecord = new Record('example.com', Record::TYPE_A, ttl: 3600, rdata: '1.1.1.1'); + $zone = new Zone('example.com', [$aRecord], $soa); + + $question = new Question('example.com', Record::TYPE_SOA); + $query = Message::query($question); + $response = Resolver::lookup($query, $zone); + + $this->assertSame(Message::RCODE_NOERROR, $response->header->responseCode); + $this->assertCount(1, $response->answers); + $this->assertSame($soa, $response->answers[0]); + $this->assertTrue($response->header->authoritative); + $this->assertFalse($response->header->recursionAvailable); + } + + public function testLookupReturnsSoaAnswerForApexSoaQueryWithNoRecords(): void + { + $soa = new Record( + 'example.com', + Record::TYPE_SOA, + ttl: 300, + rdata: 'ns1.appwrite.zone. team@appwrite.io. 1761705275 3600 600 86400 300' + ); + $zone = new Zone('example.com', [], $soa); + + $question = new Question('example.com', Record::TYPE_SOA); + $query = Message::query($question); + $response = Resolver::lookup($query, $zone); + + $this->assertSame(Message::RCODE_NOERROR, $response->header->responseCode); + $this->assertCount(1, $response->answers); + $this->assertSame($soa, $response->answers[0]); + $this->assertTrue($response->header->authoritative); + $this->assertFalse($response->header->recursionAvailable); + } + public function testLookupReturnsSoaInAuthorityForApexNonNSQuery(): void { $soa = new Record( From f2851dfa6c24837946b10275a0b193deaaf997f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 26 Mar 2026 17:53:55 +0100 Subject: [PATCH 2/3] Fix tests --- src/DNS/Zone/Resolver.php | 33 +++++++++++++++++---------------- tests/e2e/DNS/ClientTest.php | 16 ++++++++-------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/DNS/Zone/Resolver.php b/src/DNS/Zone/Resolver.php index cbcffd1..6385795 100644 --- a/src/DNS/Zone/Resolver.php +++ b/src/DNS/Zone/Resolver.php @@ -44,14 +44,7 @@ public static function lookup(Message $query, Zone $zone): Message if (empty($records)) { // SOA is stored separately; if querying SOA at the zone apex, return it if ($question->type === Record::TYPE_SOA && $question->name === $zone->name) { - return Message::response( - header: $query->header, - responseCode: Message::RCODE_NOERROR, - questions: $query->questions, - answers: [$zone->soa], - authoritative: true, - recursionAvailable: false - ); + return self::soaApexResponse($query, $zone); } return Message::response( @@ -171,14 +164,7 @@ private static function handleExactMatch(array $records, Message $query, Zone $z if ($isAuthoritative) { // SOA is stored separately in Zone; handle SOA queries at the zone apex if ($question->type === Record::TYPE_SOA && $question->name === $zone->name) { - return Message::response( - header: $query->header, - responseCode: Message::RCODE_NOERROR, - questions: $query->questions, - answers: [$zone->soa], - authoritative: true, - recursionAvailable: false - ); + return self::soaApexResponse($query, $zone); } // Path E1: Exact match of type @@ -239,6 +225,21 @@ private static function handleExactMatch(array $records, Message $query, Zone $z } } + /** + * Build an authoritative SOA answer for the zone apex. + */ + private static function soaApexResponse(Message $query, Zone $zone): Message + { + return Message::response( + header: $query->header, + responseCode: Message::RCODE_NOERROR, + questions: $query->questions, + answers: [$zone->soa], + authoritative: true, + recursionAvailable: false + ); + } + /** * Randomize RRSet order for load balancing. * diff --git a/tests/e2e/DNS/ClientTest.php b/tests/e2e/DNS/ClientTest.php index 7213f4e..ed7c6fb 100644 --- a/tests/e2e/DNS/ClientTest.php +++ b/tests/e2e/DNS/ClientTest.php @@ -213,16 +213,16 @@ public function testSoaRecords(): void $response = $client->query(Message::query( new Question('appwrite.io', Record::TYPE_SOA) )); - $this->assertCount(0, $response->answers); + $this->assertCount(0, $response->authority); - $authority = $response->authority; - $this->assertCount(1, $authority); - $this->assertSame('appwrite.io', $authority[0]->name); - $this->assertSame(Record::CLASS_IN, $authority[0]->class); - $this->assertSame(30, $authority[0]->ttl); - $this->assertSame(Record::TYPE_SOA, $authority[0]->type); + $answers = $response->answers; + $this->assertCount(1, $answers); + $this->assertSame('appwrite.io', $answers[0]->name); + $this->assertSame(Record::CLASS_IN, $answers[0]->class); + $this->assertSame(30, $answers[0]->ttl); + $this->assertSame(Record::TYPE_SOA, $answers[0]->type); - $rdata = $authority[0]->rdata; + $rdata = $answers[0]->rdata; $this->assertStringContainsString('ns1.appwrite.zone', $rdata); $this->assertStringContainsString('team.appwrite.io', $rdata); $this->assertStringContainsString('1 7200 1800 1209600 3600', $rdata); From c8c8a26c4966f62527ec9fed1f31d159c4100c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Tue, 8 Sep 2026 15:24:04 +0200 Subject: [PATCH 3/3] test(record): pin zero through the wire format and the zone file `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` used 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, taking the whole zone with it. 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. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/DNS/Message/RecordTest.php | 95 +++++++++++++++++++++++++++ tests/unit/DNS/Zone/FileTest.php | 47 +++++++++++++ 2 files changed, 142 insertions(+) diff --git a/tests/unit/DNS/Message/RecordTest.php b/tests/unit/DNS/Message/RecordTest.php index 029a581..54ada0f 100644 --- a/tests/unit/DNS/Message/RecordTest.php +++ b/tests/unit/DNS/Message/RecordTest.php @@ -148,6 +148,101 @@ public function testDecodeSrvRecordParsesFields(): void $this->assertSame(strlen($data), $offset); } + public function testEncodeMxRecordWithZeroPriorityMatchesBytes(): void + { + // Zero is the highest priority a mail exchange can be given, and it is a value the + // sender chose, not a missing one. It has to reach the wire as two zero octets. + $record = new Record( + name: 'mail.example.com', + type: Record::TYPE_MX, + class: Record::CLASS_IN, + ttl: 3600, + rdata: 'mail.exchange.example.com', + priority: 0 + ); + + // Raw RR: mail.example.com. 3600 IN MX 0 mail.exchange.example.com. + $expected = "\x04mail\x07example\x03com\x00" + . "\x00\x0F" + . "\x00\x01" + . "\x00\x00\x0E\x10" + . "\x00\x1D" + . "\x00\x00" + . "\x04mail\x08exchange\x07example\x03com\x00"; + + $this->assertSame($expected, $record->encode()); + } + + public function testDecodeMxRecordWithZeroPriorityParsesFields(): void + { + // Raw RR: mail.example.com. 3600 IN MX 0 mail.exchange.example.com. + $data = "\x04mail\x07example\x03com\x00" + . "\x00\x0F" + . "\x00\x01" + . "\x00\x00\x0E\x10" + . "\x00\x1D" + . "\x00\x00" + . "\x04mail\x08exchange\x07example\x03com\x00"; + + $offset = 0; + $record = Record::decode($data, $offset); + + // A zero read off the wire has to stay a zero, not become the null that means "this + // record type carries no priority". + $this->assertSame(0, $record->priority); + $this->assertNotNull($record->priority); + $this->assertSame('mail.exchange.example.com', $record->rdata); + $this->assertSame(strlen($data), $offset); + } + + public function testEncodeSrvRecordWithZeroNumericsMatchesBytes(): void + { + // RFC 2782 gives all three fields a meaning at zero: highest priority, no share of the + // weighted draw, and the port that marks the service as unavailable on this target. + $record = new Record( + name: '_sip._tcp.example.com', + type: Record::TYPE_SRV, + class: Record::CLASS_IN, + ttl: 7200, + rdata: 'sip.example.com', + priority: 0, + weight: 0, + port: 0 + ); + + // Raw RR: _sip._tcp.example.com. 7200 IN SRV 0 0 0 sip.example.com. + $expected = "\x04_sip\x04_tcp\x07example\x03com\x00" + . "\x00\x21" + . "\x00\x01" + . "\x00\x00\x1C\x20" + . "\x00\x17" + . "\x00\x00\x00\x00\x00\x00" + . "\x03sip\x07example\x03com\x00"; + + $this->assertSame($expected, $record->encode()); + } + + public function testDecodeSrvRecordWithZeroNumericsParsesFields(): void + { + // Raw RR: _sip._tcp.example.com. 7200 IN SRV 0 0 0 sip.example.com. + $data = "\x04_sip\x04_tcp\x07example\x03com\x00" + . "\x00\x21" + . "\x00\x01" + . "\x00\x00\x1C\x20" + . "\x00\x17" + . "\x00\x00\x00\x00\x00\x00" + . "\x03sip\x07example\x03com\x00"; + + $offset = 0; + $record = Record::decode($data, $offset); + + $this->assertSame(0, $record->priority); + $this->assertSame(0, $record->weight); + $this->assertSame(0, $record->port); + $this->assertSame('sip.example.com', $record->rdata); + $this->assertSame(strlen($data), $offset); + } + public function testEncodeTxtRecordMatchesBytes(): void { $record = new Record( diff --git a/tests/unit/DNS/Zone/FileTest.php b/tests/unit/DNS/Zone/FileTest.php index c51783e..b3dd26b 100644 --- a/tests/unit/DNS/Zone/FileTest.php +++ b/tests/unit/DNS/Zone/FileTest.php @@ -395,6 +395,53 @@ public function testExportBasicZone(): void $this->assertSame('mail.example.com', $roundTrip->records[2]->name); } + public function testExportAndImportKeepZeroMxPriority(): void + { + // The zone file is the only thing a resolver reads. A priority of 0 written as an empty + // field, or dropped, does not just lose the priority: the line stops parsing, and an + // import failure takes every other record in the zone with it. + $zone = new Zone( + 'example.com', + [ + new Record('mail.example.com', Record::TYPE_MX, Record::CLASS_IN, 300, 'mail.example.com', priority: 0), + ], + new Record('example.com', Record::TYPE_SOA, Record::CLASS_IN, 1800, 'ns1.example.com. admin.example.com. 2025011801 7200 3600 1209600 1800'), + ); + + $exported = File::export($zone, includeComments: false); + $this->assertStringContainsString("mail\t300\tIN\tMX\t0 mail\n", $exported); + + $roundTrip = File::import($exported); + $record = $this->findRecord($roundTrip->records, Record::TYPE_MX); + + $this->assertNotNull($record); + $this->assertSame(0, $record->priority); + $this->assertSame('mail.example.com', $record->rdata); + } + + public function testExportAndImportKeepZeroSrvNumerics(): void + { + $zone = new Zone( + 'example.com', + [ + new Record('_sip._tcp.example.com', Record::TYPE_SRV, Record::CLASS_IN, 300, 'sip.example.com', priority: 0, weight: 0, port: 0), + ], + new Record('example.com', Record::TYPE_SOA, Record::CLASS_IN, 1800, 'ns1.example.com. admin.example.com. 2025011801 7200 3600 1209600 1800'), + ); + + $exported = File::export($zone, includeComments: false); + $this->assertStringContainsString("_sip._tcp\t300\tIN\tSRV\t0 0 0 sip\n", $exported); + + $roundTrip = File::import($exported); + $record = $this->findRecord($roundTrip->records, Record::TYPE_SRV); + + $this->assertNotNull($record); + $this->assertSame(0, $record->priority); + $this->assertSame(0, $record->weight); + $this->assertSame(0, $record->port); + $this->assertSame('sip.example.com', $record->rdata); + } + public function testImportSupportsPtrRecords(): void { $soa = self::DEFAULT_SOA;