From 76ee7bb25548b4bd7ce7fce42d5637ba0c785cac Mon Sep 17 00:00:00 2001 From: jmwample Date: Mon, 23 Jun 2025 10:36:36 -0600 Subject: [PATCH 1/2] feat: not return UnexpectedEof when server drops without close_notify --- src/proto/connection.rs | 5 +---- src/proto/streams/streams.rs | 4 ---- tests/h2-tests/tests/client_request.rs | 10 +++------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/proto/connection.rs b/src/proto/connection.rs index f7fc8fcb5..865582f86 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -482,10 +482,7 @@ where // without error // // See https://github.com/hyperium/hyper/issues/3427 - if self.streams.is_server() - && self.streams.is_buffer_empty() - && matches!(kind, io::ErrorKind::UnexpectedEof) - { + if self.streams.is_buffer_empty() && matches!(kind, io::ErrorKind::UnexpectedEof) { *self.state = State::Closed(Reason::NO_ERROR, Initiator::Library); return Ok(()); } diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index c1e6cdeee..d18b226f8 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -337,10 +337,6 @@ impl DynStreams<'_, B> { self.send_buffer.is_empty() } - pub fn is_server(&self) -> bool { - self.peer.is_server() - } - pub fn recv_headers(&mut self, frame: frame::Headers) -> Result<(), Error> { let mut me = self.inner.lock().unwrap(); diff --git a/tests/h2-tests/tests/client_request.rs b/tests/h2-tests/tests/client_request.rs index 8c991839b..3670317ff 100644 --- a/tests/h2-tests/tests/client_request.rs +++ b/tests/h2-tests/tests/client_request.rs @@ -2,9 +2,9 @@ use futures::future::{ready, Either}; use futures::stream::FuturesUnordered; use futures::StreamExt; use h2_support::prelude::*; +use std::panic; use std::pin::Pin; use std::task::Context; -use std::{io, panic}; #[tokio::test] async fn handshake() { @@ -1914,7 +1914,7 @@ async fn receive_settings_frame_twice_with_second_one_non_empty() { } #[tokio::test] -async fn server_drop_connection_unexpectedly_return_unexpected_eof_err() { +async fn server_drop_connection_without_close_notify() { h2_support::trace_init!(); let (io, mut srv) = mock::new(); @@ -1944,11 +1944,7 @@ async fn server_drop_connection_unexpectedly_return_unexpected_eof_err() { .await .expect("request"); }); - let err = h2.await.expect_err("should receive UnexpectedEof"); - assert_eq!( - err.get_io().expect("should be UnexpectedEof").kind(), - io::ErrorKind::UnexpectedEof, - ); + let _ = h2.await.unwrap(); }; join(srv, h2).await; } From 3a14ecaaf9e3da2890563f513be4fe0206047cd8 Mon Sep 17 00:00:00 2001 From: jmwample Date: Fri, 27 Jun 2025 14:03:30 -0600 Subject: [PATCH 2/2] propogate EoF to client if server drops without go_away --- src/proto/connection.rs | 7 +++- src/proto/streams/streams.rs | 4 +++ tests/h2-tests/tests/client_request.rs | 48 ++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 865582f86..7a07001cb 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -482,7 +482,12 @@ where // without error // // See https://github.com/hyperium/hyper/issues/3427 - if self.streams.is_buffer_empty() && matches!(kind, io::ErrorKind::UnexpectedEof) { + if self.streams.is_buffer_empty() + && matches!(kind, io::ErrorKind::UnexpectedEof) + && (self.streams.is_server() + || self.error.as_ref().map(|f| f.reason() == Reason::NO_ERROR) + == Some(true)) + { *self.state = State::Closed(Reason::NO_ERROR, Initiator::Library); return Ok(()); } diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index d18b226f8..c1e6cdeee 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -337,6 +337,10 @@ impl DynStreams<'_, B> { self.send_buffer.is_empty() } + pub fn is_server(&self) -> bool { + self.peer.is_server() + } + pub fn recv_headers(&mut self, frame: frame::Headers) -> Result<(), Error> { let mut me = self.inner.lock().unwrap(); diff --git a/tests/h2-tests/tests/client_request.rs b/tests/h2-tests/tests/client_request.rs index 3670317ff..c99cc5317 100644 --- a/tests/h2-tests/tests/client_request.rs +++ b/tests/h2-tests/tests/client_request.rs @@ -2,9 +2,9 @@ use futures::future::{ready, Either}; use futures::stream::FuturesUnordered; use futures::StreamExt; use h2_support::prelude::*; -use std::panic; use std::pin::Pin; use std::task::Context; +use std::{io, panic}; #[tokio::test] async fn handshake() { @@ -1913,8 +1913,10 @@ async fn receive_settings_frame_twice_with_second_one_non_empty() { join(srv, h2).await; } +// If the server has not sent a go_away message before dropping the connection +// make sure the UnexpectedEof error is propogated. #[tokio::test] -async fn server_drop_connection_without_close_notify() { +async fn server_drop_connection_unexpectedly_return_unexpected_eof_err() { h2_support::trace_init!(); let (io, mut srv) = mock::new(); @@ -1930,6 +1932,48 @@ async fn server_drop_connection_without_close_notify() { srv.close_without_notify(); }; + let h2 = async move { + let (mut client, h2) = client::handshake(io).await.unwrap(); + tokio::spawn(async move { + let request = Request::builder() + .uri("https://http2.akamai.com/") + .body(()) + .unwrap(); + let _res = client + .send_request(request, true) + .unwrap() + .0 + .await + .expect("request"); + }); + let err = h2.await.expect_err("should receive UnexpectedEof"); + assert_eq!( + err.get_io().expect("should be UnexpectedEof").kind(), + io::ErrorKind::UnexpectedEof, + ); + }; + join(srv, h2).await; +} + +#[tokio::test] +async fn server_drop_connection_after_go_away() { + h2_support::trace_init!(); + let (io, mut srv) = mock::new(); + + let srv = async move { + let settings = srv.assert_client_handshake().await; + assert_default_settings!(settings); + srv.recv_frame( + frames::headers(1) + .request("GET", "https://http2.akamai.com/") + .eos(), + ) + .await; + srv.send_frame(frames::go_away(1)).await; + tokio::time::sleep(Duration::from_millis(50)).await; + srv.close_without_notify(); + }; + let h2 = async move { let (mut client, h2) = client::handshake(io).await.unwrap(); tokio::spawn(async move {