net/http: avoid writing/reading from unstarted SSL socket

When net/http connects to an HTTPS server through a CONNECT proxy, it
writes the CONNECT request to an unconnected OpenSSL::SSL::SSLSocket.

OpenSSL::SSL::SSLSocket traditionally fallbacks to a method call on the
underlying IO object if a read/write method is called before the TLS
connection is established. So it automagically works correctly, emitting
the "SSL session is not started yet" warning.

This is not obvious at first glance. The warning is also noisy. Let's
just write to the plain socket instead of relying on the SSLSocket's
behavior.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56857 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

lib/net/http.rb

@@ -910,6 +910,22 @@ def connect
       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 buffers after successful CONNECT response
+        end
+
         ssl_parameters = Hash.new
         iv_list = instance_variables
         SSL_IVNAMES.each_with_index do |ivname, i|
@@ -923,42 +939,29 @@ def connect
         D "starting SSL for #{conn_address}:#{conn_port}..."
         s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)
         s.sync_close = true
+        # Server Name Indication (SNI) RFC 3546
+        s.hostname = @address if s.respond_to? :hostname=
+        if @ssl_session and
+           Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
+          s.session = @ssl_session if @ssl_session
+        end
+        ssl_socket_connect(s, @open_timeout)
+        if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE
+          s.post_connection_check(@address)
+        end
+        @ssl_session = s.session
         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
-             Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
-            s.session = @ssl_session if @ssl_session
-          end
-          ssl_socket_connect(s, @open_timeout)
-          if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE
-            s.post_connection_check(@address)
-          end
-          @ssl_session = s.session
-        rescue => exception
-          D "Conn close because of connect error #{exception}"
-          @socket.close if @socket and not @socket.closed?
-          raise exception
-        end
-      end
       on_connect
+    rescue => exception
+      if s
+        D "Conn close because of connect error #{exception}"
+        s.close
+      end
+      raise
     end
     private :connect