Skip to content

Fix: Zone SOA answer#42

Merged
Meldiron merged 2 commits intomainfrom
fix-zone-soa-answer
Mar 27, 2026
Merged

Fix: Zone SOA answer#42
Meldiron merged 2 commits intomainfrom
fix-zone-soa-answer

Conversation

@Meldiron
Copy link
Copy Markdown
Contributor

@Meldiron Meldiron commented Mar 26, 2026

Fix senarios when SOA response is in authority section, instead of answer section.

We have record -> answer section
We dont have record -> authority section

This follows specs:

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR fixes a bug where SOA queries at the zone apex were not returning the SOA record in the answer section. Because the Zone class intentionally stores the SOA record separately (not in $records), the standard record-lookup flow could never match it; this caused SOA apex queries to incorrectly return NXDOMAIN (no records at all) or NODATA (SOA in the authority section only) depending on whether other records existed at the apex.

Key changes:

  • In Resolver::lookup(): an early-exit check is added before the NXDOMAIN return so that an SOA query for the zone apex name is answered with the SOA record even when no other apex records exist.
  • In Resolver::handleExactMatch(): an equivalent check is added at the top of the authoritative path so that an SOA query for the zone apex is answered correctly when other records (A, NS, AAAA, …) do exist at the apex.
  • Two new unit tests cover both cases (empty zone vs. zone with apex records), confirming both paths return RCODE_NOERROR with the SOA in the answer section and authoritative: true.

The only minor concern is that the two identical response-building blocks are duplicated; extracting them into a private helper would make future maintenance safer.

Confidence Score: 5/5

Safe to merge — the fix is correct, well-tested, and the only remaining note is a non-blocking style suggestion.

The bug fix is logically sound and covers both affected code paths. Both new test cases pass through distinct branches of the resolver (empty-records vs. exact-match), giving good coverage of the change. The Question and Zone constructors both normalize names to lowercase, so the $question->name === $zone->name comparison is safe. No existing behaviour is broken. The only open item is minor code duplication, which does not affect correctness or runtime safety.

No files require special attention.

Important Files Changed

Filename Overview
src/DNS/Zone/Resolver.php Adds SOA apex handling in two code paths (empty-records branch in lookup() and exact-match branch in handleExactMatch()); fixes a real bug where SOA queries at the zone apex returned NODATA instead of the SOA answer record.
tests/unit/DNS/Zone/ResolverTest.php Adds two new test cases covering both scenarios: SOA apex query with existing records and SOA apex query with an empty zone; tests are correct and thorough.

Reviews (1): Last reviewed commit: "Fix soa zone answer" | Re-trigger Greptile

@Meldiron Meldiron merged commit 917901e into main Mar 27, 2026
4 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