Skip to content

Commit

Permalink
fix(http1): send 'connection: close' when connection is ending (#3725)
Browse files Browse the repository at this point in the history
This includes conditions where hyper knows the connection will end after the response, such as a request error that ruins the connection, or when graceful shutdown is triggered.

Closes #3720
  • Loading branch information
seanmonstar authored Oct 11, 2024
1 parent 4c4de90 commit c86a6bc
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 12 deletions.
2 changes: 1 addition & 1 deletion benches/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn hello_world_16(b: &mut test::Bencher) {
tcp.write_all(b"GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n")
.unwrap();
let mut buf = Vec::new();
tcp.read_to_end(&mut buf).unwrap()
tcp.read_to_end(&mut buf).unwrap() - "connection: close\r\n".len()
} * PIPELINED_REQUESTS;

let mut tcp = TcpStream::connect(addr).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion benches/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ macro_rules! bench_server {
tcp.write_all(b"GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n")
.unwrap();
let mut buf = Vec::new();
tcp.read_to_end(&mut buf).unwrap()
tcp.read_to_end(&mut buf).unwrap() - "connection: close\r\n".len()
};

let mut tcp = TcpStream::connect(addr).unwrap();
Expand Down
25 changes: 17 additions & 8 deletions src/proto/h1/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::{Decoder, Encode, EncodedBuf, Encoder, Http1Transaction, ParseContext
use crate::body::DecodedLength;
#[cfg(feature = "server")]
use crate::common::time::Time;
use crate::headers::connection_keep_alive;
use crate::headers;
use crate::proto::{BodyLength, MessageHead};
#[cfg(feature = "server")]
use crate::rt::Sleep;
Expand Down Expand Up @@ -657,7 +657,7 @@ where
let outgoing_is_keep_alive = head
.headers
.get(CONNECTION)
.map_or(false, connection_keep_alive);
.map_or(false, headers::connection_keep_alive);

if !outgoing_is_keep_alive {
match head.version {
Expand All @@ -680,12 +680,21 @@ where
// If we know the remote speaks an older version, we try to fix up any messages
// to work with our older peer.
fn enforce_version(&mut self, head: &mut MessageHead<T::Outgoing>) {
if let Version::HTTP_10 = self.state.version {
// Fixes response or connection when keep-alive header is not present
self.fix_keep_alive(head);
// If the remote only knows HTTP/1.0, we should force ourselves
// to do only speak HTTP/1.0 as well.
head.version = Version::HTTP_10;
match self.state.version {
Version::HTTP_10 => {
// Fixes response or connection when keep-alive header is not present
self.fix_keep_alive(head);
// If the remote only knows HTTP/1.0, we should force ourselves
// to do only speak HTTP/1.0 as well.
head.version = Version::HTTP_10;
}
Version::HTTP_11 => {
if let KA::Disabled = self.state.keep_alive.status() {
head.headers
.insert(CONNECTION, HeaderValue::from_static("close"));
}
}
_ => (),
}
// If the remote speaks HTTP/1.1, then it *should* be fine with
// both HTTP/1.0 and HTTP/1.1 from us. So again, we just let
Expand Down
12 changes: 10 additions & 2 deletions tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,8 @@ fn pipeline_enabled() {

assert_eq!(s(lines.next().unwrap()), "HTTP/1.1 200 OK\r");
assert_eq!(s(lines.next().unwrap()), "content-length: 12\r");
// close because the last request said to close
assert_eq!(s(lines.next().unwrap()), "connection: close\r");
lines.next().unwrap(); // Date
assert_eq!(s(lines.next().unwrap()), "\r");
assert_eq!(s(lines.next().unwrap()), "Hello World");
Expand Down Expand Up @@ -1181,7 +1183,7 @@ fn http_11_uri_too_long() {
let mut req = connect(server.addr());
req.write_all(request_line.as_bytes()).unwrap();

let expected = "HTTP/1.1 414 URI Too Long\r\ncontent-length: 0\r\n";
let expected = "HTTP/1.1 414 URI Too Long\r\nconnection: close\r\ncontent-length: 0\r\n";
let mut buf = [0; 256];
let n = req.read(&mut buf).unwrap();
assert!(n >= expected.len(), "read: {:?} >= {:?}", n, expected.len());
Expand All @@ -1208,6 +1210,12 @@ async fn disable_keep_alive_mid_request() {
"should receive OK response, but buf: {:?}",
buf,
);
let sbuf = s(&buf);
assert!(
sbuf.contains("connection: close\r\n"),
"response should have sent close: {:?}",
sbuf,
);
});

let (socket, _) = listener.accept().await.unwrap();
Expand Down Expand Up @@ -2366,7 +2374,7 @@ fn streaming_body() {
buf.starts_with(b"HTTP/1.1 200 OK\r\n"),
"response is 200 OK"
);
assert_eq!(buf.len(), 100_789, "full streamed body read");
assert_eq!(buf.len(), 100_808, "full streamed body read");
}

#[test]
Expand Down

0 comments on commit c86a6bc

Please sign in to comment.