[PATCH] fix output buffering with SSL_write

[Eric Wong <e@80x24.org>]

The underlying SSL_write called by the OpenSSL socket when
we use write_nonblock must get the same arguments after a
call returns SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE.

Ensure that by always passing a copy of the user-supplied
buffer to OpenSSL::SSL::SSLSocket#write_nonblock and retaining
our copy of the string internally as @ssl_blocked if we hit
EAGAIN on the socket.

String#dup is inexpensive in modern Ruby, as copying a
non-embedded string is implemented using copy-on-write.

We also prefer to use write_nonblock directly instead of
using our kgio-dependent sendfile emulation layer to avoid
allocating a new string on partial writes.

ref: https://bugs.ruby-lang.org/issues/12085
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/73882
http://mid.gmane.org/redmine.issue-12085.20160219020243.4b790a77f1cdd593@ruby-lang.org
---
  Eric Wong <e@80x24.org> wrote:
  > --- a/lib/yahns/openssl_client.rb
  > +++ b/lib/yahns/openssl_client.rb

  <snip>

  >    def kgio_trywrite(buf)
  > +    ssl_blocked = @ssl_blocked
  > +    if ! ssl_blocked.empty? && ssl_blocked.object_id != buf.object_id
  > +      warn "bk: #{ssl_blocked.inspect}\n!= #{buf.inspect}"
  > +    end
  > +    warn "buf: #{buf.inspect}" if buf.bytesize <= 23
  > +    #warn "b: #{buf.object_id} #{buf.object_id == ssl_blocked.object_id}"
  > +    buf = ssl_blocked.replace(buf) # buf and ssl_blocked may be the same obj
  > +    buf.force_encoding Encoding::BINARY
  >      rv = @ssl.write_nonblock(buf, exception: false)
  > -    Integer === rv and
  > +    case rv
  > +    when :wait_readable, :wait_writable
  > +      #warn "#{fileno} blocking #{buf.bytesize} #{buf.object_id}"
  > +      return rv # do not clear ssl_blocked
  > +    when Integer
  >        rv = buf.bytesize == rv ? nil : buf.byteslice(rv, buf.bytesize)
  > +    end
  > +    ssl_blocked.clear

  The above call to String#clear interacts badly with the following:

  > --- a/lib/yahns/sendfile_compat.rb
  > +++ b/lib/yahns/sendfile_compat.rb
  > @@ -6,19 +6,29 @@
  >  module Yahns::SendfileCompat # :nodoc:
  >    def trysendfile(io, offset, count)

  <snip>

  >      case rv = kgio_trywrite(str)
  >      when String # partial write, keep trying
  > -      n += (str.size - rv.size)
  > +      n += (str.bytesize - rv.size)
  >        str = rv
  >      when :wait_writable, :wait_readable
  > +      if respond_to?(:ssl_blocked)
  > +        #warn "empty: #{@ssl_blocked.empty?}"
  > +      end
  >        return n > 0 ? n : rv
  >      when nil
  > -      return n + str.size # yay!
  > +      return n + str.bytesize # yay!
  >      end while true
  >    end
  >  end

  ... because str.size becomes zero when String#clear got called
  Ooops :x

 lib/yahns/openssl_client.rb | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/lib/yahns/openssl_client.rb b/lib/yahns/openssl_client.rb
index bf64255..cd7d210 100644
--- a/lib/yahns/openssl_client.rb
+++ b/lib/yahns/openssl_client.rb
@@ -1,3 +1,4 @@
+# -*- encoding: binary -*-
 # Copyright (C) 2013-2016 all contributors <yahns-public@yhbt.net>
 # License: GPL-3.0+ (https://www.gnu.org/licenses/gpl-3.0.txt)
 # frozen_string_literal: true
@@ -7,8 +8,6 @@
 # this is to be included into a Kgio::Socket-derived class
 # this requires Ruby 2.1 and later for "exception: false"
 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
@@ -37,15 +36,25 @@ def sync
   def yahns_init_ssl(ssl_ctx)
     @need_accept = true
     @ssl = OpenSSL::SSL::SSLSocket.new(self, ssl_ctx)
+    @ssl_blocked = nil
   end
 
   def kgio_trywrite(buf)
-    rv = @ssl.write_nonblock(buf, exception: false)
-    Integer === rv and
-      rv = buf.bytesize == rv ? nil : buf.byteslice(rv, buf.bytesize)
+    buf = @ssl_blocked = buf.dup
+    case rv = @ssl.write_nonblock(buf, exception: false)
+    when :wait_readable, :wait_writable
+      return rv # do not clear ssl_blocked
+    when Integer
+      rv = buf.bytesize == rv ? nil : buf.byteslice(rv, buf.bytesize - rv)
+    end
+    @ssl_blocked = nil
     rv
   end
 
+  def kgio_trywritev(buf)
+    kgio_trywrite(buf.join)
+  end
+
   def kgio_syssend(buf, flags)
     kgio_trywrite(buf)
   end
@@ -63,6 +72,27 @@ def kgio_tryread(len, buf)
     @ssl.read_nonblock(len, buf, exception: false)
   end
 
+  def trysendfile(io, offset, count)
+    return 0 if count == 0
+
+    unless buf = @ssl_blocked
+      count = 0x4000 if count > 0x4000
+      buf = Thread.current[:yahns_sfbuf] ||= ''.dup
+      io.pos = offset
+      buf = io.read(count, buf) or return # nil for EOF
+      buf = @ssl_blocked = buf.dup
+    end
+
+    # call write_nonblock directly since kgio_trywrite allocates
+    # an unnecessary string
+    case rv = @ssl.write_nonblock(buf, exception: false)
+    when :wait_readable, :wait_writable
+      return rv # do not clear ssl_blocked
+    end
+    @ssl_blocked = nil
+    rv
+  end
+
   def close
     @ssl.close # flushes SSLSocket
     super # IO#close
-- 
EW