Both lookups in internal/resolver/iterative.go now go through queryZone. It asks the nameservers whose addresses a referral gives; only when none of them gives a usable reply does it look up and ask the others. resolveARecord (a nameserver's own address) used to give up on such a referral, and followDelegation (the walk to a name's nameservers) asked only the nameservers with addresses. maxLookupDepth (3) stops lookups started inside other lookups, so delegations that point at each other end; g.ntpns.org's address needs all three when anyns.pch.net gives its referral without addresses.
Not in the diff: a nameserver's saved reason says "lookups of nameserver addresses go too deep" when the limit is why its address was not found, and "no address for any nameserver of <zone>" only when the lookups found none.
Judgement call: "asking every nameserver it names" is read as "each can be asked in turn"; looking up every address at every referral multiplied queries to the root servers.
Judgement call: when a referral's given addresses fail and the limit stops looking up the others, the error names the limit, not those failures.
Test input: TestQueryZone_GivenAddressesFail gives 192.0.2.1 as the referral's address, because a live server cannot be made to fail.
Partly verified: no real pair of delegations that point at each other is tested; the tests show lookups stop past the limit.
Left as is: resolveNSIterative, which followDelegation falls back to, still uses only the addresses a referral gives; the issue does not name it.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/221
Both lookups in `internal/resolver/iterative.go` now go through `queryZone`. It asks the nameservers whose addresses a referral gives; only when none of them gives a usable reply does it look up and ask the others. `resolveARecord` (a nameserver's own address) used to give up on such a referral, and `followDelegation` (the walk to a name's nameservers) asked only the nameservers with addresses. `maxLookupDepth` (3) stops lookups started inside other lookups, so delegations that point at each other end; `g.ntpns.org`'s address needs all three when `anyns.pch.net` gives its referral without addresses.
Not in the diff: a nameserver's saved reason says "lookups of nameserver addresses go too deep" when the limit is why its address was not found, and "no address for any nameserver of <zone>" only when the lookups found none.
- Judgement call: "asking every nameserver it names" is read as "each can be asked in turn"; looking up every address at every referral multiplied queries to the root servers.
- Judgement call: when a referral's given addresses fail and the limit stops looking up the others, the error names the limit, not those failures.
- Test input: `TestQueryZone_GivenAddressesFail` gives 192.0.2.1 as the referral's address, because a live server cannot be made to fail.
- Partly verified: no real pair of delegations that point at each other is tested; the tests show lookups stop past the limit.
- Left as is: `resolveNSIterative`, which `followDelegation` falls back to, still uses only the addresses a referral gives; the issue does not name it.
Model: opus-5-5
g.ntpns.org. still fails at random, so pool.ntp.org is not reliably watched.g.ntpns.org is a zone of its own, which ntpns.org delegates to a.ntpns.org. to i.ntpns.org.. One of ntpns.org's six servers, anyns.pch.net, gives that referral without addresses, and it is the one picked about one time in six. Looking up g.ntpns.org's address then needs a.ntpns.org's address, which needs a bitnames.com server's address: three lookups one inside another, and maxLookupDepth in internal/resolver/iterative.go allows two. The nameserver is then saved with status error and the reason "no address for any nameserver of g.ntpns.org.", which is not true either. Acceptable: g.ntpns.org's address is found whichever ntpns.org server answers (for example a limit that allows this chain, with the README's "at most two deep" changed to match), covered by a live test that does not depend on which server answers.
The tests do not guard the walk to a name's nameservers, or the depth count. In followDelegation, dropping the nameservers that referralNameservers returns without addresses leaves the whole suite green. So does queryNameservers looking nameservers up at its own depth instead of one deeper, after which lookups never reach the limit. TestQueryNameservers_GivenAddressesFail calls queryNameservers with a hand-written list, not the walk that the definition of done names. Acceptable: a live test through the walk (for example LookupNS of a name whose parent zone's referral gives no addresses, such as g.ntpns.org, expecting its own nameservers), and a live test that fails when a lookup started inside another lookup does not count one level deeper.
Model: opus-5-5
1. **`g.ntpns.org.` still fails at random, so `pool.ntp.org` is not reliably watched.** `g.ntpns.org` is a zone of its own, which `ntpns.org` delegates to `a.ntpns.org.` to `i.ntpns.org.`. One of `ntpns.org`'s six servers, `anyns.pch.net`, gives that referral without addresses, and it is the one picked about one time in six. Looking up `g.ntpns.org`'s address then needs `a.ntpns.org`'s address, which needs a `bitnames.com` server's address: three lookups one inside another, and `maxLookupDepth` in `internal/resolver/iterative.go` allows two. The nameserver is then saved with status `error` and the reason "no address for any nameserver of g.ntpns.org.", which is not true either. Acceptable: `g.ntpns.org`'s address is found whichever `ntpns.org` server answers (for example a limit that allows this chain, with the README's "at most two deep" changed to match), covered by a live test that does not depend on which server answers.
2. **The tests do not guard the walk to a name's nameservers, or the depth count.** In `followDelegation`, dropping the nameservers that `referralNameservers` returns without addresses leaves the whole suite green. So does `queryNameservers` looking nameservers up at its own depth instead of one deeper, after which lookups never reach the limit. `TestQueryNameservers_GivenAddressesFail` calls `queryNameservers` with a hand-written list, not the walk that the definition of done names. Acceptable: a live test through the walk (for example `LookupNS` of a name whose parent zone's referral gives no addresses, such as `g.ntpns.org`, expecting its own nameservers), and a live test that fails when a lookup started inside another lookup does not count one level deeper.
Model: opus-5-5
maxLookupDepth is 3 and the README says "at most three deep"; TestQueryNameservers_LookupDepth now finds g.ntpns.org's address from where anyns.pch.net's referral without addresses leaves its lookup, whichever server answers.
TestLookupNS_ParentZoneDelegatedWithoutAddresses expects a.ntpns.org. among LookupNS("g.ntpns.org"), which fails when followDelegation drops the nameservers without addresses; TestQueryNameservers_LookupDepth expects no answer when started one below the limit, which fails when a lookup started inside another is not counted one level deeper.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/dnswatcher/pulls/242#issuecomment-112330:
1. `maxLookupDepth` is 3 and the README says "at most three deep"; `TestQueryNameservers_LookupDepth` now finds `g.ntpns.org`'s address from where `anyns.pch.net`'s referral without addresses leaves its lookup, whichever server answers.
2. `TestLookupNS_ParentZoneDelegatedWithoutAddresses` expects `a.ntpns.org.` among `LookupNS("g.ntpns.org")`, which fails when `followDelegation` drops the nameservers without addresses; `TestQueryNameservers_LookupDepth` expects no answer when started one below the limit, which fails when a lookup started inside another is not counted one level deeper.
Model: opus-5-5
No test covers the path the watcher uses to reach a nameserver whose zone is delegated without addresses. The watcher looks up a nameserver's address through queryNameserver in internal/resolver/iterative.go, which starts that lookup at depth 1. The new tests reach the lookup only through the test exports ResolveNSIPs and QueryNameservers, which pass their own starting depth. So a change to how queryNameserver starts the lookup could put every nameserver of pool.ntp.org back to status error while every test passes. Acceptable: a live test through QueryNameserver (for example with liveQueryNameserver) asking a.ntpns.org. about pool.ntp.org, expecting status ok.
When the depth limit stops a lookup, the saved reason is untrue.queryNameservers then returns "no address for any nameserver of <zone>", although those nameservers have addresses; they were not looked up because of the limit. The issue counts this kind of wrong reason as part of the defect. Acceptable: when the limit is why nothing was asked, the error says so (for example its own error, which TestQueryNameservers_LookupDepth checks for). Keep "no address" for when the lookups found none.
queryNameservers differs by one letter from queryNameserver, which does something else. In internal/resolver/iterative.go, queryNameserver gets the records of one named nameserver. The new queryNameservers asks the servers of a zone until one gives a usable reply, looking up addresses when it has to. The test export QueryNameservers sits next to the public QueryNameserver in the same way, and a reader takes one for the plural of the other. Acceptable: a name that says what the function does and cannot be read as the plural of queryNameserver.
Model: opus-5-5
1. **No test covers the path the watcher uses to reach a nameserver whose zone is delegated without addresses.** The watcher looks up a nameserver's address through `queryNameserver` in `internal/resolver/iterative.go`, which starts that lookup at depth 1. The new tests reach the lookup only through the test exports `ResolveNSIPs` and `QueryNameservers`, which pass their own starting depth. So a change to how `queryNameserver` starts the lookup could put every nameserver of `pool.ntp.org` back to status `error` while every test passes. Acceptable: a live test through `QueryNameserver` (for example with `liveQueryNameserver`) asking `a.ntpns.org.` about `pool.ntp.org`, expecting status `ok`.
2. **When the depth limit stops a lookup, the saved reason is untrue.** `queryNameservers` then returns "no address for any nameserver of <zone>", although those nameservers have addresses; they were not looked up because of the limit. The issue counts this kind of wrong reason as part of the defect. Acceptable: when the limit is why nothing was asked, the error says so (for example its own error, which `TestQueryNameservers_LookupDepth` checks for). Keep "no address" for when the lookups found none.
3. **`queryNameservers` differs by one letter from `queryNameserver`, which does something else.** In `internal/resolver/iterative.go`, `queryNameserver` gets the records of one named nameserver. The new `queryNameservers` asks the servers of a zone until one gives a usable reply, looking up addresses when it has to. The test export `QueryNameservers` sits next to the public `QueryNameserver` in the same way, and a reader takes one for the plural of the other. Acceptable: a name that says what the function does and cannot be read as the plural of `queryNameserver`.
Model: opus-5-5
TestQueryNameserver_ZoneDelegatedWithoutAddresses asks a.ntpns.org. about pool.ntp.org through QueryNameserver and expects status ok; it fails when queryNameserver starts the lookup at the limit.
The new ErrLookupDepthExceeded is returned, and passed up through the lookups that started it, when the limit is why no address was found; TestQueryZone_LookupDepth checks for it, and "no address" is kept for when the lookups found none.
queryNameservers and its test export are now queryZone and QueryZone, and their tests TestQueryZone_*.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/dnswatcher/pulls/242#issuecomment-112516:
1. `TestQueryNameserver_ZoneDelegatedWithoutAddresses` asks `a.ntpns.org.` about `pool.ntp.org` through `QueryNameserver` and expects status `ok`; it fails when `queryNameserver` starts the lookup at the limit.
2. The new `ErrLookupDepthExceeded` is returned, and passed up through the lookups that started it, when the limit is why no address was found; `TestQueryZone_LookupDepth` checks for it, and "no address" is kept for when the lookups found none.
3. `queryNameservers` and its test export are now `queryZone` and `QueryZone`, and their tests `TestQueryZone_*`.
Model: opus-5-5
Looking up a nameserver's own address followed only the addresses a
referral gave, so a nameserver whose zone is delegated without them,
such as a.ntpns.org of pool.ntp.org, never resolved. The walk to a
name's nameservers looked addresses up only when a referral gave none.
Both now go through queryZone, which asks the nameservers whose
addresses the referral gives first and, if none of them gives a usable
reply, looks up and asks the others. maxLookupDepth stops lookups three
deep, so delegations that point at each other still end; when the limit
is why no address was found, the error is ErrLookupDepthExceeded, not
"no address".
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #221
Both lookups in
internal/resolver/iterative.gonow go throughqueryZone. It asks the nameservers whose addresses a referral gives; only when none of them gives a usable reply does it look up and ask the others.resolveARecord(a nameserver's own address) used to give up on such a referral, andfollowDelegation(the walk to a name's nameservers) asked only the nameservers with addresses.maxLookupDepth(3) stops lookups started inside other lookups, so delegations that point at each other end;g.ntpns.org's address needs all three whenanyns.pch.netgives its referral without addresses.Not in the diff: a nameserver's saved reason says "lookups of nameserver addresses go too deep" when the limit is why its address was not found, and "no address for any nameserver of <zone>" only when the lookups found none.
TestQueryZone_GivenAddressesFailgives 192.0.2.1 as the referral's address, because a live server cannot be made to fail.resolveNSIterative, whichfollowDelegationfalls back to, still uses only the addresses a referral gives; the issue does not name it.Model: opus-5-5
g.ntpns.org.still fails at random, sopool.ntp.orgis not reliably watched.g.ntpns.orgis a zone of its own, whichntpns.orgdelegates toa.ntpns.org.toi.ntpns.org.. One ofntpns.org's six servers,anyns.pch.net, gives that referral without addresses, and it is the one picked about one time in six. Looking upg.ntpns.org's address then needsa.ntpns.org's address, which needs abitnames.comserver's address: three lookups one inside another, andmaxLookupDepthininternal/resolver/iterative.goallows two. The nameserver is then saved with statuserrorand the reason "no address for any nameserver of g.ntpns.org.", which is not true either. Acceptable:g.ntpns.org's address is found whicheverntpns.orgserver answers (for example a limit that allows this chain, with the README's "at most two deep" changed to match), covered by a live test that does not depend on which server answers.The tests do not guard the walk to a name's nameservers, or the depth count. In
followDelegation, dropping the nameservers thatreferralNameserversreturns without addresses leaves the whole suite green. So doesqueryNameserverslooking nameservers up at its own depth instead of one deeper, after which lookups never reach the limit.TestQueryNameservers_GivenAddressesFailcallsqueryNameserverswith a hand-written list, not the walk that the definition of done names. Acceptable: a live test through the walk (for exampleLookupNSof a name whose parent zone's referral gives no addresses, such asg.ntpns.org, expecting its own nameservers), and a live test that fails when a lookup started inside another lookup does not count one level deeper.Model: opus-5-5
031e595616to661ef8d937Rework for #242 (comment):
maxLookupDepthis 3 and the README says "at most three deep";TestQueryNameservers_LookupDepthnow findsg.ntpns.org's address from whereanyns.pch.net's referral without addresses leaves its lookup, whichever server answers.TestLookupNS_ParentZoneDelegatedWithoutAddressesexpectsa.ntpns.org.amongLookupNS("g.ntpns.org"), which fails whenfollowDelegationdrops the nameservers without addresses;TestQueryNameservers_LookupDepthexpects no answer when started one below the limit, which fails when a lookup started inside another is not counted one level deeper.Model: opus-5-5
No test covers the path the watcher uses to reach a nameserver whose zone is delegated without addresses. The watcher looks up a nameserver's address through
queryNameserverininternal/resolver/iterative.go, which starts that lookup at depth 1. The new tests reach the lookup only through the test exportsResolveNSIPsandQueryNameservers, which pass their own starting depth. So a change to howqueryNameserverstarts the lookup could put every nameserver ofpool.ntp.orgback to statuserrorwhile every test passes. Acceptable: a live test throughQueryNameserver(for example withliveQueryNameserver) askinga.ntpns.org.aboutpool.ntp.org, expecting statusok.When the depth limit stops a lookup, the saved reason is untrue.
queryNameserversthen returns "no address for any nameserver of <zone>", although those nameservers have addresses; they were not looked up because of the limit. The issue counts this kind of wrong reason as part of the defect. Acceptable: when the limit is why nothing was asked, the error says so (for example its own error, whichTestQueryNameservers_LookupDepthchecks for). Keep "no address" for when the lookups found none.queryNameserversdiffers by one letter fromqueryNameserver, which does something else. Ininternal/resolver/iterative.go,queryNameservergets the records of one named nameserver. The newqueryNameserversasks the servers of a zone until one gives a usable reply, looking up addresses when it has to. The test exportQueryNameserverssits next to the publicQueryNameserverin the same way, and a reader takes one for the plural of the other. Acceptable: a name that says what the function does and cannot be read as the plural ofqueryNameserver.Model: opus-5-5
661ef8d937tod0ac850407Rework for #242 (comment):
TestQueryNameserver_ZoneDelegatedWithoutAddressesasksa.ntpns.org.aboutpool.ntp.orgthroughQueryNameserverand expects statusok; it fails whenqueryNameserverstarts the lookup at the limit.ErrLookupDepthExceededis returned, and passed up through the lookups that started it, when the limit is why no address was found;TestQueryZone_LookupDepthchecks for it, and "no address" is kept for when the lookups found none.queryNameserversand its test export are nowqueryZoneandQueryZone, and their testsTestQueryZone_*.Model: opus-5-5
Review passed on
d0ac850.Model: opus-5-5
d0ac850407toa01588f7f4