Merge pull request #394 from redis/ipv6

Don't hardcode Ruby connection to IPv4

Author: pietern

lib/redis/connection/ruby.rb

@@ -114,11 +114,9 @@ class TCPSocket < ::Socket
 
         include SocketMixin
 
-        def self.connect(host, port, timeout)
-          # Limit lookup to IPv4, as Redis doesn't yet do IPv6...
-          addr = ::Socket.getaddrinfo(host, nil, Socket::AF_INET)
-          sock = new(::Socket.const_get(addr[0][0]), Socket::SOCK_STREAM, 0)
-          sockaddr = ::Socket.pack_sockaddr_in(port, addr[0][3])
+        def self.connect_addrinfo(ai, port, timeout)
+          sock = new(::Socket.const_get(ai[0]), Socket::SOCK_STREAM, 0)
+          sockaddr = ::Socket.pack_sockaddr_in(port, ai[3])
 
           begin
             sock.connect_nonblock(sockaddr)
@@ -135,6 +133,39 @@ def self.connect(host, port, timeout)
 
           sock
         end
+
+        def self.connect(host, port, timeout)
+          # Don't pass AI_ADDRCONFIG as flag to getaddrinfo(3)
+          #
+          # From the man page for getaddrinfo(3):
+          #
+          #   If hints.ai_flags includes the AI_ADDRCONFIG flag, then IPv4
+          #   addresses are returned in the list pointed to by res only if the
+          #   local system has at least one IPv4 address configured, and IPv6
+          #   addresses are returned only if the local system has at least one
+          #   IPv6 address configured. The loopback address is not considered
+          #   for this case as valid as a configured address.
+          #
+          # We do want the IPv6 loopback address to be returned if applicable,
+          # even if it is the only configured IPv6 address on the machine.
+          # Also see: https://github.com/redis/redis-rb/pull/394.
+          addrinfo = ::Socket.getaddrinfo(host, nil, Socket::AF_UNSPEC, Socket::SOCK_STREAM)
+
+          # From the man page for getaddrinfo(3):
+          #
+          #   Normally, the application should try using the addresses in the
+          #   order in which they are returned. The sorting function used
+          #   within getaddrinfo() is defined in RFC 3484 [...].
+          #
+          addrinfo.each_with_index do |ai, i|
+            begin
+              return connect_addrinfo(ai, port, timeout)
+            rescue SystemCallError
+              # Raise if this was our last attempt.
+              raise if addrinfo.length == i+1
+            end
+          end
+        end
       end
 
       class UNIXSocket < ::Socket

test/commands_on_value_types_test.rb

@@ -99,7 +99,7 @@ def test_flushall
 
   def test_migrate
     redis_mock(:migrate => lambda { |*args| args }) do |redis|
-      options = { :host => "localhost", :port => 1234 }
+      options = { :host => "127.0.0.1", :port => 1234 }
 
       assert_raise(RuntimeError, /host not specified/) do
         redis.migrate("foo", options.reject { |key, _| key == :host })
@@ -114,17 +114,17 @@ def test_migrate
 
       # Test defaults
       actual = redis.migrate("foo", options)
-      expected = ["localhost", "1234", "foo", default_db.to_s, default_timeout.to_s]
+      expected = ["127.0.0.1", "1234", "foo", default_db.to_s, default_timeout.to_s]
       assert_equal expected, actual
 
       # Test db override
       actual = redis.migrate("foo", options.merge(:db => default_db + 1))
-      expected = ["localhost", "1234", "foo", (default_db + 1).to_s, default_timeout.to_s]
+      expected = ["127.0.0.1", "1234", "foo", (default_db + 1).to_s, default_timeout.to_s]
       assert_equal expected, actual
 
       # Test timeout override
       actual = redis.migrate("foo", options.merge(:timeout => default_timeout + 1))
-      expected = ["localhost", "1234", "foo", default_db.to_s, (default_timeout + 1).to_s]
+      expected = ["127.0.0.1", "1234", "foo", default_db.to_s, (default_timeout + 1).to_s]
       assert_equal expected, actual
     end
   end

test/connection_handling_test.rb

@@ -157,7 +157,7 @@ def test_shutdown_with_error_from_multi_exec
 
   def test_slaveof
     redis_mock(:slaveof => lambda { |host, port| "+SLAVEOF #{host} #{port}" }) do |redis|
-      assert_equal "SLAVEOF localhost 6381", redis.slaveof("localhost", 6381)
+      assert_equal "SLAVEOF somehost 6381", redis.slaveof("somehost", 6381)
     end
   end
 

test/distributed_internals_test.rb

@@ -7,32 +7,32 @@ class TestDistributedInternals < Test::Unit::TestCase
   include Helper::Distributed
 
   def test_provides_a_meaningful_inspect
-    nodes = ["redis://localhost:#{PORT}/15", *NODES]
+    nodes = ["redis://127.0.0.1:#{PORT}/15", *NODES]
     redis = Redis::Distributed.new nodes
 
     assert_equal "#<Redis client v#{Redis::VERSION} for #{redis.nodes.map(&:id).join(', ')}>", redis.inspect
   end
 
   def test_default_as_urls
