From e6eb51f22214d1dc79dcd6eadc82899c2b7dfab7 Mon Sep 17 00:00:00 2001 From: Nate Berkopec Date: Sat, 7 Mar 2020 12:44:52 -0600 Subject: [PATCH] Add tests to binder (#2156) + Make loopback_addresses private + Semi-bugfix: connected ports are unique You can bind to the same port twice in ipv4 and ipv6, which probably wasnt what we intended to have those show up twice in the list --- lib/puma/binder.rb | 23 +++++++------ test/test_binder.rb | 82 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index 08e7224c..ca9f2abe 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -43,7 +43,8 @@ module Puma @ios = [] end - attr_reader :ios, :listeners, :unix_paths, :proto_env, :envs + attr_reader :ios, :listeners, :unix_paths, :proto_env, :envs, :activated_sockets, :inherited_fds + attr_writer :ios, :listeners def env(sock) @envs.fetch(sock, @proto_env) @@ -204,12 +205,6 @@ module Puma end end - def loopback_addresses - Socket.ip_address_list.select do |addrinfo| - addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback? - end.map { |addrinfo| addrinfo.ip_address }.uniq - end - # Tell the server to listen on host +host+, port +port+. # If +optimize_for_latency+ is true (the default) then clients connecting # will be optimized for latency over throughput. @@ -238,7 +233,7 @@ module Puma end def connected_ports - ios.map { |io| io.addr[1] } + ios.map { |io| io.addr[1] }.uniq end def inherit_tcp_listener(host, port, fd) @@ -361,12 +356,12 @@ module Puma end def close_listeners - @listeners.each do |l, io| + listeners.each do |l, io| io.close unless io.closed? # Ruby 2.2 issue uri = URI.parse(l) next unless uri.scheme == 'unix' unix_path = "#{uri.host}#{uri.path}" - File.unlink unix_path if @unix_paths.include? unix_path + File.unlink unix_path if unix_paths.include? unix_path end end @@ -378,5 +373,13 @@ module Puma end redirects end + + private + + def loopback_addresses + Socket.ip_address_list.select do |addrinfo| + addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback? + end.map { |addrinfo| addrinfo.ip_address }.uniq + end end end diff --git a/test/test_binder.rb b/test/test_binder.rb index 14c24f9c..f9faf2e8 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -23,18 +23,44 @@ class TestBinderBase < Minitest::Test end class TestBinder < TestBinderBase + parallelize_me! + def test_localhost_addresses_dont_alter_listeners_for_tcp_addresses - @binder.parse ["tcp://localhost:10001"], @events + @binder.parse ["tcp://localhost:0"], @events assert_empty @binder.listeners end + def test_home_alters_listeners_for_tcp_addresses + port = UniquePort.call + @binder.parse ["tcp://127.0.0.1:#{port}"], @events + + assert_equal "tcp://127.0.0.1:#{port}", @binder.listeners[0][0] + assert_kind_of TCPServer, @binder.listeners[0][1] + end + + def test_connected_ports + ports = (1..3).map { |_| UniquePort.call } + + @binder.parse(ports.map { |p| "tcp://localhost:#{p}" }, @events) + + assert_equal ports, @binder.connected_ports + end + def test_localhost_addresses_dont_alter_listeners_for_ssl_addresses - @binder.parse ["ssl://localhost:10002?#{ssl_query}"], @events + @binder.parse ["ssl://localhost:0?#{ssl_query}"], @events assert_empty @binder.listeners end + def test_home_alters_listeners_for_ssl_addresses + port = UniquePort.call + @binder.parse ["ssl://127.0.0.1:#{port}?#{ssl_query}"], @events + + assert_equal "ssl://127.0.0.1:#{port}?#{ssl_query}", @binder.listeners[0][0] + assert_kind_of TCPServer, @binder.listeners[0][1] + end + def test_correct_zero_port @binder.parse ["tcp://localhost:0"], @events @@ -59,7 +85,7 @@ class TestBinder < TestBinderBase @binder.parse ["tcp://localhost:0"], @events assert_match %r!tcp://127.0.0.1:(\d+)!, @events.stdout.string - if @binder.loopback_addresses.include?("::1") + if Socket.ip_address_list.any? {|i| i.ipv6_loopback? } assert_match %r!tcp://\[::1\]:(\d+)!, @events.stdout.string end end @@ -69,7 +95,7 @@ class TestBinder < TestBinderBase @binder.parse ["ssl://localhost:0?#{ssl_query}"], @events assert_match %r!ssl://127.0.0.1:(\d+)!, @events.stdout.string - if @binder.loopback_addresses.include?("::1") + if Socket.ip_address_list.any? {|i| i.ipv6_loopback? } assert_match %r!ssl://\[::1\]:(\d+)!, @events.stdout.string end end @@ -156,6 +182,54 @@ class TestBinder < TestBinderBase assert_equal @events.stderr, env_hash["rack.errors"] end + def test_close_calls_close_on_ios + @mocked_ios = [Minitest::Mock.new, Minitest::Mock.new] + @mocked_ios.each { |m| m.expect(:close, true) } + @binder.ios = @mocked_ios + + @binder.close + + assert @mocked_ios.each(&:verify) + end + + # test redirect for restart + + def test_close_listeners_closes_ios + @binder.parse ["tcp://127.0.0.1:#{UniquePort.call}"], @events + + refute @binder.listeners.any? { |u, l| l.closed? } + + @binder.close_listeners + + assert @binder.listeners.all? { |u, l| l.closed? } + end + + def test_close_listeners_closes_ios_unless_closed? + @binder.parse ["tcp://127.0.0.1:0"], @events + + bomb = @binder.listeners.first[1] + bomb.close + def bomb.close; raise "Boom!"; end # the bomb has been planted + + assert @binder.listeners.any? { |u, l| l.closed? } + + @binder.close_listeners + + assert @binder.listeners.all? { |u, l| l.closed? } + end + + def test_listeners_file_unlink_if_unix_listener + skip UNIX_SKT_MSG unless UNIX_SKT_EXIST + + @binder.parse ["unix://test/#{name}_server.sock"], @events + + assert File.socket?("test/#{name}_server.sock") + + @binder.close_listeners + + refute File.socket?("test/#{name}_server.sock") + end + private def assert_parsing_logs_uri(order = [:unix, :tcp])