Skip to content

Allow binding the whois TCP socket to a specific IP address #40

Closed
wants to merge 1 commit into from

2 participants

@DerGuteMoritz

Hi Simone,

check out this commit and feel free to pull it from me if you like :-) Sorry for the brutal implementation using merge! but this was the easiest way to do it without having to change dozens of method signatures.

@DerGuteMoritz DerGuteMoritz allow passing options to server adapters and add the :bind_address op…
…tion to allow binding the TCP socket used for the whois request to a specific IP address
666f8e6
@weppos
Owner
weppos commented Nov 20, 2010

Hi Moritz,

I don't really like the idea of merging a patch with a comment

# for some reason, a few tests fail without the respond_to? check

I will investigate further the cause of the failure in the tests.

Also, could you please provide me an explanation/example of why you would need this feature?

@DerGuteMoritz

Hey Simone,

I don't really like the idea of merging a patch with a comment

you mean this new pull request style? Not sure, I think it works pretty alright!

Also, could you please provide me an explanation/example of why you would need this feature?

I needed it because some servers enforce an IP address based rate limit. This way I could bind to a different IP address when I ran into a limit.

@weppos
Owner
weppos commented Nov 23, 2010

you mean this new pull request style? Not sure, I think it works pretty alright!

No, I'm talking about the fact I don't want to merge a feature that includes a workaround that forces the test to not complain. If the tests complain, it's because the change possibly broke the interface.

Also, each feature needs to have a corresponding test so I need to create one before merging the changes.

I needed it because some servers enforce an IP address based rate limit. This way I could bind to a different IP address when I ran into a limit.

That makes sense. I'll look into the tests and try to integrate the feature.

Thanks for your contribution.

@DerGuteMoritz

No, I'm talking about the fact I don't want to merge a feature that includes a workaround that forces the test to not complain. If the tests complain, it's because the change possibly broke the interface.

Ah, I understand. Sorry for that, I was in a bit of a hurry when I needed this and when I couldn't figure out why some tests failed after a while (I guess it had to do with some setup code somewhere) I just settled for that work-around. I probably should have posponed the pull request accordingly.

Also, each feature needs to have a corresponding test so I need to create one before merging the changes.

Indeed. Maybe I'll have some time to look into it tomorrow!

@weppos
Owner
weppos commented Dec 13, 2010

I'm trying to incorporate the changes into the new release.

Just one question. Do you have any interesting documentation or use case about binding a local address to a TCPSocket in Ruby? I never had the change to do it before and, honestly, the available documentation really sucks! :S

@DerGuteMoritz

I agree, the documentation on all things related to Socket or IO is very thin. Binding to a local address works just like I do it in the patch, you pass it as a third argument to TCPSocket.open. Note that the host to connect to must be reachable from the address you bind to. So for example binding to localhost won't work for remote hosts, you will get an error like this: Errno::EINVAL: Invalid argument - bind(2).

Also, sorry for not providing any tests, yet :-(

@weppos
Owner
weppos commented Dec 20, 2010

Added ability to bind a WHOIS query to a specific local address and port (closed by 9ba4f16).

@weppos
Owner
weppos commented Dec 20, 2010

I just pushed the feature to the repository.

The implementation is a little bit different, compared to your patch.
Settings are not passed on the fly to the request, but are persistent for a Whois::Client.

You can bind a local host and/or a local port.

Here's an example

c = Whois::Client.new(:bind_host => "127.0.0.1")
c.query "first.com"
c.query "second.com"

Please give it a try and let me know if it works for you.

@weppos weppos added a commit that referenced this pull request Aug 17, 2011
@weppos Fixed `Errno::EINVAL: Invalid argument - bind(2)' error that occasion…
…ally might occur connecting to a WHOIS server.

This bug is related to issue #40. In fact, the cause of this bug is the workaround introduced in commit ffbbaff to solve the incompatibility with TCPSocket.new when resolv-replace.rb is required and you're trying to pass a nil value as :local_host argument to TCPSocket.new. The workaround was to always default the :local_host to "0.0.0.0", that obviously means localhost. Unfortunately,

This solution works quite well as long as there are not connectivity or server issues. In fact, when for whatever reason "0.0.0.0" can't connect to the remote host, you get an error `Errno::EINVAL: Invalid argument - bind(2)'. Because I don't wont to add extra-complexity when not required, I decided to revert to the original code and default :local_host and :local_port to nil when they are not present. But hey, an other issue is just around the corner!

It turns that resolv-replace.rb tries to resolve the :local_host even if it's nil (ruby bug?) because it checks the existence of any local parameter, regardless you are providing :local_port or local_host.

    class TCPSocket < IPSocket
      # :stopdoc:
      alias original_resolv_initialize initialize
      # :startdoc:
      def initialize(host, serv, *rest)
        rest[0] = IPSocket.getaddress(rest[0]) unless rest.empty?
        original_resolv_initialize(IPSocket.getaddress(host), serv, *rest)
      end
    end

While the following statement is perfectly fine with TCPSocket

    TCPSocket.new("whois.nic.it", 43, nil, nil)

it doesn't work when you require resolv-replace.rb.

    ArgumentError: cannot interpret as DNS name: nil
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:1121:in `create'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:967:in `generate_candidates'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:992:in `resolv'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:498:in `each_resource'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:391:in `each_address'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:115:in `block in each_address'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:114:in `each'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:114:in `each_address'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:92:in `getaddress'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv.rb:43:in `getaddress'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv-replace.rb:10:in `getaddress'
    	from /Users/weppos/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/resolv-replace.rb:22:in `initialize'

At this point, the only solution is to check the :local_host and :local_port parameter and avoid passing them if nil.
f64800b
@blatyo blatyo referenced this pull request in blatyo/page_rankr May 25, 2014
Closed

Can you add whois lookup? #32

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.