-    nodes = ["redis://localhost:#{PORT}/15", *NODES]
+    nodes = ["redis://127.0.0.1:#{PORT}/15", *NODES]
     redis = Redis::Distributed.new nodes
-    assert_equal ["redis://localhost:#{PORT}/15", *NODES], redis.nodes.map { |node| node.client.id}
+    assert_equal ["redis://127.0.0.1:#{PORT}/15", *NODES], redis.nodes.map { |node| node.client.id}
   end
 
   def test_default_as_config_hashes
-    nodes = [OPTIONS.merge(:host => 'localhost'), OPTIONS.merge(:host => 'localhost', :port => PORT.next)]
+    nodes = [OPTIONS.merge(:host => '127.0.0.1'), OPTIONS.merge(:host => 'somehost', :port => PORT.next)]
     redis = Redis::Distributed.new nodes
-    assert_equal ["redis://localhost:#{PORT}/15","redis://localhost:#{PORT.next}/15"], redis.nodes.map { |node| node.client.id }
+    assert_equal ["redis://127.0.0.1:#{PORT}/15","redis://somehost:#{PORT.next}/15"], redis.nodes.map { |node| node.client.id }
   end
 
   def test_as_mix_and_match
-    nodes = ["redis://localhost:7389/15", OPTIONS.merge(:host => 'localhost'), OPTIONS.merge(:host => 'localhost', :port => PORT.next)]
+    nodes = ["redis://127.0.0.1:7389/15", OPTIONS.merge(:host => 'somehost'), OPTIONS.merge(:host => 'somehost', :port => PORT.next)]
     redis = Redis::Distributed.new nodes
-    assert_equal ["redis://localhost:7389/15", "redis://localhost:#{PORT}/15", "redis://localhost:#{PORT.next}/15"], redis.nodes.map { |node| node.client.id }
+    assert_equal ["redis://127.0.0.1:7389/15", "redis://somehost:#{PORT}/15", "redis://somehost:#{PORT.next}/15"], redis.nodes.map { |node| node.client.id }
   end
 
   def test_override_id
-    nodes = [OPTIONS.merge(:host => 'localhost', :id => "test"), OPTIONS.merge( :host => 'localhost', :port => PORT.next, :id => "test1")]
+    nodes = [OPTIONS.merge(:host => '127.0.0.1', :id => "test"), OPTIONS.merge( :host => 'somehost', :port => PORT.next, :id => "test1")]
     redis = Redis::Distributed.new nodes
     assert_equal redis.nodes.first.client.id, "test"
     assert_equal redis.nodes.last.client.id,  "test1"

test/distributed_key_tags_test.rb

@@ -8,17 +8,17 @@ class TestDistributedKeyTags < Test::Unit::TestCase
   include Helper::Distributed
 
   def test_hashes_consistently
-    r1 = Redis::Distributed.new ["redis://localhost:#{PORT}/15", *NODES]
-    r2 = Redis::Distributed.new ["redis://localhost:#{PORT}/15", *NODES]
-    r3 = Redis::Distributed.new ["redis://localhost:#{PORT}/15", *NODES]
+    r1 = Redis::Distributed.new ["redis://127.0.0.1:#{PORT}/15", *NODES]
+    r2 = Redis::Distributed.new ["redis://127.0.0.1:#{PORT}/15", *NODES]
+    r3 = Redis::Distributed.new ["redis://127.0.0.1:#{PORT}/15", *NODES]
 
     assert_equal r1.node_for("foo").id, r2.node_for("foo").id
     assert_equal r1.node_for("foo").id, r3.node_for("foo").id
   end
 
   def test_allows_clustering_of_keys
     r = Redis::Distributed.new(NODES)
-    r.add_node("redis://localhost:#{PORT}/14")
+    r.add_node("redis://127.0.0.1:#{PORT}/14")
     r.flushdb
 
     100.times do |i|
@@ -29,7 +29,7 @@ def test_allows_clustering_of_keys
   end
 
   def test_distributes_keys_if_no_clustering_is_used
-    r.add_node("redis://localhost:#{PORT}/14")
+    r.add_node("redis://127.0.0.1:#{PORT}/14")
     r.flushdb
 
     r.set "users:1", 1
@@ -40,7 +40,7 @@ def test_distributes_keys_if_no_clustering_is_used
 
   def test_allows_passing_a_custom_tag_extractor
     r = Redis::Distributed.new(NODES, :tag => /^(.+?):/)
-    r.add_node("redis://localhost:#{PORT}/14")
+    r.add_node("redis://127.0.0.1:#{PORT}/14")
     r.flushdb
 
     100.times do |i|

test/distributed_test.rb

@@ -7,7 +7,7 @@ class TestDistributed < Test::Unit::TestCase
   include Helper::Distributed
 
   def test_handle_multiple_servers
