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

Collect all DNS sections by calling to each_resource instead of each_answer #3886

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ereslibre

ereslibre commented May 12, 2016

Disclaimer: same pull request on mruby impl (ruby/ruby#797), and bug open (https://bugs.ruby-lang.org/issues/12372). Not sure if I should report here a bug for the stdlib. For completeness:

A call to Message#each_answer does only include answers, however there are also two more kinds of
responses: Authority and Additional. Iterating through Message#each_resource allows us to retrieve
all information from the Message object.

This is specially important if, for example, we are trying to retrieve nameservers for a certain domain.

require 'resolv'
=> true
dns = Resolv::DNS.new(:nameserver => ['199.19.56.1']) # a0.org.afilias-nst.info
=> #<Resolv::DNS:0x3578436e ...>
puts dns.getresources('jruby.org', Resolv::DNS::Resource::IN::NS).map(&:name)
=> nil

nil is returned. This, in a terminal fashion is equivalent to:

~ > dig ns jruby.org @a0.org.afilias-nst.info
;; AUTHORITY SECTION:
jruby.org.      86400   IN  NS  ns25.domaincontrol.com.
jruby.org.      86400   IN  NS  ns26.domaincontrol.com.

There is indeed a response, but in the authority section, that is right now not collected by Resolv::DNS::getresource(s).

Collect all DNS sections by calling to each_resource instead of each_…
…answer.

When an authoritative server responds only on the authoritative section of the
DNS response, Resolv::DNS::getresource(s) interprets an empty response.
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 12, 2016

Member

@ereslibre Wow, thanks for the footwork on this one!

Our stdlib is a lightly-patched duplicate of MRI's, but we generally don't patch stdlib unilaterally, so we'll wait to see if MRI accepts this. When that happens, ping again and we'll be happy to merge this into JRuby.

One minor nit: there's unrelated whitespace changes in here. Can you eliminate those?

Member

headius commented May 12, 2016

@ereslibre Wow, thanks for the footwork on this one!

Our stdlib is a lightly-patched duplicate of MRI's, but we generally don't patch stdlib unilaterally, so we'll wait to see if MRI accepts this. When that happens, ping again and we'll be happy to merge this into JRuby.

One minor nit: there's unrelated whitespace changes in here. Can you eliminate those?

@ereslibre

This comment has been minimized.

Show comment
Hide comment
@ereslibre

ereslibre May 12, 2016

@headius Amazing timing. Of course, I will get rid of those space changes, sorry ! Thanks for your wonderful work !

ereslibre commented May 12, 2016

@headius Amazing timing. Of course, I will get rid of those space changes, sorry ! Thanks for your wonderful work !

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 19, 2016

Member

@ereslibre I pinged the MRI issue to see if we can get some movement.

Member

headius commented May 19, 2016

@ereslibre I pinged the MRI issue to see if we can get some movement.

@headius headius added this to the JRuby 9.1.2.0 milestone May 19, 2016

@ereslibre

This comment has been minimized.

Show comment
Hide comment
@ereslibre

ereslibre May 20, 2016

Thank you @headius ❤️

ereslibre commented May 20, 2016

Thank you @headius ❤️

@enebo enebo modified the milestones: JRuby 9.1.2.0, JRuby 9.1.3.0 May 25, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 24, 2016

Member

Doesn't look like MRI's going to get back to us in time for 9.1.3.0. If they don't respond for 9.1.4.0 and we're still happy with this change, we'll just merge it.

Member

headius commented Aug 24, 2016

Doesn't look like MRI's going to get back to us in time for 9.1.3.0. If they don't respond for 9.1.4.0 and we're still happy with this change, we'll just merge it.

@headius headius modified the milestones: JRuby 9.1.4.0, JRuby 9.1.3.0 Aug 24, 2016

@ereslibre

This comment has been minimized.

Show comment
Hide comment
@ereslibre

ereslibre Aug 25, 2016

Thank you again @headius.

ereslibre commented Aug 25, 2016

Thank you again @headius.

headius added a commit that referenced this pull request Nov 8, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 8, 2016

Member

MRI finally merged your fix! I've gone with their patch (yours sans whitespace changes) in 9187c5e.

Member

headius commented Nov 8, 2016

MRI finally merged your fix! I've gone with their patch (yours sans whitespace changes) in 9187c5e.

@headius headius closed this Nov 8, 2016

headius added a commit to jruby/ruby that referenced this pull request Nov 8, 2016

@ereslibre ereslibre deleted the ereslibre:resolv-ns-authoritative branch Jan 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment