Update deps and fix tests for new Ruby versions #59
Conversation
… fails in newer implementation
…d update minimum ruby version supported to >= 2.7
In older rubies, using an `SSLSocket` before a session was started worked as a normal (plain text) `Socket` (displaying a warning). After ruby 3.1 it raised an error, the workaround is to use the underlying `Socket` (via `to_io`) until an SSL session is started.
|
This is an exciting PR, and looks like quality work. Thank you! I'm getting a failure for four of the features when run on my box (details below). I assume this is just on my box, and the tests run cleanly for you. I'll start looking at this. |
|
Thanks for the kind words, I would have liked for the Github Action CI to run in order to check on linux, but who knows what combination of source/forked and branches would trigger a run properly 😀. |
I'll elevate your perms for this project if you want to try to figure that out (see also: Helping Me Whitewash The Fence)
Thanks! Here's what I know so far. The server thread just dies... goes away, no exception, no nothing, right here in command_loop.rb: This smells like one of those bugs that is of type "not very fun" and might not even be of your or my making. |
|
Yeah I got a lot of those while trying to find the issue in ruby >= 3.1, but I tested only on MacOS, and the differences are always subtle between OSes when dealing with Sockets. |
I really appreciate you helping to bring this abandoned lib back to life. I added two new commits in this branch; I think you should get them if you do a |
|
Just a quick report of what I found so far. I can reproduce the issues in a linux container. After a lot of debug, I can now make it fail/pass based on a single It seems the data transfer socket (opened by I'm not sure why it works on MacOS but fail on Linux, I suppose there's something different in the implementation of |
|
After messing with debug logs pretty much everywhere in ruby source code, I reduced the issue to a script which pass all the time on MacOS and fail "sometimes" on Linux (docker image I'm attaching the script here, in case someone is fluent in IO/Socket/SSL on Linux. For the server:
Then I start a new client (in a thread, the behaviour is the same without the thread, or when running the client in a different process) which:
require 'io/wait'
require "openssl"
require "net/ftp"
$stdout.sync = true
def log_server(msg)
$stdout.puts "\e[31m[SERVER]\e[0m #{msg}"
end
def log_server_session(msg)
log_server "\e[35m[SessionThread]\e[0m #{msg}"
end
def log_client(msg)
$stdout.puts "\e[34m[CLIENT]\e[0m #{msg}"
end
server = TCPServer.new("127.0.0.1", 0)
log_server "Server started on #{server.addr[2]}:#{server.addr[1]}"
ssl_context = OpenSSL::SSL::SSLContext.new
File.open(File.expand_path("../../insecure-test-cert.pem", __FILE__)) do |certfile|
ssl_context.cert = OpenSSL::X509::Certificate.new(certfile)
certfile.rewind
ssl_context.key = OpenSSL::PKey::RSA.new(certfile)
end
server_ssl_socket = OpenSSL::SSL::SSLServer.new(server, ssl_context)
def start_session(session_socket)
log_server "Starting new session #{session_socket}"
Thread.new do
sleep 1
loop do
begin
log_server_session "waiting on readable session_socket"
ready = IO.select([session_socket], nil, nil, nil)
if ready.nil?
log_server_session "timeout while waiting read on socket"
break
end
data = ready[0].first.gets
log_server_session "read: #{data.inspect}"
if data =~ /STOP/
break
end
rescue => e
log_server_session "Error: #{e.inspect}"
break
end
end
log_server_session "ending"
exit 1
end
end
server_thread = Thread.new do
# Thread.abort_on_exception = true
loop do
begin
session_socket = server_ssl_socket.accept
start_session(session_socket)
rescue Errno::EAGAIN, Errno::ECONNABORTED, Errno::EPROTO, Errno::EINVAL
IO.select([server_ssl_socket])
sleep(0.2)
retry
end
end
end
client_thread = Thread.new do
begin
log_client "start"
client = TCPSocket.new("127.0.0.1", server.addr[1].to_i)
ssl_client = OpenSSL::SSL::SSLSocket.new(client)
ssl_client.connect
ssl_client.sync_close = true
# buffered_ssl_client = ssl_client
buffered_ssl_client = Net::FTP::BufferedSSLSocket.new(ssl_client)
12.times do
buffered_ssl_client.write "DEMO\n"
log_client "write 'DEMO'"
end
buffered_ssl_client.write "STOP\n"
log_client "write 'STOP'"
# To stay inline with `Net::FTP#storlines`
buffered_ssl_client.shutdown(Socket::SHUT_RD) # this is overriden in Net::FTP::BufferedSSLSocket to not call `Socket#shutdown`
buffered_ssl_client.read_timeout = 1
buffered_ssl_client.read
buffered_ssl_client.close
log_client "socket closed"
rescue => e
log_client "Error: #{e.inspect}"
end
log_client "thread finished"
end
client_thread.join
server_thread.joinExample outputs:
[SERVER] Server started on 127.0.0.1:33809
[CLIENT] start
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'STOP'
[SERVER] Starting new session #<OpenSSL::SSL::SSLSocket:0x0000ffff8381fa00>
[CLIENT] socket closed
[CLIENT] thread finished
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] Error: #<Errno::ECONNRESET: Connection reset by peer>
[SERVER] [SessionThread] ending
[SERVER] Server started on 127.0.0.1:56239
[CLIENT] start
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'DEMO'
[CLIENT] write 'STOP'
[SERVER] Starting new session #<OpenSSL::SSL::SSLSocket:0x000000010102ff60>
[CLIENT] socket closed
[CLIENT] thread finished
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "DEMO\n"
[SERVER] [SessionThread] waiting on readable session_socket
[SERVER] [SessionThread] read: "STOP\n"
[SERVER] [SessionThread] endingThe idea is to have the server read from the socket, after the client has been disconnected. I have no idea what is supposed to be the expected behaviour 😃 |
I haven't had time to dig into it like you have, and my life is full of things at the moment. But in a quick search, I get a not quite conclusive answer from the internet on whether the server is guaranteed to be able to read all the data after a disconnect. It might be that a normal disconnect (FIN) does guarantee that the server gets to read the data, but an abnormal disconnect (RST) allows for data to be discarded. Maybe more research is needed to find out if that's true per spec, and also respected by all the implementations we are working with. And maybe a look-see with tcpdump to see if an RST is involved at all in this pathology. |
Yeah no worries, I'll continue to dig and see what needs some update.
Indeed, I'll get a tcpdump and see. |
|
I can confirm in the failure case, the last packet is: So And for the record a capture with a success on linux: |
|
It's the same on MacOS but there it works... But from what I can see every message on MacOS has an After looking more closely to the Linux capture, it also aknowledges each TLS message with an ACK, but labeled as |
JonathanTron
force-pushed
the
update-for-new-rubies
branch
2 times, most recently
from
26c9021
to
3858830
Compare
|
Hi @JonathanTron . I see the force push from a few days ago, to what I assume is your own branch in your fork, but it doesn't reflect in this pull request. Is that something you have to do to update the PR? |
|
Hi @wconrad, I made a commit which made the test pass on a linux container locally, but ultimately didn't change a thing on CI and failed afterwards also on my container. Also, I pushed with the wrong email. So I reverted and force push as it the commit didn't matter. |
|
That makes sense. Ok, thanks for the attempt.
…On Sun, Jan 7, 2024, 22:54 Jonathan Tron ***@***.***> wrote:
Hi @wconrad <https://github.com/wconrad>, I made a commit which made the
test pass on a linux container locally, but ultimately didn't change a
thing on CI and failed afterwards also on my VM. And I pushed with the
wrong email too. So I reverted and force push as it the commit didn't
matter.
—
Reply to this email directly, view it on GitHub
<#59 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYTCAAW4V4LEMKUUCWALLYNOCZBAVCNFSM6AAAAABBHTJEB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQGQZDCOBZGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@wconrad The issue seems to be in the |
An update to make tests pass on newer ruby versions (>= 3.1).
I switched CI to Github Actions, and added tests on 2.7, 3.0, 3.1, 3.2, and 3.3.
Ruby 2.7 is EOL since 2023-03-31, but as there is no issue with dependencies I included it in the CI.