rustls: handle EOF during initial handshake#21242
Conversation
10e1be4 to
c1360c7
Compare
cpu
left a comment
There was a problem hiding this comment.
Thanks! This diff looks good to me from the rustls-ffi API usage perspective.
I have less input to offer w.r.t the error choice + message, or the right place to consider test coverage (though I do agree it feels like there's a gap here!). Hopefully one of the friendly neighborhood curl gurus has some opinions on these topics.
|
There is one failing test on windows, but I'm not sure why, I can't identify the error and there should be no side-effect on schannel. |
|
I noticed openssl curl has the following output: I guess adding |
|
In case it's helpful, I tried to take a crack at writing a regression unit test: cpu@eb4ae39 I verified locally that it failed with c1360c7 reverted, as expected. Feel free to cherry-pick it in here, or otherwise fiddle with it further if folks don't like the location/approach used. |
Test that connecting to a server that immediately closes the connection produces an error instead of hanging/timing out.
c1360c7 to
e3e18a9
Compare
|
Thanks! |
I noticed curl built with vtls-rustls gets stuck if the server closes the connection immediately without writing anything.
This adds a check if
rustls_connection_read_tlshad returned 0, which is also interpreted as EOF in the demo client:https://github.com/rustls/rustls-ffi/blob/3f6476095541972da158226746a5d73e70842056/librustls/tests/client.c#L747-L752
Things I'm not sure about regarding this patch:
peer_closedin this case or leave it unmodified?== 0or< 0first"?TLS connect error: Connection closed abruptlyCURLE_RECV_ERRORthe best error code for this?CURLE_SSL_CONNECT_ERRORAlso happy to add a unit/integration test for this when pointed in the right direction.
cc: @cpu
Steps to reproduce
Before this patch
[hangs]
With this patch