From 6329ff032377fed07d7e92cff80ec4f69f5925b4 Mon Sep 17 00:00:00 2001 From: jeraki <83373294+jeraki@users.noreply.github.com> Date: Wed, 30 Nov 2022 15:29:25 -0800 Subject: [PATCH 1/4] Strip brackets from IPv6 addresses before passing them to a socket. Fixes #730. --- lib/http/timeout/global.rb | 1 + lib/http/timeout/null.rb | 9 +++++++++ lib/http/timeout/per_operation.rb | 1 + spec/lib/http/timeout/global_spec.rb | 16 ++++++++++++++++ spec/lib/http/timeout/null_spec.rb | 14 ++++++++++++++ spec/lib/http/timeout/per_operation_spec.rb | 14 ++++++++++++++ 6 files changed, 55 insertions(+) create mode 100644 spec/lib/http/timeout/global_spec.rb create mode 100644 spec/lib/http/timeout/null_spec.rb create mode 100644 spec/lib/http/timeout/per_operation_spec.rb diff --git a/lib/http/timeout/global.rb b/lib/http/timeout/global.rb index 1148aef9..7517bc41 100644 --- a/lib/http/timeout/global.rb +++ b/lib/http/timeout/global.rb @@ -22,6 +22,7 @@ def reset_counter def connect(socket_class, host, port, nodelay = false) reset_timer ::Timeout.timeout(@time_left, ConnectTimeoutError) do + host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end diff --git a/lib/http/timeout/null.rb b/lib/http/timeout/null.rb index a5ee19ca..bf6c571f 100644 --- a/lib/http/timeout/null.rb +++ b/lib/http/timeout/null.rb @@ -18,6 +18,7 @@ def initialize(options = {}) # Connects to a socket def connect(socket_class, host, port, nodelay = false) + host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end @@ -71,6 +72,14 @@ def rescue_writable(timeout = write_timeout) retry if @socket.to_io.wait_writable(timeout) raise TimeoutError, "Write timed out after #{timeout} seconds" end + + # Strip brackets from IPv6 addresses + def strip_ipv6_brackets(host) + match_data = /\[(.+)\]/.match(host) + return host if match_data.nil? + + match_data[1] + end end end end diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 9274325e..455795ac 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -21,6 +21,7 @@ def initialize(*args) def connect(socket_class, host, port, nodelay = false) ::Timeout.timeout(@connect_timeout, ConnectTimeoutError) do + host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end diff --git a/spec/lib/http/timeout/global_spec.rb b/spec/lib/http/timeout/global_spec.rb new file mode 100644 index 00000000..cf2af41f --- /dev/null +++ b/spec/lib/http/timeout/global_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::Timeout::Global do + subject { described_class.new(:global_timeout => 1) } + + let(:socket_class) { double("SocketClass", :open => socket) } + let(:socket) { double("Socket", :setsockopt => nil) } + + describe "#connect" do + it "strips brackets from IPv6 addresses before sending them to the socket" do + expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) + + subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) + end + end +end diff --git a/spec/lib/http/timeout/null_spec.rb b/spec/lib/http/timeout/null_spec.rb new file mode 100644 index 00000000..6d393c02 --- /dev/null +++ b/spec/lib/http/timeout/null_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::Timeout::Null do + let(:socket_class) { double("SocketClass", :open => socket) } + let(:socket) { double("Socket", :setsockopt => nil) } + + describe "#connect" do + it "strips brackets from IPv6 addresses before sending them to the socket" do + expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) + + subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) + end + end +end diff --git a/spec/lib/http/timeout/per_operation_spec.rb b/spec/lib/http/timeout/per_operation_spec.rb new file mode 100644 index 00000000..9f2d41ec --- /dev/null +++ b/spec/lib/http/timeout/per_operation_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::Timeout::PerOperation do + let(:socket_class) { double("SocketClass", :open => socket) } + let(:socket) { double("Socket", :setsockopt => nil) } + + describe "#connect" do + it "strips brackets from IPv6 addresses before sending them to the socket" do + expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) + + subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) + end + end +end From 1a61644017effe943c12314505f6fada92da7613 Mon Sep 17 00:00:00 2001 From: jeraki <83373294+jeraki@users.noreply.github.com> Date: Thu, 1 Dec 2022 17:24:46 -0800 Subject: [PATCH 2/4] Move IPv6 bracket stripping into HTTP::URI. --- lib/http/timeout/global.rb | 1 - lib/http/timeout/null.rb | 9 ---- lib/http/timeout/per_operation.rb | 1 - lib/http/uri.rb | 49 ++++++++++++++++++++- spec/lib/http/timeout/global_spec.rb | 16 ------- spec/lib/http/timeout/null_spec.rb | 14 ------ spec/lib/http/timeout/per_operation_spec.rb | 14 ------ spec/lib/http/uri_spec.rb | 26 +++++++++++ 8 files changed, 74 insertions(+), 56 deletions(-) delete mode 100644 spec/lib/http/timeout/global_spec.rb delete mode 100644 spec/lib/http/timeout/null_spec.rb delete mode 100644 spec/lib/http/timeout/per_operation_spec.rb diff --git a/lib/http/timeout/global.rb b/lib/http/timeout/global.rb index 7517bc41..1148aef9 100644 --- a/lib/http/timeout/global.rb +++ b/lib/http/timeout/global.rb @@ -22,7 +22,6 @@ def reset_counter def connect(socket_class, host, port, nodelay = false) reset_timer ::Timeout.timeout(@time_left, ConnectTimeoutError) do - host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end diff --git a/lib/http/timeout/null.rb b/lib/http/timeout/null.rb index bf6c571f..a5ee19ca 100644 --- a/lib/http/timeout/null.rb +++ b/lib/http/timeout/null.rb @@ -18,7 +18,6 @@ def initialize(options = {}) # Connects to a socket def connect(socket_class, host, port, nodelay = false) - host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end @@ -72,14 +71,6 @@ def rescue_writable(timeout = write_timeout) retry if @socket.to_io.wait_writable(timeout) raise TimeoutError, "Write timed out after #{timeout} seconds" end - - # Strip brackets from IPv6 addresses - def strip_ipv6_brackets(host) - match_data = /\[(.+)\]/.match(host) - return host if match_data.nil? - - match_data[1] - end end end end diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 455795ac..9274325e 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -21,7 +21,6 @@ def initialize(*args) def connect(socket_class, host, port, nodelay = false) ::Timeout.timeout(@connect_timeout, ConnectTimeoutError) do - host = strip_ipv6_brackets(host) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end diff --git a/lib/http/uri.rb b/lib/http/uri.rb index f694ed08..841a0846 100644 --- a/lib/http/uri.rb +++ b/lib/http/uri.rb @@ -9,7 +9,6 @@ class URI def_delegators :@uri, :scheme, :normalized_scheme, :scheme= def_delegators :@uri, :user, :normalized_user, :user= def_delegators :@uri, :password, :normalized_password, :password= - def_delegators :@uri, :host, :normalized_host, :host= def_delegators :@uri, :authority, :normalized_authority, :authority= def_delegators :@uri, :origin, :origin= def_delegators :@uri, :normalized_port, :port= @@ -110,6 +109,54 @@ def hash @hash ||= to_s.hash * -1 end + # Host, either a domain name or IP address. If the host is an IPv6 address, it will be returned + # without brackets surrounding it. + # + # @return [String] The host of the URI + def host + @host ||= begin + ip = IPAddr.new(@uri.host) + + if ip.ipv6? + ip.to_s + else + @uri.host + end + rescue IPAddr::Error + @uri.host + end + end + + # Normalized host, either a domain name or IP address. If the host is an IPv6 address, it will + # be returned without brackets surrounding it. + # + # @return [String] The normalized host of the URI + def normalized_host + @normalized_host ||= begin + ip = IPAddr.new(@uri.normalized_host) + + if ip.ipv6? + ip.to_s + else + @uri.normalized_host + end + rescue IPAddr::Error + @uri.normalized_host + end + end + + # Sets the host component for the URI. + # + # @param [String, #to_str] new_host The new host component. + # @return [void] + def host=(new_host) + @uri.host = new_host + + # Reset dependent values + remove_instance_variable(:@host) if defined?(@host) + remove_instance_variable(:@normalized_host) if defined?(@normalized_host) + end + # Port number, either as specified or the default if unspecified # # @return [Integer] port number diff --git a/spec/lib/http/timeout/global_spec.rb b/spec/lib/http/timeout/global_spec.rb deleted file mode 100644 index cf2af41f..00000000 --- a/spec/lib/http/timeout/global_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe HTTP::Timeout::Global do - subject { described_class.new(:global_timeout => 1) } - - let(:socket_class) { double("SocketClass", :open => socket) } - let(:socket) { double("Socket", :setsockopt => nil) } - - describe "#connect" do - it "strips brackets from IPv6 addresses before sending them to the socket" do - expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) - - subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) - end - end -end diff --git a/spec/lib/http/timeout/null_spec.rb b/spec/lib/http/timeout/null_spec.rb deleted file mode 100644 index 6d393c02..00000000 --- a/spec/lib/http/timeout/null_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe HTTP::Timeout::Null do - let(:socket_class) { double("SocketClass", :open => socket) } - let(:socket) { double("Socket", :setsockopt => nil) } - - describe "#connect" do - it "strips brackets from IPv6 addresses before sending them to the socket" do - expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) - - subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) - end - end -end diff --git a/spec/lib/http/timeout/per_operation_spec.rb b/spec/lib/http/timeout/per_operation_spec.rb deleted file mode 100644 index 9f2d41ec..00000000 --- a/spec/lib/http/timeout/per_operation_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe HTTP::Timeout::PerOperation do - let(:socket_class) { double("SocketClass", :open => socket) } - let(:socket) { double("Socket", :setsockopt => nil) } - - describe "#connect" do - it "strips brackets from IPv6 addresses before sending them to the socket" do - expect(socket_class).to receive(:open).with("2606:2800:220:1:248:1893:25c8:1946", 80) - - subject.connect(socket_class, "[2606:2800:220:1:248:1893:25c8:1946]", 80) - end - end -end diff --git a/spec/lib/http/uri_spec.rb b/spec/lib/http/uri_spec.rb index 7e91760e..a92e0cff 100644 --- a/spec/lib/http/uri_spec.rb +++ b/spec/lib/http/uri_spec.rb @@ -3,9 +3,11 @@ RSpec.describe HTTP::URI do let(:example_http_uri_string) { "http://example.com" } let(:example_https_uri_string) { "https://example.com" } + let(:example_ipv6_uri_string) { "https://[2606:2800:220:1:248:1893:25c8:1946]" } subject(:http_uri) { described_class.parse(example_http_uri_string) } subject(:https_uri) { described_class.parse(example_https_uri_string) } + subject(:ipv6_uri) { described_class.parse(example_ipv6_uri_string) } it "knows URI schemes" do expect(http_uri.scheme).to eq "http" @@ -20,6 +22,30 @@ expect(https_uri.port).to eq 443 end + describe "#host" do + it "strips brackets from IPv6 addresses" do + expect(ipv6_uri.host).to eq("2606:2800:220:1:248:1893:25c8:1946") + end + end + + describe "#normalized_host" do + it "strips brackets from IPv6 addresses" do + expect(ipv6_uri.normalized_host).to eq("2606:2800:220:1:248:1893:25c8:1946") + end + end + + describe "#host=" do + it "resets cached values for #host and #normalized_host" do + expect(http_uri.host).to eq("example.com") + expect(http_uri.normalized_host).to eq("example.com") + + http_uri.host = "[2606:2800:220:1:248:1893:25c8:1946]" + + expect(http_uri.host).to eq("2606:2800:220:1:248:1893:25c8:1946") + expect(http_uri.normalized_host).to eq("2606:2800:220:1:248:1893:25c8:1946") + end + end + describe "#dup" do it "doesn't share internal value between duplicates" do duplicated_uri = http_uri.dup From 921513437b0aa43214c4d02fcc8215d6d00030a6 Mon Sep 17 00:00:00 2001 From: jeraki <83373294+jeraki@users.noreply.github.com> Date: Thu, 1 Dec 2022 17:32:12 -0800 Subject: [PATCH 3/4] Exclude HTTP::URI spec from RuboCop Metrics/BlockLength. --- .rubocop_todo.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 81c4d362..47d5f9fe 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -74,7 +74,7 @@ Metrics/AbcSize: - 'lib/http/request.rb' - 'lib/http/response.rb' -# Offense count: 69 +# Offense count: 70 # Configuration parameters: CountComments, Max, CountAsOne, ExcludedMethods, IgnoredMethods. # IgnoredMethods: refine Metrics/BlockLength: @@ -98,6 +98,7 @@ Metrics/BlockLength: - 'spec/lib/http/response/parser_spec.rb' - 'spec/lib/http/response/status_spec.rb' - 'spec/lib/http/response_spec.rb' + - 'spec/lib/http/uri_spec.rb' - 'spec/lib/http_spec.rb' - 'spec/support/http_handling_shared.rb' From b57e7ec34263682355e468f41da515059ea56f86 Mon Sep 17 00:00:00 2001 From: jeraki <83373294+jeraki@users.noreply.github.com> Date: Thu, 15 Dec 2022 17:10:42 -0800 Subject: [PATCH 4/4] Eagerly strip brackets from IPv6 addresses and add them to the underlying Addressable::URI when calling URI#host=. --- lib/http/uri.rb | 78 +++++++++++++++++++-------------------- spec/lib/http/uri_spec.rb | 23 +++++++++--- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/lib/http/uri.rb b/lib/http/uri.rb index 841a0846..9c8a54c3 100644 --- a/lib/http/uri.rb +++ b/lib/http/uri.rb @@ -19,6 +19,18 @@ class URI def_delegators :@uri, :fragment, :normalized_fragment, :fragment= def_delegators :@uri, :omit, :join, :normalize + # Host, either a domain name or IP address. If the host is an IPv6 address, it will be returned + # without brackets surrounding it. + # + # @return [String] The host of the URI + attr_reader :host + + # Normalized host, either a domain name or IP address. If the host is an IPv6 address, it will + # be returned without brackets surrounding it. + # + # @return [String] The normalized host of the URI + attr_reader :normalized_host + # @private HTTP_SCHEME = "http" @@ -82,6 +94,9 @@ def initialize(options_or_uri = {}) else raise TypeError, "expected Hash for options, got #{options_or_uri.class}" end + + @host = process_ipv6_brackets(@uri.host) + @normalized_host = process_ipv6_brackets(@uri.normalized_host) end # Are these URI objects equal? Normalizes both URIs prior to comparison @@ -109,52 +124,15 @@ def hash @hash ||= to_s.hash * -1 end - # Host, either a domain name or IP address. If the host is an IPv6 address, it will be returned - # without brackets surrounding it. - # - # @return [String] The host of the URI - def host - @host ||= begin - ip = IPAddr.new(@uri.host) - - if ip.ipv6? - ip.to_s - else - @uri.host - end - rescue IPAddr::Error - @uri.host - end - end - - # Normalized host, either a domain name or IP address. If the host is an IPv6 address, it will - # be returned without brackets surrounding it. - # - # @return [String] The normalized host of the URI - def normalized_host - @normalized_host ||= begin - ip = IPAddr.new(@uri.normalized_host) - - if ip.ipv6? - ip.to_s - else - @uri.normalized_host - end - rescue IPAddr::Error - @uri.normalized_host - end - end - # Sets the host component for the URI. # # @param [String, #to_str] new_host The new host component. # @return [void] def host=(new_host) - @uri.host = new_host + @uri.host = process_ipv6_brackets(new_host, :brackets => true) - # Reset dependent values - remove_instance_variable(:@host) if defined?(@host) - remove_instance_variable(:@normalized_host) if defined?(@normalized_host) + @host = process_ipv6_brackets(@uri.host) + @normalized_host = process_ipv6_brackets(@uri.normalized_host) end # Port number, either as specified or the default if unspecified @@ -193,5 +171,25 @@ def to_s def inspect format("#<%s:0x%014x URI:%s>", self.class.name, object_id << 1, to_s) end + + private + + # Process a URI host, adding or removing surrounding brackets if the host is an IPv6 address. + # + # @param [Boolean] brackets When true, brackets will be added to IPv6 addresses if missing. When + # false, they will be removed if present. + # + # @return [String] Host with IPv6 address brackets added or removed + def process_ipv6_brackets(raw_host, brackets: false) + ip = IPAddr.new(raw_host) + + if ip.ipv6? + brackets ? "[#{ip}]" : ip.to_s + else + raw_host + end + rescue IPAddr::Error + raw_host + end end end diff --git a/spec/lib/http/uri_spec.rb b/spec/lib/http/uri_spec.rb index a92e0cff..4a330d61 100644 --- a/spec/lib/http/uri_spec.rb +++ b/spec/lib/http/uri_spec.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true RSpec.describe HTTP::URI do + let(:example_ipv6_address) { "2606:2800:220:1:248:1893:25c8:1946" } + let(:example_http_uri_string) { "http://example.com" } let(:example_https_uri_string) { "https://example.com" } - let(:example_ipv6_uri_string) { "https://[2606:2800:220:1:248:1893:25c8:1946]" } + let(:example_ipv6_uri_string) { "https://[#{example_ipv6_address}]" } subject(:http_uri) { described_class.parse(example_http_uri_string) } subject(:https_uri) { described_class.parse(example_https_uri_string) } @@ -35,14 +37,25 @@ end describe "#host=" do - it "resets cached values for #host and #normalized_host" do + it "updates cached values for #host and #normalized_host" do + expect(http_uri.host).to eq("example.com") + expect(http_uri.normalized_host).to eq("example.com") + + http_uri.host = "[#{example_ipv6_address}]" + + expect(http_uri.host).to eq(example_ipv6_address) + expect(http_uri.normalized_host).to eq(example_ipv6_address) + end + + it "ensures IPv6 addresses are bracketed in the inner Addressable::URI" do expect(http_uri.host).to eq("example.com") expect(http_uri.normalized_host).to eq("example.com") - http_uri.host = "[2606:2800:220:1:248:1893:25c8:1946]" + http_uri.host = example_ipv6_address - expect(http_uri.host).to eq("2606:2800:220:1:248:1893:25c8:1946") - expect(http_uri.normalized_host).to eq("2606:2800:220:1:248:1893:25c8:1946") + expect(http_uri.host).to eq(example_ipv6_address) + expect(http_uri.normalized_host).to eq(example_ipv6_address) + expect(http_uri.instance_variable_get(:@uri).host).to eq("[#{example_ipv6_address}]") end end