Skip to content

Add DNS record RDATA validation#48

Merged
ChiragAgg5k merged 1 commit into
mainfrom
fix/validate-dns-rdata
May 20, 2026
Merged

Add DNS record RDATA validation#48
ChiragAgg5k merged 1 commit into
mainfrom
fix/validate-dns-rdata

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

What

Adds reusable DNS record RDATA validation so callers can validate answers, authority, and additional records without serializing a full DNS message.

Why

Invalid RDATA such as a hostname in an A record (ns2.appwrite.zone) currently surfaces only while encoding. Exposing validation at the Record and Message level lets DNS services detect malformed records earlier and return an appropriate failure response.

Changes

  • Added Record::validateRdata() for record-type-specific RDATA checks.
  • Added Message::validate() to validate answer, authority, and additional sections.
  • Kept encodeRdata() aligned by calling validateRdata() before encoding.
  • Added regression coverage for invalid A-record RDATA and valid NS hostname RDATA.

Testing

  • composer test -- --filter 'RecordTest|MessageTest'
  • composer format:check
  • ./vendor/bin/phpstan analyse --level max -c phpstan.neon src tests --memory-limit=1G

Full composer test was also run and only failed in localhost DNS client e2e tests because nothing was listening on 127.0.0.1:5300.

@ChiragAgg5k ChiragAgg5k force-pushed the fix/validate-dns-rdata branch from 4165bb7 to fb06d68 Compare May 20, 2026 04:08
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR introduces Record::validateRdata() and Message::validate() to expose RDATA validation without requiring a full encode, allowing callers to surface malformed records (e.g. a hostname in an A record) before serialisation.

  • Record::validateRdata() is a thin public wrapper over the existing private encodeRdata(''); it works correctly for all current record types, though the empty-packet argument creates an implicit assumption that no record type uses packet context during encoding.
  • Message::validate() iterates answers, authority, and additional sections and delegates to validateRdata() on each record, stopping at the first invalid record.
  • Tests use a DataProvider to verify that each of the three sections is independently exercised, addressing the coverage gap noted in the previous review.

Confidence Score: 5/5

Safe to merge — new public surface is additive, existing encoding paths are unchanged, and the decode→encode round-trip for all record types (including unknown types stored as hex) remains intact.

The change adds two small, well-tested methods with no side effects on existing behaviour. All current record types are correctly handled by the empty-packet call in validateRdata(). The data-provider test covers all three message sections independently.

No files require special attention.

Important Files Changed

Filename Overview
src/DNS/Message/Record.php Adds public validateRdata() that delegates to the private encodeRdata(''); the empty-packet argument is safe for all current record types but creates a silent contract that future types using packet context for compression would violate.
src/DNS/Message.php Adds Message::validate() that iterates answers, authority, and additional sections calling validateRdata() on each record; correct and straightforward.
tests/unit/DNS/Message/RecordTest.php Adds two new tests — one rejection test for a hostname in an A record, and one acceptance test for a hostname in an NS record; the latter uses addToAssertionCount(1) instead of the idiomatic expectNotToPerformAssertions().
tests/unit/DNS/MessageTest.php Adds a data-provider-driven test covering the answers, authority, and additional sections of Message::validate() — each section is exercised as a separate test case.

Reviews (3): Last reviewed commit: "Add DNS record rdata validation" | Re-trigger Greptile

Comment thread tests/unit/DNS/MessageTest.php
Comment thread src/DNS/Message/Record.php
@ChiragAgg5k ChiragAgg5k force-pushed the fix/validate-dns-rdata branch from fb06d68 to 75a075b Compare May 20, 2026 04:13
@ChiragAgg5k ChiragAgg5k force-pushed the fix/validate-dns-rdata branch from 75a075b to b2b8d07 Compare May 20, 2026 04:16
@ChiragAgg5k ChiragAgg5k merged commit 5225f52 into main May 20, 2026
5 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix/validate-dns-rdata branch May 20, 2026 04:49
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.

2 participants