-    @r = Redis::Distributed.new ["redis://localhost:#{PORT}/15", *NODES]
+    @r = Redis::Distributed.new ["redis://127.0.0.1:#{PORT}/15", *NODES]
 
     100.times do |idx|
       @r.set(idx.to_s, "foo#{idx}")
@@ -32,9 +32,9 @@ def test_add_nodes
     assert_equal 10, @r.nodes[0].client.timeout
     assert_equal logger, @r.nodes[0].client.logger
 
-    @r.add_node("redis://localhost:6380/14")
+    @r.add_node("redis://127.0.0.1:6380/14")
 
-    assert_equal "localhost", @r.nodes[1].client.host
+    assert_equal "127.0.0.1", @r.nodes[1].client.host
     assert_equal 6380, @r.nodes[1].client.port
     assert_equal 14, @r.nodes[1].client.db
     assert_equal 10, @r.nodes[1].client.timeout

test/helper.rb

@@ -151,13 +151,13 @@ def teardown
     end
 
     def redis_mock(commands, options = {}, &blk)
-      RedisMock.start(commands) do |port|
+      RedisMock.start(commands, options) do |port|
         yield _new_client(options.merge(:port => port))
       end
     end
 
     def redis_mock_with_handler(handler, options = {}, &blk)
-      RedisMock.start_with_handler(handler) do |port|
+      RedisMock.start_with_handler(handler, options) do |port|
         yield _new_client(options.merge(:port => port))
       end
     end

test/internals_test.rb

@@ -343,6 +343,62 @@ def test_does_not_change_self_client_options
     assert_equal "foo", redis.client.options[:scheme]
   end
 
+  def test_resolves_localhost
+    assert_nothing_raised do
+      Redis.new(OPTIONS.merge(:host => 'localhost')).ping
+    end
+  end
+
+  class << self
+    def af_family_supported(af)
+      hosts = {
+        Socket::AF_INET  => "127.0.0.1",
+        Socket::AF_INET6 => "::1",
+      }
+
+      begin
+        s = Socket.new(af, Socket::SOCK_STREAM, 0)
+        begin
+          sa = Socket.pack_sockaddr_in(9999, hosts[af])
+          s.bind(sa)
+          yield
+        rescue Errno::EADDRNOTAVAIL
+        ensure
+          s.close
+        end
+      rescue Errno::ESOCKTNOSUPPORT
+      end
+    end
+  end
+
+  def af_test(host)
+    commands = {
+      :ping => lambda { |*_| "+pong" },
+    }
+
+    redis_mock(commands, :host => host) do |redis|
+      assert_nothing_raised do
+        redis.ping
+      end
+    end
+  end
+
+  driver(:ruby) do
+    af_family_supported(Socket::AF_INET) do
+      def test_connect_ipv4
+        af_test("127.0.0.1")
+      end
+    end
+  end
+
+  driver(:ruby) do
+    af_family_supported(Socket::AF_INET6) do
+      def test_connect_ipv6
+        af_test("::1")
+      end
+    end
+  end
+
   def test_can_be_duped_to_create_a_new_connection
     clients = r.info["connected_clients"].to_i
 

test/publish_subscribe_test.rb

@@ -191,7 +191,7 @@ def test_unsubscribe_without_a_subscribe
   def test_subscribe_past_a_timeout
     # For some reason, a thread here doesn't reproduce the issue.
     sleep = %{sleep #{OPTIONS[:timeout] * 2}}
-    publish = %{echo "publish foo bar\r\n" | nc localhost #{OPTIONS[:port]}}
+    publish = %{echo "publish foo bar\r\n" | nc 127.0.0.1 #{OPTIONS[:port]}}
     cmd = [sleep, publish].join("; ")
 
     IO.popen(cmd, "r+") do |pipe|

test/support/redis_mock.rb

@@ -4,8 +4,8 @@ module RedisMock
   class Server
     VERBOSE = false
 
-    def initialize(port, &block)
-      @server = TCPServer.new("127.0.0.1", port)
+    def initialize(port, options = {}, &block)
+      @server = TCPServer.new(options[:host] || "127.0.0.1", port)
       @server.setsockopt(Socket::SOL_SOCKET,Socket::SO_REUSEADDR, true)
     end
 
@@ -53,8 +53,8 @@ def run
   #     # Every connection will be closed immediately
   #   end
   #
-  def self.start_with_handler(blk)
-    server = Server.new(MOCK_PORT)
+  def self.start_with_handler(blk, options = {})
+    server = Server.new(MOCK_PORT, options)
 
     begin
       server.start(&blk)
@@ -75,7 +75,7 @@ def self.start_with_handler(blk)
   #     assert_equal "PONG", Redis.new(:port => MOCK_PORT).ping
   #   end
   #
-  def self.start(commands = {}, &blk)
+  def self.start(commands, options = {}, &blk)
     handler = lambda do |session|
       while line = session.gets
         argv = Array.new(line[1..-3].to_i) do
@@ -110,6 +110,6 @@ def self.start(commands = {}, &blk)
       end
     end
 
-    start_with_handler(handler, &blk)
+    start_with_handler(handler, options, &blk)
   end
 end