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 <ocowchun@gmail.com>
puma · Sep 22, 2020 · bed5b24 · bed5b24
1 parent a734909
commit bed5b24
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 32 deletions.
diff --git 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
@@ -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 <Puma::MiniSSL::SSLError>
+    # @param ssl_socket <Puma::MiniSSL::Socket>
     #
-    def ssl_error(error, peeraddr, peercert)
+    def ssl_error(error, ssl_socket)
+      peeraddr = ssl_socket.peeraddr.last rescue "<unknown>"
+      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
@@ -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 = "<unknown>"
-                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
@@ -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
@@ -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 "<unknown>"
+      self.cert = ssl_socket.peercert
     end
   end