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

Use ipaddr to compare the sender's address #4496

Merged
merged 1 commit into from
Mar 1, 2017

Conversation

nbarrientos
Copy link
Contributor

When resolving names using compressed IPv6-only DNS servers, it could
happen that when processing the response of a name query, the IP of the
sender is not compressed so using a simple string comparison is not
safe.

This patch encapsulates the from address and the address of the sender in
IPAddr objects so the comparison is safe and just works in all cases.

This fixes #3663.

When resolving names using compressed IPv6-only DNS servers, it could
happen that when processing the response of a name query, the IP of the
sender is not compressed so using a simple string comparison is not
safe.

This patch encapsulates the from address and the address of the sender in
IPAddr objects so the comparison is safe and just works in all cases.

This fixes jruby#3663.
@headius
Copy link
Member

headius commented Feb 23, 2017

This looks fine to me, and we'll probably merge it, but I have a few concerns.

We normally try to avoid patching the standard library if we can help it. Especially like this, where a new dependency is being introduced. But perhaps MRI has this problem too when running in an IPv6 environment?

I don't have any IPv6 setup right now. Any chance you can try to reproduce with MRI?

@headius headius added this to the JRuby 9.1.8.0 milestone Feb 23, 2017
@nbarrientos
Copy link
Contributor Author

Hi,

Thanks for looking at the patch.

I could not reproduce the problem with MRI, see this comment. But anyway:

# grep nameserver /etc/resolv.conf
nameserver 2001:1458:201:1100::5
nameserver 2001:1458:201:1100::5
# ruby -v
ruby 2.0.0p648 (2015-12-16) [x86_64-linux]
# time ruby -e "require 'resolv'; puts Resolv.getaddress 'web.cern.ch'"
188.184.9.235

real	0m0.083s
user	0m0.061s
sys	0m0.021s

Same machine, same environment, but on top of jRuby:

# jruby -v
jruby 9.1.7.0 (2.3.1) 2017-01-11 68056ae OpenJDK 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [linux-x86_64]
# time jruby -e "require 'resolv'; puts Resolv.getaddress 'web.cern.ch'"
Resolv::ResolvError: no address for web.cern.ch
  getaddress at /root/jruby-9.1.7.0/lib/ruby/stdlib/resolv.rb:100
  getaddress at /root/jruby-9.1.7.0/lib/ruby/stdlib/resolv.rb:50
      <main> at -e:1

real	2m45.023s
user	0m5.508s
sys	0m0.343s
#

I'm happy to perform other tests with MRI, just let me know :)

Do you see feasible to backport the patch to 1.7? That'd be awesome!

@headius
Copy link
Member

headius commented Feb 23, 2017

Ok, bummer that MRI works ok here but I think we can move forward with this patch for now.

@enebo Any objections to this patch other than what I mentioned above? It would at least get people on their feet while we investigate the root cause.

@enebo
Copy link
Member

enebo commented Feb 23, 2017

@headius I think if we can open up a new issue to figure out why we are different and close the issue around this PR we can land it. Then it will work as a workaround until we dig deeper. I would rather we release sooner than later...and hey it will not be our first modification to stdlib files :|

I would like to close the original report so we can display this is now working in the release in our notes (but we can link to it)

@headius
Copy link
Member

headius commented Mar 1, 2017

@enebo Agreed.

@nbarrientos
Copy link
Contributor Author

Thanks. Any chance to see it backported to 1.7?

headius added a commit to jruby/ruby that referenced this pull request Oct 28, 2019
This patch is from jruby/jruby#5722, which fixes some missed logic
in the patch from jruby/jruby#4496.
@headius headius mentioned this pull request Dec 10, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Never-ending getaddress() call when using compressed IPv6 nameservers in /etc/resolv.conf
4 participants