From a2cdc0bc77a5dab3b32cca2f8de671db67b45a20 Mon Sep 17 00:00:00 2001 From: Andrew Stuntz Date: Tue, 19 Nov 2019 10:36:15 -0800 Subject: [PATCH] Removes connected_port, implements connected_ports This work starts the effort of removing connected_port from the specs. I have moved the code around a bit to get all connected_ports, the tests seem to pass in the appropriate areas. I want to verify this path before attempting to remove all UniquePort calls in the tests. --- lib/puma/binder.rb | 19 +++++++++++-------- lib/puma/launcher.rb | 6 +++--- lib/puma/minissl.rb | 4 ++++ lib/puma/server.rb | 2 +- test/test_cli.rb | 8 ++------ test/test_events.rb | 3 ++- test/test_puma_server.rb | 14 +++++++++----- test/test_puma_server_ssl.rb | 11 ++++++----- test/test_rack_handler.rb | 2 +- test/test_rack_server.rb | 10 +++++----- test/test_web_server.rb | 6 +++--- 11 files changed, 47 insertions(+), 38 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index 28fcb5e47a..89df84b256 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -226,19 +226,22 @@ def add_tcp_listener(host, port, optimize_for_latency=true, backlog=1024) end host = host[1..-2] if host and host[0..0] == '[' - s = TCPServer.new(host, port) + tcp_server = TCPServer.new(host, port) if optimize_for_latency - s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) + tcp_server.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) end - s.setsockopt(Socket::SOL_SOCKET,Socket::SO_REUSEADDR, true) - s.listen backlog - @connected_port = s.addr[1] + tcp_server.setsockopt(Socket::SOL_SOCKET,Socket::SO_REUSEADDR, true) + tcp_server.listen backlog - @ios << s - s + @ios << tcp_server + tcp_server end - attr_reader :connected_port + attr_reader :connected_ports + + def connected_ports + ios.map{ |io| io.addr[1] } + end def inherit_tcp_listener(host, port, fd) if fd.kind_of? TCPServer diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 4b96588a57..747388cc1e 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -187,9 +187,9 @@ def run @binder.close_unix_paths end - # Return which tcp port the launcher is using, if it's using TCP - def connected_port - @binder.connected_port + # Return all tcp ports the launcher may be using, TCP or SSL + def connected_ports + @binder.connected_ports end def restart_args diff --git a/lib/puma/minissl.rb b/lib/puma/minissl.rb index f83fdeb1bd..f281902dd7 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -270,6 +270,10 @@ def accept_nonblock Socket.new io, engine end + def addr + @socket.addr + end + def close @socket.close unless @socket.closed? # closed? call is for Windows end diff --git a/lib/puma/server.rb b/lib/puma/server.rb index 9f56ccd121..670bbc52e2 100644 --- a/lib/puma/server.rb +++ b/lib/puma/server.rb @@ -89,7 +89,7 @@ def initialize(app, events=Events.stdio, options={}) attr_accessor :binder, :leak_stack_on_error, :early_hints - def_delegators :@binder, :add_tcp_listener, :add_ssl_listener, :add_unix_listener, :connected_port + def_delegators :@binder, :add_tcp_listener, :add_ssl_listener, :add_unix_listener, :connected_ports def inherit_binder(bind) @binder = bind diff --git a/test/test_cli.rb b/test/test_cli.rb index 5e75cb2eb0..5d43c32e3c 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -37,12 +37,10 @@ def teardown end def test_control_for_tcp - tcp = UniquePort.call cntl = UniquePort.call url = "tcp://127.0.0.1:#{cntl}/" - cli = Puma::CLI.new ["-b", "tcp://127.0.0.1:#{tcp}", - "--control-url", url, + cli = Puma::CLI.new [ "--control-url", url, "--control-token", "", "test/rackup/lobster.ru"], @events @@ -66,14 +64,12 @@ def test_control_for_tcp end def test_control_for_ssl - app_port = UniquePort.call control_port = UniquePort.call control_host = "127.0.0.1" control_url = "ssl://#{control_host}:#{control_port}?#{ssl_query}" token = "token" - cli = Puma::CLI.new ["-b", "tcp://127.0.0.1:#{app_port}", - "--control-url", control_url, + cli = Puma::CLI.new ["--control-url", control_url, "--control-token", token, "test/rackup/lobster.ru"], @events diff --git a/test/test_events.rb b/test/test_events.rb index f966253514..a749362330 100644 --- a/test/test_events.rb +++ b/test/test_events.rb @@ -167,7 +167,8 @@ def test_parse_error server.add_tcp_listener host, port server.run - sock = TCPSocket.new host, server.connected_port + port = server.connected_ports[0] + sock = TCPSocket.new host, port path = "/" params = "a"*1024*10 diff --git a/test/test_puma_server.rb b/test/test_puma_server.rb index 4321b9bf88..be98b0368d 100644 --- a/test/test_puma_server.rb +++ b/test/test_puma_server.rb @@ -39,14 +39,16 @@ def header(sock) end def send_http_and_read(req) - sock = TCPSocket.new @host, @server.connected_port + port = @server.connected_ports[0] + sock = TCPSocket.new @host, port @ios << sock sock << req sock.read end def send_http(req) - sock = TCPSocket.new @host, @server.connected_port + port = @server.connected_ports[0] + sock = TCPSocket.new @host, port @ios << sock sock << req sock @@ -137,7 +139,8 @@ def test_default_server_port req = Net::HTTP::Get.new '/' req['HOST'] = 'example.com' - res = Net::HTTP.start @host, @server.connected_port do |http| + port = @server.connected_ports[0] + res = Net::HTTP.start @host, port do |http| http.request(req) end @@ -153,7 +156,8 @@ def test_default_server_port_respects_x_forwarded_proto req['HOST'] = "example.com" req['X_FORWARDED_PROTO'] = "https,http" - res = Net::HTTP.start @host, @server.connected_port do |http| + port = @server.connected_ports[0] + res = Net::HTTP.start @host, port do |http| http.request(req) end @@ -740,7 +744,7 @@ def test_request_body_wait_chunked } sock = send_http "GET / HTTP/1.1\r\nConnection: close\r\nTransfer-Encoding: chunked\r\n\r\n1\r\nh\r\n" - sleep 1 + sleep 3 sock << "4\r\nello\r\n0\r\n\r\n" sock.gets diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index 50d4cff3d9..296e64b786 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -45,7 +45,7 @@ def teardown # yields ctx to block, use for ctx setup & configuration def start_server - @port = UniquePort.call + @port = 0 @host = "127.0.0.1" app = lambda { |env| [200, {}, [env['rack.url_scheme']]] } @@ -69,7 +69,7 @@ def start_server @ssl_listener = @server.add_ssl_listener @host, @port, ctx @server.run - @http = Net::HTTP.new @host, @port + @http = Net::HTTP.new @host, @server.connected_ports[0] @http.use_ssl = true @http.verify_mode = OpenSSL::SSL::VERIFY_NONE end @@ -93,7 +93,8 @@ def test_request_wont_block_thread # Open a connection and give enough data to trigger a read, then wait ctx = OpenSSL::SSL::SSLContext.new ctx.verify_mode = OpenSSL::SSL::VERIFY_NONE - socket = OpenSSL::SSL::SSLSocket.new TCPSocket.new(@host, @port), ctx + port = @server.connected_ports[0] + socket = OpenSSL::SSL::SSLSocket.new TCPSocket.new(@host, port), ctx socket.write "x" sleep 0.1 @@ -206,7 +207,7 @@ class TestPumaServerSSLClient < Minitest::Test parallelize_me! def assert_ssl_client_error_match(error, subject=nil, &blk) host = "127.0.0.1" - port = UniquePort.call + port = 0 app = lambda { |env| [200, {}, [env['rack.url_scheme']]] } @@ -226,7 +227,7 @@ def assert_ssl_client_error_match(error, subject=nil, &blk) server.add_ssl_listener host, port, ctx server.run - http = Net::HTTP.new host, port + http = Net::HTTP.new host, server.connected_ports[0] http.use_ssl = true http.verify_mode = OpenSSL::SSL::VERIFY_NONE diff --git a/test/test_rack_handler.rb b/test/test_rack_handler.rb index 81790804fc..a94e67fbba 100644 --- a/test/test_rack_handler.rb +++ b/test/test_rack_handler.rb @@ -49,7 +49,7 @@ def test_handler_boots host = windows? ? "127.0.1.1" : "0.0.0.0" opts = { Host: host } in_handler(app, opts) do |launcher| - hit(["http://#{host}:#{ launcher.connected_port }/test"]) + hit(["http://#{host}:#{ launcher.connected_ports[0] }/test"]) assert_equal("/test", @input["PATH_INFO"]) end end diff --git a/test/test_rack_server.rb b/test/test_rack_server.rb index d0fcac7db0..f7d2a68305 100644 --- a/test/test_rack_server.rb +++ b/test/test_rack_server.rb @@ -55,7 +55,7 @@ def test_lint @server.run - hit(["http://127.0.0.1:#{ @server.connected_port }/test"]) + hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"]) stop @@ -70,7 +70,7 @@ def test_large_post_body big = "x" * (1024 * 16) - Net::HTTP.post_form URI.parse("http://127.0.0.1:#{ @server.connected_port }/test"), + Net::HTTP.post_form URI.parse("http://127.0.0.1:#{ @server.connected_ports[0] }/test"), { "big" => big } stop @@ -83,7 +83,7 @@ def test_path_info @server.app = lambda { |env| input = env; @simple.call(env) } @server.run - hit(["http://127.0.0.1:#{ @server.connected_port }/test/a/b/c"]) + hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test/a/b/c"]) stop @@ -100,7 +100,7 @@ def test_after_reply @server.run - hit(["http://127.0.0.1:#{ @server.connected_port }/test"]) + hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"]) stop @@ -116,7 +116,7 @@ def test_common_logger @server.run - hit(["http://127.0.0.1:#{ @server.connected_port }/test"]) + hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"]) stop diff --git a/test/test_web_server.rb b/test/test_web_server.rb index 5e9daa7fa0..acc8ba36f4 100644 --- a/test/test_web_server.rb +++ b/test/test_web_server.rb @@ -34,7 +34,7 @@ def teardown end def test_simple_server - hit(["http://127.0.0.1:#{@server.connected_port}/test"]) + hit(["http://127.0.0.1:#{@server.connected_ports[0]}/test"]) assert @tester.ran_test, "Handler didn't really run" end @@ -75,7 +75,7 @@ def test_file_streamed_request def do_test(string, chunk) # Do not use instance variables here, because it needs to be thread safe - socket = TCPSocket.new("127.0.0.1", @server.connected_port); + socket = TCPSocket.new("127.0.0.1", @server.connected_ports[0]); request = StringIO.new(string) chunks_out = 0 @@ -88,7 +88,7 @@ def do_test(string, chunk) def do_test_raise(string, chunk, close_after = nil) # Do not use instance variables here, because it needs to be thread safe - socket = TCPSocket.new("127.0.0.1", @server.connected_port); + socket = TCPSocket.new("127.0.0.1", @server.connected_ports[0]); request = StringIO.new(string) chunks_out = 0