diff --git a/src/main/java/org/jruby/ext/openssl/SSLSocket.java b/src/main/java/org/jruby/ext/openssl/SSLSocket.java index 0a2f801e..661c0b6d 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLSocket.java +++ b/src/main/java/org/jruby/ext/openssl/SSLSocket.java @@ -688,7 +688,7 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept if ( netWriteData.hasRemaining() ) { flushData(blocking); } - netWriteData.clear(); + netWriteData.compact(); final SSLEngineResult result = engine.wrap(src, netWriteData); if ( result.getStatus() == SSLEngineResult.Status.CLOSED ) { throw getRuntime().newIOError("closed SSL engine"); @@ -830,6 +830,12 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l } try { + // Flush any pending encrypted write data before reading, so the + // remote side receives the complete request before we wait for a response. + if ( engine != null && netWriteData.hasRemaining() ) { + flushData(true); + } + // So we need to make sure to only block when there is no data left to process if ( engine == null || ! ( appReadData.hasRemaining() || netReadData.position() > 0 ) ) { final Object ex = waitSelect(SelectionKey.OP_READ, blocking, exception); diff --git a/src/test/ruby/ssl/test_write_flush.rb b/src/test/ruby/ssl/test_write_flush.rb new file mode 100644 index 00000000..e6ce817b --- /dev/null +++ b/src/test/ruby/ssl/test_write_flush.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: false +require File.expand_path('test_helper', File.dirname(__FILE__)) + +class TestSSLWriteFlush < TestCase + + include SSLTestHelper + + # write_nonblock a large payload then read the server's response. + # + # This exercises the write -> read transition used by net/http for POST + # requests. Two bugs in SSLSocket caused data loss here: + # 1. write() called netWriteData.clear() after a partial non-blocking + # flush, discarding encrypted bytes not yet sent to the socket. + # 2. sysreadImpl() did not flush pending netWriteData before reading. + # + # Without the fix the TLS stream is corrupted and the connection breaks + # with EPIPE. + def test_write_nonblock_then_read + data_size = 256 * 1024 + data = "X" * data_size + + server_proc = proc { |ctx, ssl| + begin + received = "" + begin + while received.bytesize < data_size + received << ssl.readpartial(8192) + end + rescue EOFError + end + ssl.write("GOT:#{received.bytesize}") + ensure + ssl.close rescue nil + end + } + + start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, + server_proc: server_proc) do |server, port| + sock = TCPSocket.new("127.0.0.1", port) + sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 2048) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.connect + ssl.sync_close = true + + remaining = data + while remaining.bytesize > 0 + begin + written = ssl.write_nonblock(remaining) + remaining = remaining.byteslice(written..-1) + rescue IO::WaitWritable + IO.select(nil, [ssl]) + retry + end + end + + response = "" + deadline = Time.now + 30 + loop do + remaining_time = deadline - Time.now + break if remaining_time <= 0 + if IO.select([ssl], nil, nil, [remaining_time, 1].min) + begin + chunk = ssl.read_nonblock(16384, exception: false) + case chunk + when :wait_readable then next + when nil then break + else response << chunk + end + rescue EOFError + break + end + end + end + + assert_match(/^GOT:#{data_size}$/, response, + "Server should receive all #{data_size} bytes") + ssl.close + end + end + +end