Fix SSL test harness and re-enable skipped HTTPS tests

sferik · sferik · commit 1d05346a9d7a · 2026-03-07T12:13:37.000-08:00
Closes #627.
diff --git a/lib/http/connection.rb b/lib/http/connection.rb
@@ -112,7 +112,13 @@ def readpartial(size = BUFFER_SIZE)
       chunk = @parser.read(size)
       return chunk if chunk
 
-      finished = (read_more(size) == :eof) || @parser.finished?
+      eof = read_more(size) == :eof
+      if eof && !@parser.finished? && body_framed?
+        close
+        raise ConnectionError, "response body ended prematurely"
+      end
+
+      finished = eof || @parser.finished?
       chunk    = @parser.read(size)
       finish_response if finished
 
@@ -215,6 +221,11 @@ def init_state(options)
       @parser               = Response::Parser.new
     end
 
+    def body_framed?
+      @parser.headers.include?(Headers::TRANSFER_ENCODING) ||
+        @parser.headers.include?(Headers::CONTENT_LENGTH)
+    end
+
     # Connect socket and set up proxy/TLS
     # @return [void]
     # @api private
diff --git a/test/http/client_test.rb b/test/http/client_test.rb
@@ -431,10 +431,10 @@ def wrap_response(res)
     include HTTPHandlingTests
   end
 
-  # TODO: https://github.com/httprb/http/issues/627
   describe "working with SSL" do
     run_server(:dummy_ssl) { DummyServer.new(ssl: true) }
 
+    let(:options) { {} }
     let(:extra_options) { {} }
 
     let(:client) do
@@ -443,8 +443,6 @@ def wrap_response(res)
 
     let(:server) { dummy_ssl }
 
-    before { skip "TODO: https://github.com/httprb/http/issues/627" }
-
     include HTTPHandlingTests
 
     it "just works" do
@@ -566,8 +564,6 @@ def wrap_response(res)
 
       context "with broken body (too early closed connection)" do
         it "raises HTTP::ConnectionError" do
