🥅 Re-raise #starttls error from receiver thread

Fixes #394. When `start_tls_session` raises an exception, that's caught in the receiver thread, but not re-raised. Fortunately, `@sock` will now be a permanently broken SSLSocket, so I don't think this can lead to accidentally using an insecure connection. Even so, `#starttls` should disconnect the socket and re-raise the error immediately. Failing test case was provided by @rhenium in #394. Co-authored-by: Kazuki Yamaguchi <[email protected]>
ruby · Feb 3, 2025 · 2fe469a · 2fe469a
1 parent d51d12e
commit 2fe469a
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/lib/net/imap.rb b/lib/net/imap.rb
@@ -1239,13 +1239,21 @@ def logout!
     #
     def starttls(**options)
       @ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options)
-      send_command("STARTTLS") do |resp|
+      error = nil
+      ok = send_command("STARTTLS") do |resp|
         if resp.kind_of?(TaggedResponse) && resp.name == "OK"
           clear_cached_capabilities
           clear_responses
           start_tls_session
         end
+      rescue Exception => error
+        raise # note that the error backtrace is in the receiver_thread
       end
+      if error
+        disconnect
+        raise error
+      end
+      ok
     end
 
     # :call-seq:

diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb
@@ -113,17 +113,16 @@ def test_starttls_unknown_ca
       omit "This test is not working with Windows" if RUBY_PLATFORM =~ /mswin|mingw/
 
       imap = nil
-      assert_raise(OpenSSL::SSL::SSLError) do
-        ex = nil
-        starttls_test do |port|
-          imap = Net::IMAP.new("localhost", port: port)
+      ex = nil
+      starttls_test do |port|
+        imap = Net::IMAP.new("localhost", port: port)
+        begin
           imap.starttls
-          imap
         rescue => ex
-          imap
         end
-        raise ex if ex
+        imap
       end
+      assert_kind_of(OpenSSL::SSL::SSLError, ex)
       assert_equal false, imap.tls_verified?
       assert_equal({}, imap.ssl_ctx_params)
       assert_equal(nil, imap.ssl_ctx.ca_file)