Use the existence of an NS record to determine whether a domain is an apex domain #22
Conversation
Net::DNS was returning NS records it shouldn't have
stubbing the domain method fails to invalidate any of the memoized results
haileys
force-pushed
the
check-dns-for-apex-domains
branch
from
431ae3c to
2c9e2a5
Compare
|
In 2c9e2a5 I changed one of the specs to create a new Should I go and update the rest of the stubs to just create a new object instead? |
| return @apex_domain if defined?(@apex_domain) | ||
|
|
||
| answers = Resolv::DNS.open { |dns| | ||
| dns.getresources(absolute_domain, Resolv::DNS::Resource::IN::NS) |
There was a problem hiding this comment.
If I'm understanding correctly, since we already make a call to grab the DNS record, could this be simplified to use the dns method to save a call?
dns.any? { |answer| answer.class == Net::DNS::RR::NS }There was a problem hiding this comment.
Unfortunately the DNS lookup we're already doing doesn't fetch NS records, it just asks for an A record. I don't think there's a way to look up multiple record types (except an ANY request - but that's on its way out)
There was a problem hiding this comment.
Ahh. Makes sense. Last thought would be that we might be able to continue to use Net:Dns (if there's any advantage over Resolve (?) with something like:
Net::DNS::Resolver.start("google.com", Net::DNS::NS)You're the true Rubyist here, so trust your judgement on the dependency. Feel free to merge when you're ready. Thanks for tackling this. Gonna make many users 😸 .
There was a problem hiding this comment.
I went with Resolv because it was acting up on one of the test cases and returning records it shouldn't have:
>> Net::DNS::Resolver.start("blog.parkermoore.de", Net::DNS::NS).answer
=> [blog.parkermoore.de. 261 IN CNAME parkr.github.io., parkr.github.io. 3561 IN CNAME github.map.fastly.net.]
>> Resolv::DNS.open { |dns| dns.getresources("blog.parkermoore.de", Resolv::DNS::Resource::IN::NS) }
=> []But we can probably just filter non-NS records out in Rubyland
There was a problem hiding this comment.
Makes sense. I think Net::DNS just wraps resolve, and it's a core lib, so it's not like we're adding an external dependency. If resolve works more cleanly, lets use that. 👍.
If you don't mind, that'd be awesome. Good catch. |
|
Also, a dumb question, I know it doesn't make sense, but could I add an NS record to a subdomain? |
Yes, you can. But if you do, then you wouldn't want to have a CNAME for that subdomain either. For example, |
|
Ahh. @spraints thanks for dropping the :books. |
|
As soon as this merges, glad to cut and deploy a new gem with this and the Proxy checks from #21. |
|
@benbalter Is this ok to merge? I'll fix merge conflicts and merge if it looks good to you. |
👍 |
* master: (28 commits) travis: use Ruby 2.1.7 Upgrade to Ruby 2.1.7-github per https://pages.github.com/versions/ 💎 bump Memoize served_by_pages? == false Memoize dns = nil typo in comment 💎 bump actually check that a domains dns resolves 💎 bump add invalid_dns.rb to gemspec add timeout to DNS check add script/check 💎 bump require net-dns 0.8 add check for #26 💎 bump properly detect if domain is served by pages for non-200 response codes 💎 bump remove some diff noise use addressable::uri to support domains with underscores ...
Use the existence of an NS record to determine whether a domain is an apex domain
This will behave correctly even if a domain is not a listed public suffix. This could happen if a domain is a delegated zone, which can't have a CNAME record at the apex.
cc @benbalter @jdennes @spraints