[PATCH] ssl: ensure rack.hijack users get "normal" IO methods

[PATCH] ssl: ensure rack.hijack users get "normal" IO methods

[Eric Wong]

We do not want rack.hijack users relying on kgio_* methods since
kgio is trying to make itself obsolete (as Ruby itself adopts
kgio features).  This is a bit wonky since our common case tries
to minimize object allocation by only using the Kgio::Socket
derived class.
---
 lib/yahns/openssl_client.rb | 25 ++++++++++++++++
 test/helper.rb              | 10 +++++++
 test/test_rack_hijack.rb    | 10 -------
 test/test_ssl.rb            | 72 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/lib/yahns/openssl_client.rb b/lib/yahns/openssl_client.rb
index 5842e97..ffa4b3e 100644
--- a/lib/yahns/openssl_client.rb
+++ b/lib/yahns/openssl_client.rb
@@ -8,6 +8,31 @@ require_relative 'sendfile_compat'
 module Yahns::OpenSSLClient # :nodoc:
   include Yahns::SendfileCompat
 
+  def self.included(cls)
+    # Forward these methods to OpenSSL::SSL::SSLSocket so hijackers
+    # can rely on stdlib methods instead of ugly kgio stuff that
+    # we hope to phase out.
+    # This is a bit weird, since OpenSSL::SSL::SSLSocket wraps
+    # our actual socket, too, so we must take care to not blindly
+    # use method_missing and cause infinite recursion
+    %w(sync= read write readpartial write_nonblock read_nonblock
+       print printf puts gets readlines readline getc
+       readchar ungetc eof eof? << flush
+       sysread syswrite).map!(&:to_sym).each do |m|
+      cls.__send__(:define_method, m) { |*a| @ssl.__send__(m, *a) }
+    end
+
+    # block captures, ugh, but nobody really uses them
+    %w(each each_line each_byte).map!(&:to_sym).each do |m|
+      cls.__send__(:define_method, m) { |*a, &b| @ssl.__send__(m, *a, &b) }
+    end
+  end
+
+  # this is special, called during IO initialization in Ruby
+  def sync
+    defined?(@ssl) ? @ssl.sync : super
+  end
+
   def yahns_init_ssl(ssl_ctx)
     @need_accept = true
     @ssl = OpenSSL::SSL::SSLSocket.new(self, ssl_ctx)
diff --git a/test/helper.rb b/test/helper.rb
index 27adade..3e9f535 100644
--- a/test/helper.rb
+++ b/test/helper.rb
@@ -133,6 +133,16 @@ def require_exec(cmd)
   false
 end
 
+class DieIfUsed
+  def each
+    abort "body.each called after response hijack\n"
+  end
+
+  def close
+    abort "body.close called after response hijack\n"
+  end
+end
+
 require 'yahns'
 
 # needed for parallel (MT) tests)
diff --git a/test/test_rack_hijack.rb b/test/test_rack_hijack.rb
index 2cc6b2d..3e382eb 100644
--- a/test/test_rack_hijack.rb
+++ b/test/test_rack_hijack.rb
@@ -8,16 +8,6 @@ class TestRackHijack < Testcase
   alias setup server_helper_setup
   alias teardown server_helper_teardown
 
-  class DieIfUsed
-    def each
-      abort "body.each called after response hijack\n"
-    end
-
-    def close
-      abort "body.close called after response hijack\n"
-    end
-  end
-
   HIJACK_APP = lambda { |env|
     case env["PATH_INFO"]
     when "/hijack_input"
diff --git a/test/test_ssl.rb b/test/test_ssl.rb
index 13c14f0..8f01ef7 100644
--- a/test/test_ssl.rb
+++ b/test/test_ssl.rb
@@ -71,6 +71,15 @@ AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC
       end
     end
     client = ssl_client(host, port)
+    client.write("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")
+    buf = ''
+    Timeout.timeout(60) do
+      buf << client.readpartial(111) until buf =~ /HI\z/
+    end
+    head, body = buf.split("\r\n\r\n", 2)
+    assert_equal "HI", body
+    assert_match %r{\AHTTP/1\.\d 200 OK\r\n}, head
+
     client.write("GET / HTTP/1.0\r\n\r\n")
     head, body = client.read.split("\r\n\r\n", 2)
     assert_equal "HI", body
@@ -79,4 +88,67 @@ AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC
     client.close if client
     quit_wait(pid)
   end
+
+  def test_ssl_hijack
+    err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
+    ctx = srv_ctx
+    pid = mkserver(cfg) do
+      cfg.instance_eval do
+        ru = lambda do |env|
+          io = env['rack.hijack'].call
+          Thread.new(io) do |s|
+            s.write "HTTP/1.1 201 Switching Protocols\r\n\r\n"
+            case req = s.gets
+            when "inspect\n"
+              s.puts(s.instance_variable_get(:@ssl).inspect)
+            when "remote_address\n"
+              s.puts(s.remote_address.inspect)
+            when "each\n"
+              line = ''
+              s.each do |l|
+                l.strip!
+                line << l
+                break if l == 'd'
+              end
+              s.puts line
+            when "missing\n"
+              begin
+                s.any_old_invalid_test_method
+                s.puts "FAIL"
+              rescue => e
+                s.puts "#{e.class}: #{e.message}"
+              end
+            when nil
+              s.close
+            else
+              p [ :ERR, req ]
+            end until s.closed?
+          end
+          [ 200, DieIfUsed, DieIfUsed ]
+        end
+        app(:rack, ru) { listen "#{host}:#{port}", ssl_ctx: ctx }
+        logger(Logger.new(err.path))
+      end
+    end
+    client = ssl_client(host, port)
+    client.write("GET / HTTP/1.0\r\n\r\n")
+
+    Timeout.timeout(60) do
+      assert_equal "HTTP/1.1 201 Switching Protocols\r\n", client.gets
+      assert_equal "\r\n", client.gets
+      client.puts "inspect"
+      assert_match %r{SSLSocket}, client.gets
+      client.puts "remote_address"
+      assert_equal client.to_io.local_address.inspect, client.gets.strip
+      client.puts "missing"
+      assert_match %r{NoMethodError}, client.gets
+
+      client.puts "each"
+      %w(a b c d).each { |x| client.puts(x) }
+      assert_equal "abcd", client.gets.strip
+    end
+  ensure
+    client.close if client
+    quit_wait(pid)
+  end
 end if defined?(OpenSSL)
-- 
EW