Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(http1): send 'connection: close' during graceful shutdown #3725

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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