[fix] return wait_writable on non-blocking reads

Author: kares

src/main/java/org/jruby/ext/openssl/SSLSocket.java

@@ -141,7 +141,7 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind
         return sites[ index.ordinal() ];
     }
 
-    private SSLContext sslContext;
+    SSLContext sslContext;
     private SSLEngine engine;
     private RubyIO io;
 
@@ -209,7 +209,7 @@ private IRubyObject fallback_set_io_nonblock_checked(ThreadContext context, Ruby
 
     private static final String SESSION_SOCKET_ID = "socket_id";
 
-    private SSLEngine ossl_ssl_setup(final ThreadContext context, final boolean server) {
+    SSLEngine ossl_ssl_setup(final ThreadContext context, final boolean server) {
         SSLEngine engine = this.engine;
         if ( engine != null ) return engine;
 
@@ -574,7 +574,7 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception)
                 doTasks();
                 break;
             case NEED_UNWRAP:
-                if (readAndUnwrap(blocking) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
+                if (readAndUnwrap(blocking, exception) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
                     throw new SSLHandshakeException("Socket closed");
                 }
                 // during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable.
@@ -721,10 +721,6 @@ private int read(final ByteBuffer dst, final boolean blocking, final boolean exc
         return limit;
     }
 
-    private int readAndUnwrap(final boolean blocking) throws IOException {
-        return readAndUnwrap(blocking, true);
-    }
-
     private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException {
         final int bytesRead = socketChannelImpl().read(netReadData);
         if ( bytesRead == -1 ) {
@@ -813,10 +809,10 @@ private void doShutdown() throws IOException {
     }
 
     /**
-     * @return the (@link RubyString} buffer or :wait_readable / :wait_writeable {@link RubySymbol}
+     * @return the {@link RubyString} buffer or :wait_readable / :wait_writeable {@link RubySymbol}
      */
-    private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject len, final IRubyObject buff,
-                                    final boolean blocking, final boolean exception) {
+    private IRubyObject sysreadImpl(final ThreadContext context,
+        final IRubyObject len, final IRubyObject buff, final boolean blocking, final boolean exception) {
         final Ruby runtime = context.runtime;
 
         final int length = RubyNumeric.fix2int(len);
@@ -836,11 +832,12 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l
         }
 
         try {
-            // flush any pending encrypted write data before reading; after write_nonblock,
-            // encrypted bytes may remain in the buffer that haven't been sent, if we read wout flushing,
-            // server may not have received the complete request (e.g. net/http POST body) and will not respond
+            // Flush pending write data before reading (after write_nonblock encrypted bytes may still be buffered)
             if ( engine != null && netWriteData.hasRemaining() ) {
-                flushData(blocking);
+                if ( flushData(blocking) && ! blocking ) {
+                    if ( exception ) throw newSSLErrorWaitWritable(runtime, "write would block");
+                    return runtime.newSymbol("wait_writable");
+                }
             }
 
             // So we need to make sure to only block when there is no data left to process
@@ -851,7 +848,7 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l
 
             final ByteBuffer dst = ByteBuffer.allocate(length);
             int read = -1;
-            // ensure >0 bytes read; sysread is blocking read.
+            // ensure > 0 bytes read; sysread is blocking read
             while ( read <= 0 ) {
                 if ( engine == null ) {
                     read = socketChannelImpl().read(dst);
@@ -1238,7 +1235,7 @@ public IRubyObject ssl_version(ThreadContext context) {
         return context.runtime.newString( engine.getSession().getProtocol() );
     }
 
-    private transient SocketChannelImpl socketChannel;
+    transient SocketChannelImpl socketChannel;
 
     private SocketChannelImpl socketChannelImpl() {
         if ( socketChannel != null ) return socketChannel;
@@ -1253,7 +1250,7 @@ private SocketChannelImpl socketChannelImpl() {
         throw new IllegalStateException("unknow channel impl: " + channel + " of type " + channel.getClass().getName());
     }
 
-    private interface SocketChannelImpl {
+    interface SocketChannelImpl {
 
         boolean isOpen() ;
 

src/test/java/org/jruby/ext/openssl/SSLSocketTest.java

@@ -1,9 +1,15 @@
 package org.jruby.ext.openssl;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.SelectionKey;
+import java.nio.channels.Selector;
+import javax.net.ssl.SSLEngine;
 
+import org.jruby.Ruby;
 import org.jruby.RubyArray;
 import org.jruby.RubyFixnum;
+import org.jruby.RubyHash;
 import org.jruby.RubyInteger;
 import org.jruby.RubyString;
 import org.jruby.exceptions.RaiseException;
@@ -12,6 +18,7 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+
 import static org.junit.Assert.*;
 
 public class SSLSocketTest extends OpenSSLHelper {
@@ -173,4 +180,72 @@ private void closeQuietly(final RubyArray sslPair) {
             }
         }
     }
+
+    // ----------
+
+    /**
+     * MRI's ossl_ssl_read_internal returns :wait_writable (or raises SSLErrorWaitWritable / "write would block")
+     * when SSL_read hits SSL_ERROR_WANT_WRITE. Pending netWriteData is JRuby's equivalent state.
+     */
+    @Test
+    public void sysreadNonblockReturnsWaitWritableWhenPendingEncryptedBytesRemain() {
+        final SSLSocket socket = newSSLSocket(runtime, partialWriteChannel(1));
+        final SSLEngine engine = socket.ossl_ssl_setup(currentContext(), false);
+        engine.setUseClientMode(true);
+
+        socket.netWriteData = ByteBuffer.wrap(new byte[] { 1, 2 });
+
+        final RubyHash opts = RubyHash.newKwargs(runtime, "exception", runtime.getFalse()); // exception: false
+        final IRubyObject result = socket.sysread_nonblock(currentContext(), runtime.newFixnum(1), opts);
+
+        assertEquals("wait_writable", result.asJavaString());
+        assertEquals(1, socket.netWriteData.remaining());
+    }
+
+    @Test
+    public void sysreadNonblockRaisesWaitWritableWhenPendingEncryptedBytesRemain() {
+        final SSLSocket socket = newSSLSocket(runtime, partialWriteChannel(1));
+        final SSLEngine engine = socket.ossl_ssl_setup(currentContext(), false);
+        engine.setUseClientMode(true);
+
+        socket.netWriteData = ByteBuffer.wrap(new byte[] { 1, 2 });
+
+        try {
+            socket.sysread_nonblock(currentContext(), runtime.newFixnum(1));
+            fail("expected SSLErrorWaitWritable");
+        }
+        catch (RaiseException ex) {
+            assertEquals("OpenSSL::SSL::SSLErrorWaitWritable", ex.getException().getMetaClass().getName());
+            assertTrue(ex.getMessage().contains("write would block"));
+            assertEquals(1, socket.netWriteData.remaining());
+        }
+    }
+
+    private static SSLSocket newSSLSocket(final Ruby runtime, final SSLSocket.SocketChannelImpl socketChannel) {
+        final SSLContext sslContext = new SSLContext(runtime);
+        sslContext.doSetup(runtime.getCurrentContext());
+        final SSLSocket sslSocket = new SSLSocket(runtime, runtime.getObject());
+        sslSocket.sslContext = sslContext;
+        sslSocket.socketChannel = socketChannel;
+        return sslSocket;
+    }
+
+    private static SSLSocket.SocketChannelImpl partialWriteChannel(final int bytesPerWrite) {
+        return new SSLSocket.SocketChannelImpl() {
+            public boolean isOpen() { return true; }
+            public int read(final ByteBuffer dst) { return 0; }
+            public int write(final ByteBuffer src) {
+                final int written = Math.min(bytesPerWrite, src.remaining());
+                src.position(src.position() + written);
+                return written;
+            }
+            public int getRemotePort() { return 443; }
+            public boolean isSelectable() { return false; }
+            public boolean isBlocking() { return false; }
+            public void configureBlocking(final boolean block) { }
+            public SelectionKey register(final Selector selector, final int ops) throws IOException {
+                throw new UnsupportedOperationException();
+            }
+        };
+    }
 }