From bed5b24e2a830c98e6ee33b92ec7b22aee8498c6 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Sun, 20 Sep 2020 13:41:35 -0500 Subject: [PATCH] Change Events#ssl_error signature from (error, peeraddr, peercert) to (error, ssl_socket) The method signature should include the socket, so it determines what socket info is logged Co-authored-by: ocowchun --- History.md | 3 +++ lib/puma/events.rb | 8 +++++--- lib/puma/reactor.rb | 15 ++------------- lib/puma/server.rb | 16 +++------------- test/test_puma_server_ssl.rb | 6 +++--- 5 files changed, 16 insertions(+), 32 deletions(-) diff --git a/History.md b/History.md index 948ab3d5fa..0e6457434e 100644 --- a/History.md +++ b/History.md @@ -6,6 +6,9 @@ * Bugfixes * Your bugfix goes here (#Github Number) +* Refactor + * Change Events#ssl_error signature from (error, peeraddr, peercert) to (error, ssl_socket) (#2375) + ## 5.0.0 * Features diff --git a/lib/puma/events.rb b/lib/puma/events.rb index 36af4a6625..92679f428e 100644 --- a/lib/puma/events.rb +++ b/lib/puma/events.rb @@ -106,10 +106,12 @@ def parse_error(error, req) end # An SSL error has occurred. - # +error+ an exception object, +peeraddr+ peer address, - # and +peercert+ any peer certificate (if present). + # @param error + # @param ssl_socket # - def ssl_error(error, peeraddr, peercert) + def ssl_error(error, ssl_socket) + peeraddr = ssl_socket.peeraddr.last rescue "" + peercert = ssl_socket.peercert subject = peercert ? peercert.subject : nil @error_logger.info(error: error, text: "SSL error, peer: #{peeraddr}, peer cert: #{subject}") end diff --git a/lib/puma/reactor.rb b/lib/puma/reactor.rb index 8fcc2fa267..2941bea0ec 100644 --- a/lib/puma/reactor.rb +++ b/lib/puma/reactor.rb @@ -237,23 +237,12 @@ def run_internal # SSL handshake failure rescue MiniSSL::SSLError => e - @server.lowlevel_error(e, c.env) - - ssl_socket = c.io - begin - addr = ssl_socket.peeraddr.last - # EINVAL can happen when browser closes socket w/security exception - rescue IOError, Errno::EINVAL - addr = "" - end - - cert = ssl_socket.peercert + @server.lowlevel_error e, c.env + @events.ssl_error e, c.io c.close clear_monitor mon - @events.ssl_error e, addr, cert - # The client doesn't know HTTP well rescue HttpParserError => e @server.lowlevel_error(e, c.env) diff --git a/lib/puma/server.rb b/lib/puma/server.rb index d14b7fdfa4..5fb38efa1f 100644 --- a/lib/puma/server.rb +++ b/lib/puma/server.rb @@ -220,13 +220,9 @@ def run(background=true) process_now = true end rescue MiniSSL::SSLError => e - ssl_socket = client.io - addr = ssl_socket.peeraddr.last - cert = ssl_socket.peercert - + @events.ssl_error e, client.io client.close - @events.ssl_error e, addr, cert rescue HttpParserError => e client.write_error(400) client.close @@ -423,16 +419,10 @@ def process_client(client, buffer) # SSL handshake error rescue MiniSSL::SSLError => e - lowlevel_error(e, client.env) - - ssl_socket = client.io - addr = ssl_socket.peeraddr.last - cert = ssl_socket.peercert - + lowlevel_error e, client.env + @events.ssl_error e, client.io close_socket = true - @events.ssl_error e, addr, cert - # The client doesn't know HTTP well rescue HttpParserError => e lowlevel_error(e, client.env) diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index c31368c54b..1a5cf2e608 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -12,10 +12,10 @@ class SSLEventsHelper < ::Puma::Events attr_accessor :addr, :cert, :error - def ssl_error(error, peeraddr, peercert) + def ssl_error(error, ssl_socket) self.error = error - self.addr = peeraddr - self.cert = peercert + self.addr = ssl_socket.peeraddr.last rescue "" + self.cert = ssl_socket.peercert end end