From 0e75202fc77109a72a934e4a235a2bd191033535 Mon Sep 17 00:00:00 2001 From: krynju Date: Fri, 16 Oct 2026 11:30:55 +0200 Subject: [PATCH] fix: eof spun forever on a record truncated by the peer closing When the peer closes mid-record, `SSL_peek_ex` keeps returning WANT_READ, `haspending` stays true for the partial record, and `eof(ssl.io)` returns true at once, so `eof` looped without ever yielding, starving every other task on the thread. Return true there: the bytes that would complete the record are never coming. One more round is allowed first: a concurrent read waiting on the socket ahead of this task may have taken the rest of the record while this task waited, in which case `bytesavailable` shows the decrypted bytes and `eof` is false. Adds `TruncatedRecordEOF`, which sends a partial record header and closes the socket cleanly. Without the fix it spins instead of failing. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 16 +++++++++-- test/runtests.jl | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index a469f2d..c5c004d 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -641,6 +641,9 @@ end function Base.eof(ssl::SSLStream)::Bool bytesavailable(ssl) > 0 && return false + # whether a round already found the socket at EOF: one more is allowed, in case a + # concurrent read took the bytes that completed the record meanwhile + atend = false while isopen(ssl) # note that care needs to be taken here to avoid a potential bad # race condition; for SSLStream, we have to manage the state of @@ -684,8 +687,17 @@ function Base.eof(ssl::SSLStream)::Bool elseif ret == SSL_ERROR_WANT_READ # if we get WANT_READ back, that means there were pending bytes # to be processed, but not a full record, so we need to wait - # for additional bytes to come in before we can process - eof(ssl.io) + # for additional bytes to come in before we can process. If the + # socket is at EOF they never will: the peer went away mid-record, + # so this is the end of the stream. Looping instead would spin: + # `haspending` stays true for the partial record and `eof(ssl.io)` + # returns at once, without ever yielding. One more round first: a + # concurrent read waiting on the socket ahead of this task may have + # taken the rest of the record, which `bytesavailable` then shows + if eof(ssl.io) + atend && return true + atend = true + end end end end diff --git a/test/runtests.jl b/test/runtests.jl index 5be468a..85d0cd4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -23,6 +23,51 @@ macro catch_exception_object(code) end end +# A self-signed server context for tests that talk to themselves over 127.0.0.1. +function selfsigned_server_ctx() + cert = X509Certificate() + key = EvpPKey(rsa_generate_key()) + cert.public_key = key + name = X509Name() + add_entry(name, "CN", "localhost") + cert.subject_name = name + cert.issuer_name = name + Dates.adjust(cert.time_not_before, Second(0)) + Dates.adjust(cert.time_not_after, Year(1)) + sign_certificate(cert, key) + ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") + OpenSSL.ssl_use_certificate(ctx, cert) + OpenSSL.ssl_use_private_key(ctx, key) + return ctx +end + +# `accept(ssl)` runs one round of the handshake and throws `OpenSSLError` when it needs +# more bytes from the peer; this waits for them and calls again until it completes. +function accept_handshake(ssl) + while true + try + return Sockets.accept(ssl) + catch ex + ex isa OpenSSL.OpenSSLError || rethrow() + eof(ssl.io) && throw(EOFError()) + end + end +end + +# A connected client stream and the server stream at the other end. +function connected_pair(server_ctx, server) + server_task = @async begin + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + accept_handshake(ssl) + ssl + end + port = Sockets.getsockname(server)[2] + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + return client, fetch(server_task) +end + # Verifies calling into OpenSSL library. @testset "OpenSSL" begin @test OpenSSL.BIO_STREAM_CALLBACKS.x.on_bio_create_ptr != C_NULL @@ -686,3 +731,30 @@ end @test_throws ErrorException OpenSSL.SSLContext(ssl_method, "does_not_exist") end + +@testset "TruncatedRecordEOF" begin + # A peer that goes away in the middle of a record leaves a partial record buffered: + # `haspending` stays true, `SSL_peek_ex` keeps asking for more bytes, and the socket + # is at EOF. `eof` has to report the end of the stream rather than loop on that. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + server_task = @async eof(ssl) + # keep taking whatever the server sends after the handshake (session tickets) off + # the socket, so that closing it sends a FIN rather than a RST + drain = @async while isopen(client.io) + readavailable(client.io) + end + sleep(0.5) + # a record header announcing 100 bytes, followed by only 3 of them, then close + write(client.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x64, 0xaa, 0xbb, 0xcc]) + close(client.io) + @test timedwait(() -> istaskdone(drain), 5.0) === :ok + # the spin never yields, so on one thread a hang here starves this task too: a + # task that does not finish is reported by the timeout, a spinning one by CI + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + @test istaskdone(server_task) && fetch(server_task) === true + close(client, false) + close(ssl) + close(server) +end