Tidy up IPv6 handling. (#63)

* Remove normalization and prefer `.build`.

* Add release notes.

Author: ioquatix

lib/async/redis/endpoint.rb

@@ -19,14 +19,15 @@ def self.local_endpoint(**options)
 
 		# Represents a way to connect to a remote Redis server.
 		class Endpoint < ::IO::Endpoint::Generic
-			LOCALHOST = URI.parse("redis://localhost").freeze
+			LOCALHOST = URI::Generic.build(scheme: "redis", host: "localhost").freeze
 
 			def self.local(**options)
 				self.new(LOCALHOST, **options)
 			end
 
 			def self.remote(host, port = 6379, **options)
-				self.new(URI.parse("redis://#{host}:#{port}"), **options)
+				# URI::Generic.build automatically handles IPv6 addresses correctly:
+				self.new(URI::Generic.build(scheme: "redis", host: host, port: port), **options)
 			end
 
 			SCHEMES = {
@@ -35,7 +36,7 @@ def self.remote(host, port = 6379, **options)
 			}
 
 			def self.parse(string, endpoint = nil, **options)
-				url = URI.parse(string).normalize
+				url = URI.parse(string)
 
 				return self.new(url, endpoint, **options)
 			end
@@ -45,7 +46,7 @@ def self.parse(string, endpoint = nil, **options)
 			# @parameter scheme [String] The scheme to use, e.g. "redis" or "rediss".
 			# @parameter hostname [String] The hostname to connect to (or bind to).
 			# @parameter options [Hash] Additional options, passed to {#initialize}.
-			def self.for(scheme, hostname, credentials: nil, port: nil, database: nil, **options)
+			def self.for(scheme, host, credentials: nil, port: nil, database: nil, **options)
 				uri_klass = SCHEMES.fetch(scheme.downcase) do
 					raise ArgumentError, "Unsupported scheme: #{scheme.inspect}"
 				end
@@ -55,7 +56,13 @@ def self.for(scheme, hostname, credentials: nil, port: nil, database: nil, **opt
 				end
 
 				self.new(
-					uri_klass.new(scheme, credentials&.join(":"), hostname, port, nil, path, nil, nil, nil).normalize,
+					uri_klass.build(
+						scheme: scheme,
+						userinfo: credentials&.join(":"),
+						host: host,
+						port: port,
+						path: path,
+					),
 					**options
 				)
 			end

releases.md

@@ -1,5 +1,9 @@
 # Releases
 
+## Unreleased
+
+  - Fix handling of IPv6 address literals, including those returned by Redis Cluster / Sentinel.
+
 ## v0.11.1
 
   - Correctly pass `@options` to `Async::Redis::Client` instances created by `Async::Redis::ClusterClient`.

test/async/redis/endpoint.rb

@@ -53,4 +53,94 @@
 			end
 		end
 	end
+	
+	with ".remote" do
+		it "handles IPv4 addresses correctly" do
+			endpoint = Async::Redis::Endpoint.remote("127.0.0.1", 6380)
+			expect(endpoint.url.to_s).to be == "redis://127.0.0.1:6380"
+			expect(endpoint.url.host).to be == "127.0.0.1"
+			expect(endpoint.url.hostname).to be == "127.0.0.1"
+		end
+		
+		it "handles IPv6 addresses correctly" do
+			endpoint = Async::Redis::Endpoint.remote("::1", 6380)
+			expect(endpoint.url.to_s).to be == "redis://[::1]:6380"
+			expect(endpoint.url.host).to be == "[::1]"
+			expect(endpoint.url.hostname).to be == "::1"
+		end
+		
+		it "handles expanded IPv6 addresses correctly" do
+			ipv6 = "2600:1f28:372:c404:5c2d:ce68:3620:cc4b"
+			endpoint = Async::Redis::Endpoint.remote(ipv6, 6380)
+			expect(endpoint.url.to_s).to be == "redis://[#{ipv6}]:6380"
+			expect(endpoint.url.host).to be == "[#{ipv6}]"
+			expect(endpoint.url.hostname).to be == ipv6
+		end
+	end
+	
+	with ".for" do
+		it "handles IPv4 addresses correctly" do
+			endpoint = Async::Redis::Endpoint.for("redis", "127.0.0.1", port: 6380)
+			expect(endpoint.url.to_s).to be == "redis://127.0.0.1:6380"
+			expect(endpoint.url.host).to be == "127.0.0.1"
+			expect(endpoint.url.hostname).to be == "127.0.0.1"
+			expect(endpoint.port).to be == 6380
+		end
+		
+		it "handles IPv6 addresses correctly" do
+			endpoint = Async::Redis::Endpoint.for("redis", "::1", port: 6380)
+			expect(endpoint.url.to_s).to be == "redis://[::1]:6380"
+			expect(endpoint.url.host).to be == "[::1]"
+			expect(endpoint.url.hostname).to be == "::1"
+			expect(endpoint.port).to be == 6380
+		end
+		
+		it "handles expanded IPv6 addresses correctly" do
+			ipv6 = "2600:1f28:372:c404:5c2d:ce68:3620:cc4b"
+			endpoint = Async::Redis::Endpoint.for("redis", ipv6, port: 6380)
+			expect(endpoint.url.to_s).to be == "redis://[#{ipv6}]:6380"
+			expect(endpoint.url.host).to be == "[#{ipv6}]"
+			expect(endpoint.url.hostname).to be == ipv6
+			expect(endpoint.port).to be == 6380
+		end
+		
+		it "handles credentials correctly" do
+			endpoint = Async::Redis::Endpoint.for("redis", "localhost", credentials: ["user", "pass"], port: 6380)
+			expect(endpoint.url.to_s).to be == "redis://user:pass@localhost:6380"
+			expect(endpoint.url.userinfo).to be == "user:pass"
+			expect(endpoint.credentials).to be == ["user", "pass"]
+		end
+		
+		it "handles database selection correctly" do
+			endpoint = Async::Redis::Endpoint.for("redis", "localhost", database: 2)
+			expect(endpoint.url.to_s).to be == "redis://localhost/2"
+			expect(endpoint.url.path).to be == "/2"
+			expect(endpoint.database).to be == 2
+		end
+		
+		it "handles secure connections correctly" do
+			endpoint = Async::Redis::Endpoint.for("rediss", "localhost")
+			expect(endpoint.url.to_s).to be == "rediss://localhost"
+			expect(endpoint).to be(:secure?)
+		end
+		
+		it "handles all parameters together correctly" do
+			ipv6 = "2600:1f28:372:c404:5c2d:ce68:3620:cc4b"
+			endpoint = Async::Redis::Endpoint.for("rediss", ipv6, 
+				credentials: ["user", "pass"], 
+				port: 6380, 
+				database: 3
+			)
+			expect(endpoint.url.to_s).to be == "rediss://user:pass@[#{ipv6}]:6380/3"
+			expect(endpoint.url.scheme).to be == "rediss"
+			expect(endpoint.url.host).to be == "[#{ipv6}]"
+			expect(endpoint.url.hostname).to be == ipv6
+			expect(endpoint.url.userinfo).to be == "user:pass"
+			expect(endpoint.url.port).to be == 6380
+			expect(endpoint.url.path).to be == "/3"
+			expect(endpoint).to be(:secure?)
+			expect(endpoint.credentials).to be == ["user", "pass"]
+			expect(endpoint.database).to be == 3
+		end
+	end
 end