-          skip "TODO: broken chunked body handling"
-
           response_data = [
             "HTTP/1.1 200 OK\r\n" \
             "Content-Type: application/json\r\n" \
diff --git a/test/http_test.rb b/test/http_test.rb
@@ -105,17 +105,14 @@
         assert_match(/<!doctype html>/, response.to_s)
       end
 
-      # TODO: https://github.com/httprb/http/issues/627
       context "ssl" do
         it "responds with the endpoint's body" do
-          skip "TODO: https://github.com/httprb/http/issues/627"
           response = ssl_client.via(proxy.addr, proxy.port).get dummy_ssl.endpoint
 
           assert_match(/<!doctype html>/, response.to_s)
         end
 
         it "ignores credentials" do
-          skip "TODO: https://github.com/httprb/http/issues/627"
           response = ssl_client.via(proxy.addr, proxy.port, "username", "password").get dummy_ssl.endpoint
 
           assert_match(/<!doctype html>/, response.to_s)
@@ -150,24 +147,20 @@
         assert_equal 407, response.status.to_i
       end
 
-      # TODO: https://github.com/httprb/http/issues/627
       context "ssl" do
         it "responds with the endpoint's body" do
-          skip "TODO: https://github.com/httprb/http/issues/627"
           response = ssl_client.via(proxy.addr, proxy.port, "username", "password").get dummy_ssl.endpoint
 
           assert_match(/<!doctype html>/, response.to_s)
         end
 
         it "responds with 407 when wrong credentials given" do
-          skip "TODO: https://github.com/httprb/http/issues/627"
           response = ssl_client.via(proxy.addr, proxy.port, "user", "pass").get dummy_ssl.endpoint
 
           assert_equal 407, response.status.to_i
         end
 
         it "responds with 407 if no credentials given" do
-          skip "TODO: https://github.com/httprb/http/issues/627"
           response = ssl_client.via(proxy.addr, proxy.port).get dummy_ssl.endpoint
 
           assert_equal 407, response.status.to_i
diff --git a/test/support/dummy_server.rb b/test/support/dummy_server.rb
@@ -15,6 +15,7 @@ def initialize(options = {})
     @memo       = {}
     @servlet    = Servlet.new(self, @memo)
     @running    = false
+    ssl_context if @ssl
   end
 
   def addr
diff --git a/test/support/proxy_server.rb b/test/support/proxy_server.rb
@@ -7,6 +7,8 @@
 require "support/servers/runner"
 
 class ProxyServer
+  Target = Struct.new(:host, :port, :path, :query, keyword_init: true)
+
   def initialize
     @tcp_server = TCPServer.new("127.0.0.1", 0)
     @port       = @tcp_server.addr[1]
@@ -40,16 +42,20 @@ def shutdown
   private
 
   def handle_request(client)
-    method, uri, version, headers, body = read_proxy_request(client)
-    return unless method
+    method, target, version, headers, body = read_proxy_request(client)
+    return unless method && target
 
     if (response = authenticate(headers))
       client.write(response)
       return
     end
 
-    forward_and_respond(client, method, uri, body, version)
-  rescue IOError, Errno::ECONNRESET, Errno::EPIPE
+    if method == "CONNECT"
+      tunnel_connection(client, target)
+    else
+      forward_and_respond(client, method, target, body, version)
+    end
+  rescue IOError, Errno::ECONNRESET, Errno::EPIPE, URI::InvalidURIError
     # Connection closed
   ensure
     client.close rescue nil # rubocop:disable Style/RescueModifier
@@ -59,11 +65,27 @@ def read_proxy_request(client)
     line = client.gets
     return unless line
 
-    method, url, version = line.strip.split(" ", 3)
+    method, target, version = line.strip.split(" ", 3)
     headers = read_headers(client)
     body = headers["Content-Length"] ? client.read(headers["Content-Length"].to_i) : nil
 
-    [method, URI.parse(url), version, headers, body]
+    [method, parse_target(method, target), version, headers, body]
+  end
+
+  def parse_target(method, target)
+    return parse_connect_target(target) if method == "CONNECT"
+
+    uri = URI.parse(target)
+    Target.new(host: uri.host, port: uri.port, path: uri.path, query: uri.query)
+  end
+
+  def parse_connect_target(target)
+    host, port = target.split(":", 2)
+    return unless host && port
+
+    Target.new(host: host, port: Integer(port))
+  rescue ArgumentError
+    nil
   end
 
   def read_headers(client)
@@ -81,24 +103,24 @@ def authenticate(_headers)
     nil
   end
 
-  def forward_and_respond(client, method, uri, body, version)
-    target = send_to_target(method, uri, body, version)
-    relay_response(client, target)
+  def forward_and_respond(client, method, target, body, version)
+    target_socket = send_to_target(method, target, body, version)
+    relay_response(client, target_socket)
   ensure
-    target&.close rescue nil # rubocop:disable Style/RescueModifier
+    target_socket&.close rescue nil # rubocop:disable Style/RescueModifier
   end
 
-  def send_to_target(method, uri, body, version)
-    target = TCPSocket.new(uri.host, uri.port)
-    path = uri.path.empty? ? "/" : uri.path
-    path = "#{path}?#{uri.query}" if uri.query
-
-    target.write("#{method} #{path} #{version}\r\n")
-    target.write("Host: #{uri.host}:#{uri.port}\r\n")
-    target.write("Content-Length: #{body.bytesize}\r\n") if body
-    target.write("\r\n")
-    target.write(body) if body
-    target
+  def send_to_target(method, target, body, version)
+    socket = TCPSocket.new(target.host, target.port)
+    path = target.path.to_s.empty? ? "/" : target.path
+    path = "#{path}?#{target.query}" if target.query
+
+    socket.write("#{method} #{path} #{version}\r\n")
+    socket.write("Host: #{target.host}:#{target.port}\r\n")
+    socket.write("Content-Length: #{body.bytesize}\r\n") if body
+    socket.write("\r\n")
+    socket.write(body) if body
+    socket
   end
 
   def relay_response(client, target)
@@ -113,6 +135,30 @@ def relay_response(client, target)
     # Target connection error
   end
 
+  def tunnel_connection(client, target)
+    target_socket = TCPSocket.new(target.host, target.port)
+
+    client.write("HTTP/1.1 200 Connection established\r\n\r\n")
+    relay_tunnel(client, target_socket)
+  ensure
+    target_socket&.close rescue nil # rubocop:disable Style/RescueModifier
+  end
+
+  def relay_tunnel(client, target)
+    [
+      Thread.new { copy_stream(client, target) },
+      Thread.new { copy_stream(target, client) }
+    ].each(&:join)
+  end
+
+  def copy_stream(source, destination)
+    loop do
+      destination.write(source.readpartial(1024))
+    end
+  rescue EOFError, IOError, Errno::ECONNRESET, Errno::EPIPE
+    nil
+  end
+
   def read_response_headers(target)
     headers = +""
     content_length = nil
diff --git a/test/support/ssl_helper.rb b/test/support/ssl_helper.rb
@@ -6,10 +6,19 @@
 
 module SSLHelper
   CERTS_PATH = Pathname.new File.expand_path("../../tmp/certs", __dir__)
+  CA_EXTENSIONS = {
+    "basicConstraints" => { "ca" => true, "critical" => true },
+    "keyUsage"         => { "usage" => %w[critical keyCertSign cRLSign] },
+    "extendedKeyUsage" => { "usage" => [] }
+  }.freeze
+  SERVER_EXTENSIONS = {
+    "basicConstraints" => { "ca" => false },
+    "keyUsage"         => { "usage" => %w[critical digitalSignature keyEncipherment] },
+    "extendedKeyUsage" => { "usage" => ["serverAuth"] },
+    "subjectAltName"   => { "ips" => ["127.0.0.1"] }
+  }.freeze
 
   class RootCertificate < ::CertificateAuthority::Certificate
-    EXTENSIONS = { "keyUsage" => { "usage" => %w[critical keyCertSign] } }.freeze
-
     def initialize
       super
 
@@ -19,7 +28,7 @@ def initialize
 
       self.signing_entity = true
 
-      sign!("extensions" => EXTENSIONS)
+      sign!("extensions" => CA_EXTENSIONS)
     end
 
     def file
@@ -35,17 +44,17 @@ def file
   end
 
   class ChildCertificate < ::CertificateAuthority::Certificate
-    def initialize(parent)
+    def initialize(parent, common_name:, serial_number:, extensions:)
       super()
 
-      subject.common_name  = "127.0.0.1"
-      serial_number.number = 1
+      subject.common_name = common_name
+      self.serial_number.number = serial_number
 
       key_material.generate_key
 
       self.parent = parent
 
-      sign!
+      sign!("extensions" => extensions)
     end
 
     def cert
@@ -61,7 +70,7 @@ class << self
     def server_context
       context = OpenSSL::SSL::SSLContext.new
 
-      context.verify_mode = OpenSSL::SSL::VERIFY_PEER
+      context.verify_mode = OpenSSL::SSL::VERIFY_NONE
       context.key         = server_cert.key
       context.cert        = server_cert.cert
       context.ca_file     = ca.file
@@ -70,31 +79,31 @@ def server_context
     end
 
     def client_context
+      server_cert
       context = OpenSSL::SSL::SSLContext.new
 
       context.options     = OpenSSL::SSL::SSLContext::DEFAULT_PARAMS[:options]
       context.verify_mode = OpenSSL::SSL::VERIFY_PEER
-      context.key         = client_cert.key
-      context.cert        = client_cert.cert
-      context.ca_file     = ca.file
+      context.verify_hostname = true if context.respond_to?(:verify_hostname=)
+      context.ca_file = ca.file
 
       context
     end
 
     def client_params
+      server_cert
       {
-        key:     client_cert.key,
-        cert:    client_cert.cert,
         ca_file: ca.file
       }
     end
 
-    %w[server client].each do |side|
-      class_eval <<-RUBY, __FILE__, __LINE__ + 1
-        def #{side}_cert
-          @#{side}_cert ||= ChildCertificate.new ca
-        end
-      RUBY
+    def server_cert
+      @server_cert ||= ChildCertificate.new(
+        ca,
+        common_name:   "127.0.0.1",
+        serial_number: 2,
+        extensions:    SERVER_EXTENSIONS
+      )
     end
 
     def ca
