Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read Authority field in Cloudflare/Google DNS response #56

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

stevefrench39
Copy link
Contributor

@stevefrench39 stevefrench39 commented Jul 8, 2024

For some DNS queries, such as type=NS when name=subdomain.example.com, the response from Cloudflare/Google contains an Authority field instead of Answer.

This change reads the Authority field if present in order to fix a bug with getAllDNSRecords("subdomain.example.com", { resolver: "cloudflare-dns" }). Currently, instead of returning the records associated with the subdomain, it incorrectly returns an empty array because it does not read the Authority field in the NS records response. As a result, nsRecords.length = 0 which fails the nsRecords.length check here, and no requests for additional records are made.

curl --location 'https://dns.google/resolve?name=subdomain.example.com&type=NS&cd=1' \
--header 'accept: application/dns-json'

{
    "Status": 3,
    "TC": false,
    "RD": true,
    "RA": true,
    "AD": false,
    "CD": true,
    "Question": [
        {
            "name": "subdomain.example.com.",
            "type": 2
        }
    ],
    "Authority": [
        {
            "name": "example.com.",
            "type": 6,
            "TTL": 1800,
            "data": "ns.icann.org. noc.dns.icann.org. 2024041842 7200 3600 1209600 3600"
        }
    ],
    "Comment": "Response from 199.43.135.53."
}

stevefrench39 and others added 2 commits July 8, 2024 14:48
For some DNS queries, such as type=NS when name=subdomain.example.com, the response contains an `Authority` field instead of `Answer`.
@AndreiIgna
Copy link
Member

Hey @stevefrench39 thanks for this. Is merged now, and will be released soon on npm and jsr.io

@AndreiIgna AndreiIgna merged commit e15e298 into LayeredStudio:main Jan 22, 2025
1 of 3 checks passed
@AndreiIgna
Copy link
Member

@stevefrench39 saw it in effect now, good addition 👍
Screenshot 2025-01-27 at 15 30 29

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