Skip to content

Commit c86a6bc

Browse files
authored
fix(http1): send 'connection: close' when connection is ending (#3725)
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
1 parent 4c4de90 commit c86a6bc

File tree

4 files changed

+29
-12
lines changed

4 files changed

+29
-12
lines changed

benches/pipeline.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ fn hello_world_16(b: &mut test::Bencher) {
7676
tcp.write_all(b"GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n")
7777
.unwrap();
7878
let mut buf = Vec::new();
79-
tcp.read_to_end(&mut buf).unwrap()
79+
tcp.read_to_end(&mut buf).unwrap() - "connection: close\r\n".len()
8080
} * PIPELINED_REQUESTS;
8181

8282
let mut tcp = TcpStream::connect(addr).unwrap();

benches/server.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ macro_rules! bench_server {
7272
tcp.write_all(b"GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n")
7373
.unwrap();
7474
let mut buf = Vec::new();
75-
tcp.read_to_end(&mut buf).unwrap()
75+
tcp.read_to_end(&mut buf).unwrap() - "connection: close\r\n".len()
7676
};
7777

7878
let mut tcp = TcpStream::connect(addr).unwrap();

src/proto/h1/conn.rs

+17-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use super::{Decoder, Encode, EncodedBuf, Encoder, Http1Transaction, ParseContext
2121
use crate::body::DecodedLength;
2222
#[cfg(feature = "server")]
2323
use crate::common::time::Time;
24-
use crate::headers::connection_keep_alive;
24+
use crate::headers;
2525
use crate::proto::{BodyLength, MessageHead};
2626
#[cfg(feature = "server")]
2727
use crate::rt::Sleep;
@@ -657,7 +657,7 @@ where
657657
let outgoing_is_keep_alive = head
658658
.headers
659659
.get(CONNECTION)
660-
.map_or(false, connection_keep_alive);
660+
.map_or(false, headers::connection_keep_alive);
661661

662662
if !outgoing_is_keep_alive {
663663
match head.version {
@@ -680,12 +680,21 @@ where
680680
// If we know the remote speaks an older version, we try to fix up any messages
681681
// to work with our older peer.
682682
fn enforce_version(&mut self, head: &mut MessageHead<T::Outgoing>) {
683-
if let Version::HTTP_10 = self.state.version {
684-
// Fixes response or connection when keep-alive header is not present
685-
self.fix_keep_alive(head);
686-
// If the remote only knows HTTP/1.0, we should force ourselves
687-
// to do only speak HTTP/1.0 as well.
688-
head.version = Version::HTTP_10;
683+
match self.state.version {
684+
Version::HTTP_10 => {
685+
// Fixes response or connection when keep-alive header is not present
686+
self.fix_keep_alive(head);
687+
// If the remote only knows HTTP/1.0, we should force ourselves
688+
// to do only speak HTTP/1.0 as well.
689+
head.version = Version::HTTP_10;
690+
}
691+
Version::HTTP_11 => {
692+
if let KA::Disabled = self.state.keep_alive.status() {
693+
head.headers
694+
.insert(CONNECTION, HeaderValue::from_static("close"));
695+
}
696+
}
697+
_ => (),
689698
}
690699
// If the remote speaks HTTP/1.1, then it *should* be fine with
691700
// both HTTP/1.0 and HTTP/1.1 from us. So again, we just let

tests/server.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,8 @@ fn pipeline_enabled() {
11401140

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

1184-
let expected = "HTTP/1.1 414 URI Too Long\r\ncontent-length: 0\r\n";
1186+
let expected = "HTTP/1.1 414 URI Too Long\r\nconnection: close\r\ncontent-length: 0\r\n";
11851187
let mut buf = [0; 256];
11861188
let n = req.read(&mut buf).unwrap();
11871189
assert!(n >= expected.len(), "read: {:?} >= {:?}", n, expected.len());
@@ -1208,6 +1210,12 @@ async fn disable_keep_alive_mid_request() {
12081210
"should receive OK response, but buf: {:?}",
12091211
buf,
12101212
);
1213+
let sbuf = s(&buf);
1214+
assert!(
1215+
sbuf.contains("connection: close\r\n"),
1216+
"response should have sent close: {:?}",
1217+
sbuf,
1218+
);
12111219
});
12121220

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

23722380
#[test]

0 commit comments

Comments
 (0)