net/http: avoid reading/writing from unstarted SSL sockettopic/net-http-unstarted-sslsocket

This is currently done when net/http connects to an HTTPS server through
a CONNECT proxy.

OpenSSL::SSL::SSLSocket traditionally forwards to a method call to the
underlying IO object if an IO-ish method is called before the TLS
connection is established on it.

So it is working correctly, but this is not obvious at first glance.
Let's avoid the behavior. This will also eliminates the warning "SSL
session not started yet" that appears on $VERBOSE.

1 files changed, 21 insertions, 18 deletions

diff --git a/lib/net/http.rb b/lib/net/http.rb
index 2ada8aba3d..c739e41221 100644
--- a/lib/net/http.rb
+++ b/lib/net/http.rb
@@ -910,6 +910,22 @@ module Net   #:nodoc:
       s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
       D "opened"
       if use_ssl?
+        if proxy?
+          plain_sock = BufferedIO.new(s, read_timeout: @read_timeout,
+                                      continue_timeout: @continue_timeout,
+                                      debug_output: @debug_output)
+          buf = "CONNECT #{@address}:#{@port} HTTP/#{HTTPVersion}\r\n"
+          buf << "Host: #{@address}:#{@port}\r\n"
+          if proxy_user
+            credential = ["#{proxy_user}:#{proxy_pass}"].pack('m0')
+            buf << "Proxy-Authorization: Basic #{credential}\r\n"
+          end
+          buf << "\r\n"
+          plain_sock.write(buf)
+          HTTPResponse.read_new(plain_sock).value
+          # assuming nothing left in BufferedIO after successful CONNECT
+        end
+
         ssl_parameters = Hash.new
         iv_list = instance_variables
         SSL_IVNAMES.each_with_index do |ivname, i|
@@ -923,24 +939,7 @@ module Net   #:nodoc:
         D "starting SSL for #{conn_address}:#{conn_port}..."
         s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)
         s.sync_close = true
-        D "SSL established"
-      end
-      @socket = BufferedIO.new(s, read_timeout: @read_timeout,
-                               continue_timeout: @continue_timeout,
-                               debug_output: @debug_output)
-      if use_ssl?
         begin
-          if proxy?
-            buf = "CONNECT #{@address}:#{@port} HTTP/#{HTTPVersion}\r\n"
-            buf << "Host: #{@address}:#{@port}\r\n"
-            if proxy_user
-              credential = ["#{proxy_user}:#{proxy_pass}"].pack('m0')
-              buf << "Proxy-Authorization: Basic #{credential}\r\n"
-            end
-            buf << "\r\n"
-            @socket.write(buf)
-            HTTPResponse.read_new(@socket).value
-          end
           # Server Name Indication (SNI) RFC 3546
           s.hostname = @address if s.respond_to? :hostname=
           if @ssl_session and
@@ -952,12 +951,16 @@ module Net   #:nodoc:
             s.post_connection_check(@address)
           end
           @ssl_session = s.session
+          D "SSL established"
         rescue => exception
           D "Conn close because of connect error #{exception}"
-          @socket.close if @socket and not @socket.closed?
+          s.close
           raise exception
         end
       end
+      @socket = BufferedIO.new(s, read_timeout: @read_timeout,
+                               continue_timeout: @continue_timeout,
+                               debug_output: @debug_output)
       on_connect
     end
     private :connect