Skip to content

Commit 240fe65

Browse files
committed
[fix] read_nonblock(exception: false) throwing on TLS 1.3
propagate exception flag through read() and readAndUnwrap()
1 parent 9a54b42 commit 240fe65

2 files changed

Lines changed: 552 additions & 15 deletions

File tree

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

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,18 @@ public void wakeup() {
539539
}
540540
}
541541

542-
private static final int READ_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 1;
543-
private static final int WRITE_WOULD_BLOCK_RESULT = Integer.MIN_VALUE + 2;
542+
// Legitimate return values are -1 (EOF) and >= 0 (byte counts), so any value < -1 is safely in sentinel territory.
543+
private static final int READ_WOULD_BLOCK_RESULT = -2;
544+
private static final int WRITE_WOULD_BLOCK_RESULT = -3;
545+
546+
private static boolean isWouldBlockResult(final int result) {
547+
return result < -1;
548+
}
549+
550+
private RubySymbol wouldBlockSymbol(final int result) {
551+
assert isWouldBlockResult(result) : "unexpected result: " + result;
552+
return getRuntime().newSymbol(result == READ_WOULD_BLOCK_RESULT ? "wait_readable" : "wait_writable");
553+
}
544554

545555
private static void readWouldBlock(final Ruby runtime, final boolean exception, final int[] result) {
546556
if ( exception ) throw newSSLErrorWaitReadable(runtime, "read would block");
@@ -552,10 +562,6 @@ private static void writeWouldBlock(final Ruby runtime, final boolean exception,
552562
result[0] = WRITE_WOULD_BLOCK_RESULT;
553563
}
554564

555-
private void doHandshake(final boolean blocking) throws IOException {
556-
doHandshake(blocking, true);
557-
}
558-
559565
// might return :wait_readable | :wait_writable in case (true, false)
560566
private IRubyObject doHandshake(final boolean blocking, final boolean exception) throws IOException {
561567
while (true) {
@@ -577,7 +583,11 @@ private IRubyObject doHandshake(final boolean blocking, final boolean exception)
577583
doTasks();
578584
break;
579585
case NEED_UNWRAP:
580-
if (readAndUnwrap(blocking) == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
586+
int unwrapResult = readAndUnwrap(blocking, exception);
587+
if (isWouldBlockResult(unwrapResult)) {
588+
return wouldBlockSymbol(unwrapResult);
589+
}
590+
if (unwrapResult == -1 && handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
581591
throw new SSLHandshakeException("Socket closed");
582592
}
583593
// during initialHandshake, calling readAndUnwrap that results UNDERFLOW does not mean writable.
@@ -703,12 +713,16 @@ public int write(ByteBuffer src, boolean blocking) throws SSLException, IOExcept
703713
}
704714

705715
public int read(final ByteBuffer dst, final boolean blocking) throws IOException {
716+
return read(dst, blocking, true);
717+
}
718+
719+
private int read(final ByteBuffer dst, final boolean blocking, final boolean exception) throws IOException {
706720
if ( initialHandshake ) return 0;
707721
if ( engine.isInboundDone() ) return -1;
708722

709723
if ( ! appReadData.hasRemaining() ) {
710-
int appBytesProduced = readAndUnwrap(blocking);
711-
if (appBytesProduced == -1 || appBytesProduced == 0) {
724+
final int appBytesProduced = readAndUnwrap(blocking, exception);
725+
if (appBytesProduced == -1 || appBytesProduced == 0 || isWouldBlockResult(appBytesProduced)) {
712726
return appBytesProduced;
713727
}
714728
}
@@ -718,7 +732,15 @@ public int read(final ByteBuffer dst, final boolean blocking) throws IOException
718732
return limit;
719733
}
720734

721-
private int readAndUnwrap(final boolean blocking) throws IOException {
735+
/**
736+
* @param blocking whether to block on I/O
737+
* @param exception when false, returns {@link #READ_WOULD_BLOCK_RESULT} or
738+
* {@link #WRITE_WOULD_BLOCK_RESULT} instead of throwing if the
739+
* post-handshake processing would block
740+
* @return application bytes available, -1 on EOF/close, 0 when no app data
741+
* produced, or a WOULD_BLOCK sentinel when would-block with exception=false
742+
*/
743+
private int readAndUnwrap(final boolean blocking, final boolean exception) throws IOException {
722744
final int bytesRead = socketChannelImpl().read(netReadData);
723745
if ( bytesRead == -1 ) {
724746
if ( ! netReadData.hasRemaining() ||
@@ -767,7 +789,11 @@ private int readAndUnwrap(final boolean blocking) throws IOException {
767789
handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_TASK ||
768790
handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP ||
769791
handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED ) ) {
770-
doHandshake(blocking);
792+
IRubyObject ex = doHandshake(blocking, exception);
793+
if ( ex != null ) { // :wait_readable | :wait_writable
794+
// TODO needs refactoring to avoid Symbol -> int -> Symbol
795+
return "wait_writable".equals(ex.asJavaString()) ? WRITE_WOULD_BLOCK_RESULT : READ_WOULD_BLOCK_RESULT;
796+
}
771797
}
772798
return appReadData.remaining();
773799
}
@@ -843,12 +869,18 @@ private IRubyObject sysreadImpl(final ThreadContext context, final IRubyObject l
843869
if ( engine == null ) {
844870
read = socketChannelImpl().read(dst);
845871
} else {
846-
read = read(dst, blocking);
872+
read = read(dst, blocking, exception);
847873
}
848874

849-
if ( read == -1 ) {
850-
if ( exception ) throw runtime.newEOFError();
851-
return context.nil;
875+
switch ( read ) {
876+
case -1 :
877+
if ( exception ) throw runtime.newEOFError();
878+
return context.nil;
879+
// Post-handshake processing (e.g. TLS 1.3 NewSessionTicket) signaled would-block
880+
case READ_WOULD_BLOCK_RESULT :
881+
return runtime.newSymbol("wait_readable");
882+
case WRITE_WOULD_BLOCK_RESULT :
883+
return runtime.newSymbol("wait_writable");
852884
}
853885

854886
if ( read == 0 && status == SSLEngineResult.Status.BUFFER_UNDERFLOW ) {

0 commit comments

Comments
 (0)