Skip to content

Commit 0eb1b6c

Browse files
seanmonstarseanmonstar
authoredJun 3, 2024··
fix(server): start header read timeout immediately (#3185)
The `http1_header_read_timeout` used to start once there was a single read of headers. This change makes it start the timer immediately, right when the connection is estabilished.
·
v1.6.0v1.4.0
1 parent f9aa697 commit 0eb1b6c

File tree

5 files changed

+71
-159
lines changed

5 files changed

+71
-159
lines changed
 

‎src/proto/h1/conn.rs

+49-13
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use std::fmt;
2+
#[cfg(feature = "server")]
3+
use std::future::Future;
24
use std::io;
35
use std::marker::{PhantomData, Unpin};
46
use std::pin::Pin;
57
use std::task::{Context, Poll};
68
#[cfg(feature = "server")]
7-
use std::time::Duration;
9+
use std::time::{Duration, Instant};
810

911
use crate::rt::{Read, Write};
1012
use bytes::{Buf, Bytes};
@@ -209,33 +211,67 @@ where
209211
debug_assert!(self.can_read_head());
210212
trace!("Conn::read_head");
211213

212-
let msg = match ready!(self.io.parse::<T>(
214+
#[cfg(feature = "server")]
215+
if !self.state.h1_header_read_timeout_running {
216+
if let Some(h1_header_read_timeout) = self.state.h1_header_read_timeout {
217+
let deadline = Instant::now() + h1_header_read_timeout;
218+
self.state.h1_header_read_timeout_running = true;
219+
match self.state.h1_header_read_timeout_fut {
220+
Some(ref mut h1_header_read_timeout_fut) => {
221+
trace!("resetting h1 header read timeout timer");
222+
self.state.timer.reset(h1_header_read_timeout_fut, deadline);
223+
}
224+
None => {
225+
trace!("setting h1 header read timeout timer");
226+
self.state.h1_header_read_timeout_fut =
227+
Some(self.state.timer.sleep_until(deadline));
228+
}
229+
}
230+
}
231+
}
232+
233+
let msg = match self.io.parse::<T>(
213234
cx,
214235
ParseContext {
215236
cached_headers: &mut self.state.cached_headers,
216237
req_method: &mut self.state.method,
217238
h1_parser_config: self.state.h1_parser_config.clone(),
218239
h1_max_headers: self.state.h1_max_headers,
219-
#[cfg(feature = "server")]
220-
h1_header_read_timeout: self.state.h1_header_read_timeout,
221-
#[cfg(feature = "server")]
222-
h1_header_read_timeout_fut: &mut self.state.h1_header_read_timeout_fut,
223-
#[cfg(feature = "server")]
224-
h1_header_read_timeout_running: &mut self.state.h1_header_read_timeout_running,
225-
#[cfg(feature = "server")]
226-
timer: self.state.timer.clone(),
227240
preserve_header_case: self.state.preserve_header_case,
228241
#[cfg(feature = "ffi")]
229242
preserve_header_order: self.state.preserve_header_order,
230243
h09_responses: self.state.h09_responses,
231244
#[cfg(feature = "ffi")]
232245
on_informational: &mut self.state.on_informational,
246+
},
247+
) {
248+
Poll::Ready(Ok(msg)) => msg,
249+
Poll::Ready(Err(e)) => return self.on_read_head_error(e),
250+
Poll::Pending => {
251+
#[cfg(feature = "server")]
252+
if self.state.h1_header_read_timeout_running {
253+
if let Some(ref mut h1_header_read_timeout_fut) =
254+
self.state.h1_header_read_timeout_fut
255+
{
256+
if Pin::new(h1_header_read_timeout_fut).poll(cx).is_ready() {
257+
self.state.h1_header_read_timeout_running = false;
258+
259+
warn!("read header from client timeout");
260+
return Poll::Ready(Some(Err(crate::Error::new_header_timeout())));
261+
}
262+
}
263+
}
264+
265+
return Poll::Pending;
233266
}
234-
)) {
235-
Ok(msg) => msg,
236-
Err(e) => return self.on_read_head_error(e),
237267
};
238268

269+
#[cfg(feature = "server")]
270+
{
271+
self.state.h1_header_read_timeout_running = false;
272+
self.state.h1_header_read_timeout_fut = None;
273+
}
274+
239275
// Note: don't deconstruct `msg` into local variables, it appears
240276
// the optimizer doesn't remove the extra copies.
241277

‎src/proto/h1/io.rs

+1-37
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
use std::cmp;
22
use std::fmt;
3-
#[cfg(feature = "server")]
4-
use std::future::Future;
53
use std::io::{self, IoSlice};
64
use std::pin::Pin;
75
use std::task::{Context, Poll};
@@ -183,14 +181,6 @@ where
183181
req_method: parse_ctx.req_method,
184182
h1_parser_config: parse_ctx.h1_parser_config.clone(),
185183
h1_max_headers: parse_ctx.h1_max_headers,
186-
#[cfg(feature = "server")]
187-
h1_header_read_timeout: parse_ctx.h1_header_read_timeout,
188-
#[cfg(feature = "server")]
189-
h1_header_read_timeout_fut: parse_ctx.h1_header_read_timeout_fut,
190-
#[cfg(feature = "server")]
191-
h1_header_read_timeout_running: parse_ctx.h1_header_read_timeout_running,
192-
#[cfg(feature = "server")]
193-
timer: parse_ctx.timer.clone(),
194184
preserve_header_case: parse_ctx.preserve_header_case,
195185
#[cfg(feature = "ffi")]
196186
preserve_header_order: parse_ctx.preserve_header_order,
@@ -201,12 +191,6 @@ where
201191
)? {
202192
Some(msg) => {
203193
debug!("parsed {} headers", msg.head.headers.len());
204-
205-
#[cfg(feature = "server")]
206-
{
207-
*parse_ctx.h1_header_read_timeout_running = false;
208-
parse_ctx.h1_header_read_timeout_fut.take();
209-
}
210194
return Poll::Ready(Ok(msg));
211195
}
212196
None => {
@@ -215,20 +199,6 @@ where
215199
debug!("max_buf_size ({}) reached, closing", max);
216200
return Poll::Ready(Err(crate::Error::new_too_large()));
217201
}
218-
219-
#[cfg(feature = "server")]
220-
if *parse_ctx.h1_header_read_timeout_running {
221-
if let Some(h1_header_read_timeout_fut) =
222-
parse_ctx.h1_header_read_timeout_fut
223-
{
224-
if Pin::new(h1_header_read_timeout_fut).poll(cx).is_ready() {
225-
*parse_ctx.h1_header_read_timeout_running = false;
226-
227-
warn!("read header from client timeout");
228-
return Poll::Ready(Err(crate::Error::new_header_timeout()));
229-
}
230-
}
231-
}
232202
}
233203
}
234204
if ready!(self.poll_read_from_io(cx)).map_err(crate::Error::new_io)? == 0 {
@@ -660,10 +630,8 @@ enum WriteStrategy {
660630

661631
#[cfg(test)]
662632
mod tests {
663-
use crate::common::io::Compat;
664-
use crate::common::time::Time;
665-
666633
use super::*;
634+
use crate::common::io::Compat;
667635
use std::time::Duration;
668636

669637
use tokio_test::io::Builder as Mock;
@@ -726,10 +694,6 @@ mod tests {
726694
req_method: &mut None,
727695
h1_parser_config: Default::default(),
728696
h1_max_headers: None,
729-
h1_header_read_timeout: None,
730-
h1_header_read_timeout_fut: &mut None,
731-
h1_header_read_timeout_running: &mut false,
732-
timer: Time::Empty,
733697
preserve_header_case: false,
734698
#[cfg(feature = "ffi")]
735699
preserve_header_order: false,

‎src/proto/h1/mod.rs

-15
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,9 @@
1-
#[cfg(feature = "server")]
2-
use std::{pin::Pin, time::Duration};
3-
41
use bytes::BytesMut;
52
use http::{HeaderMap, Method};
63
use httparse::ParserConfig;
74

85
use crate::body::DecodedLength;
9-
#[cfg(feature = "server")]
10-
use crate::common::time::Time;
116
use crate::proto::{BodyLength, MessageHead};
12-
#[cfg(feature = "server")]
13-
use crate::rt::Sleep;
147

158
pub(crate) use self::conn::Conn;
169
pub(crate) use self::decode::Decoder;
@@ -79,14 +72,6 @@ pub(crate) struct ParseContext<'a> {
7972
req_method: &'a mut Option<Method>,
8073
h1_parser_config: ParserConfig,
8174
h1_max_headers: Option<usize>,
82-
#[cfg(feature = "server")]
83-
h1_header_read_timeout: Option<Duration>,
84-
#[cfg(feature = "server")]
85-
h1_header_read_timeout_fut: &'a mut Option<Pin<Box<dyn Sleep>>>,
86-
#[cfg(feature = "server")]
87-
h1_header_read_timeout_running: &'a mut bool,
88-
#[cfg(feature = "server")]
89-
timer: Time,
9075
preserve_header_case: bool,
9176
#[cfg(feature = "ffi")]
9277
preserve_header_order: bool,

‎src/proto/h1/role.rs

-94
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ use std::mem::MaybeUninit;
22

33
#[cfg(feature = "client")]
44
use std::fmt::{self, Write as _};
5-
#[cfg(feature = "server")]
6-
use std::time::Instant;
75

86
use bytes::Bytes;
97
use bytes::BytesMut;
@@ -80,24 +78,6 @@ where
8078

8179
let _entered = trace_span!("parse_headers");
8280

83-
#[cfg(feature = "server")]
84-
if !*ctx.h1_header_read_timeout_running {
85-
if let Some(h1_header_read_timeout) = ctx.h1_header_read_timeout {
86-
let deadline = Instant::now() + h1_header_read_timeout;
87-
*ctx.h1_header_read_timeout_running = true;
88-
match ctx.h1_header_read_timeout_fut {
89-
Some(h1_header_read_timeout_fut) => {
90-
debug!("resetting h1 header read timeout timer");
91-
ctx.timer.reset(h1_header_read_timeout_fut, deadline);
92-
}
93-
None => {
94-
debug!("setting h1 header read timeout timer");
95-
*ctx.h1_header_read_timeout_fut = Some(ctx.timer.sleep_until(deadline));
96-
}
97-
}
98-
}
99-
}
100-
10181
T::parse(bytes, ctx)
10282
}
10383

@@ -1631,8 +1611,6 @@ fn extend(dst: &mut Vec<u8>, data: &[u8]) {
16311611
mod tests {
16321612
use bytes::BytesMut;
16331613

1634-
use crate::common::time::Time;
1635-
16361614
use super::*;
16371615

16381616
#[test]
@@ -1647,10 +1625,6 @@ mod tests {
16471625
req_method: &mut method,
16481626
h1_parser_config: Default::default(),
16491627
h1_max_headers: None,
1650-
h1_header_read_timeout: None,
1651-
h1_header_read_timeout_fut: &mut None,
1652-
h1_header_read_timeout_running: &mut false,
1653-
timer: Time::Empty,
16541628
preserve_header_case: false,
16551629
#[cfg(feature = "ffi")]
16561630
preserve_header_order: false,
@@ -1679,10 +1653,6 @@ mod tests {
16791653
req_method: &mut Some(crate::Method::GET),
16801654
h1_parser_config: Default::default(),
16811655
h1_max_headers: None,
1682-
h1_header_read_timeout: None,
1683-
h1_header_read_timeout_fut: &mut None,
1684-
h1_header_read_timeout_running: &mut false,
1685-
timer: Time::Empty,
16861656
preserve_header_case: false,
16871657
#[cfg(feature = "ffi")]
16881658
preserve_header_order: false,
@@ -1706,10 +1676,6 @@ mod tests {
17061676
req_method: &mut None,
17071677
h1_parser_config: Default::default(),
17081678
h1_max_headers: None,
1709-
h1_header_read_timeout: None,
1710-
h1_header_read_timeout_fut: &mut None,
1711-
h1_header_read_timeout_running: &mut false,
1712-
timer: Time::Empty,
17131679
preserve_header_case: false,
17141680
#[cfg(feature = "ffi")]
17151681
preserve_header_order: false,
@@ -1731,10 +1697,6 @@ mod tests {
17311697
req_method: &mut Some(crate::Method::GET),
17321698
h1_parser_config: Default::default(),
17331699
h1_max_headers: None,
1734-
h1_header_read_timeout: None,
1735-
h1_header_read_timeout_fut: &mut None,
1736-
h1_header_read_timeout_running: &mut false,
1737-
timer: Time::Empty,
17381700
preserve_header_case: false,
17391701
#[cfg(feature = "ffi")]
17401702
preserve_header_order: false,
@@ -1758,10 +1720,6 @@ mod tests {
17581720
req_method: &mut Some(crate::Method::GET),
17591721
h1_parser_config: Default::default(),
17601722
h1_max_headers: None,
1761-
h1_header_read_timeout: None,
1762-
h1_header_read_timeout_fut: &mut None,
1763-
h1_header_read_timeout_running: &mut false,
1764-
timer: Time::Empty,
17651723
preserve_header_case: false,
17661724
#[cfg(feature = "ffi")]
17671725
preserve_header_order: false,
@@ -1789,10 +1747,6 @@ mod tests {
17891747
req_method: &mut Some(crate::Method::GET),
17901748
h1_parser_config,
17911749
h1_max_headers: None,
1792-
h1_header_read_timeout: None,
1793-
h1_header_read_timeout_fut: &mut None,
1794-
h1_header_read_timeout_running: &mut false,
1795-
timer: Time::Empty,
17961750
preserve_header_case: false,
17971751
#[cfg(feature = "ffi")]
17981752
preserve_header_order: false,
@@ -1817,10 +1771,6 @@ mod tests {
18171771
req_method: &mut Some(crate::Method::GET),
18181772
h1_parser_config: Default::default(),
18191773
h1_max_headers: None,
1820-
h1_header_read_timeout: None,
1821-
h1_header_read_timeout_fut: &mut None,
1822-
h1_header_read_timeout_running: &mut false,
1823-
timer: Time::Empty,
18241774
preserve_header_case: false,
18251775
#[cfg(feature = "ffi")]
18261776
preserve_header_order: false,
@@ -1840,10 +1790,6 @@ mod tests {
18401790
req_method: &mut None,
18411791
h1_parser_config: Default::default(),
18421792
h1_max_headers: None,
1843-
h1_header_read_timeout: None,
1844-
h1_header_read_timeout_fut: &mut None,
1845-
h1_header_read_timeout_running: &mut false,
1846-
timer: Time::Empty,
18471793
preserve_header_case: true,
18481794
#[cfg(feature = "ffi")]
18491795
preserve_header_order: false,
@@ -1884,10 +1830,6 @@ mod tests {
18841830
req_method: &mut None,
18851831
h1_parser_config: Default::default(),
18861832
h1_max_headers: None,
1887-
h1_header_read_timeout: None,
1888-
h1_header_read_timeout_fut: &mut None,
1889-
h1_header_read_timeout_running: &mut false,
1890-
timer: Time::Empty,
18911833
preserve_header_case: false,
18921834
#[cfg(feature = "ffi")]
18931835
preserve_header_order: false,
@@ -1909,10 +1851,6 @@ mod tests {
19091851
req_method: &mut None,
19101852
h1_parser_config: Default::default(),
19111853
h1_max_headers: None,
1912-
h1_header_read_timeout: None,
1913-
h1_header_read_timeout_fut: &mut None,
1914-
h1_header_read_timeout_running: &mut false,
1915-
timer: Time::Empty,
19161854
preserve_header_case: false,
19171855
#[cfg(feature = "ffi")]
19181856
preserve_header_order: false,
@@ -2143,10 +2081,6 @@ mod tests {
21432081
req_method: &mut Some(Method::GET),
21442082
h1_parser_config: Default::default(),
21452083
h1_max_headers: None,
2146-
h1_header_read_timeout: None,
2147-
h1_header_read_timeout_fut: &mut None,
2148-
h1_header_read_timeout_running: &mut false,
2149-
timer: Time::Empty,
21502084
preserve_header_case: false,
21512085
#[cfg(feature = "ffi")]
21522086
preserve_header_order: false,
@@ -2168,10 +2102,6 @@ mod tests {
21682102
req_method: &mut Some(m),
21692103
h1_parser_config: Default::default(),
21702104
h1_max_headers: None,
2171-
h1_header_read_timeout: None,
2172-
h1_header_read_timeout_fut: &mut None,
2173-
h1_header_read_timeout_running: &mut false,
2174-
timer: Time::Empty,
21752105
preserve_header_case: false,
21762106
#[cfg(feature = "ffi")]
21772107
preserve_header_order: false,
@@ -2193,10 +2123,6 @@ mod tests {
21932123
req_method: &mut Some(Method::GET),
21942124
h1_parser_config: Default::default(),
21952125
h1_max_headers: None,
2196-
h1_header_read_timeout: None,
2197-
h1_header_read_timeout_fut: &mut None,
2198-
h1_header_read_timeout_running: &mut false,
2199-
timer: Time::Empty,
22002126
preserve_header_case: false,
22012127
#[cfg(feature = "ffi")]
22022128
preserve_header_order: false,
@@ -2756,10 +2682,6 @@ mod tests {
27562682
req_method: &mut Some(Method::GET),
27572683
h1_parser_config: Default::default(),
27582684
h1_max_headers: None,
2759-
h1_header_read_timeout: None,
2760-
h1_header_read_timeout_fut: &mut None,
2761-
h1_header_read_timeout_running: &mut false,
2762-
timer: Time::Empty,
27632685
preserve_header_case: false,
27642686
#[cfg(feature = "ffi")]
27652687
preserve_header_order: false,
@@ -2803,10 +2725,6 @@ mod tests {
28032725
req_method: &mut None,
28042726
h1_parser_config: Default::default(),
28052727
h1_max_headers: max_headers,
2806-
h1_header_read_timeout: None,
2807-
h1_header_read_timeout_fut: &mut None,
2808-
h1_header_read_timeout_running: &mut false,
2809-
timer: Time::Empty,
28102728
preserve_header_case: false,
28112729
#[cfg(feature = "ffi")]
28122730
preserve_header_order: false,
@@ -2831,10 +2749,6 @@ mod tests {
28312749
req_method: &mut None,
28322750
h1_parser_config: Default::default(),
28332751
h1_max_headers: max_headers,
2834-
h1_header_read_timeout: None,
2835-
h1_header_read_timeout_fut: &mut None,
2836-
h1_header_read_timeout_running: &mut false,
2837-
timer: Time::Empty,
28382752
preserve_header_case: false,
28392753
#[cfg(feature = "ffi")]
28402754
preserve_header_order: false,
@@ -2982,10 +2896,6 @@ mod tests {
29822896
req_method: &mut None,
29832897
h1_parser_config: Default::default(),
29842898
h1_max_headers: None,
2985-
h1_header_read_timeout: None,
2986-
h1_header_read_timeout_fut: &mut None,
2987-
h1_header_read_timeout_running: &mut false,
2988-
timer: Time::Empty,
29892899
preserve_header_case: false,
29902900
#[cfg(feature = "ffi")]
29912901
preserve_header_order: false,
@@ -3031,10 +2941,6 @@ mod tests {
30312941
req_method: &mut None,
30322942
h1_parser_config: Default::default(),
30332943
h1_max_headers: None,
3034-
h1_header_read_timeout: None,
3035-
h1_header_read_timeout_fut: &mut None,
3036-
h1_header_read_timeout_running: &mut false,
3037-
timer: Time::Empty,
30382944
preserve_header_case: false,
30392945
#[cfg(feature = "ffi")]
30402946
preserve_header_order: false,

‎tests/server.rs

+21
Original file line numberDiff line numberDiff line change
@@ -1508,6 +1508,27 @@ async fn header_read_timeout_slow_writes() {
15081508
conn.without_shutdown().await.expect_err("header timeout");
15091509
}
15101510

1511+
#[tokio::test]
1512+
async fn header_read_timeout_starts_immediately() {
1513+
let (listener, addr) = setup_tcp_listener();
1514+
1515+
thread::spawn(move || {
1516+
let mut tcp = connect(&addr);
1517+
thread::sleep(Duration::from_secs(3));
1518+
let mut buf = [0u8; 256];
1519+
let n = tcp.read(&mut buf).expect("read 1");
1520+
assert_eq!(n, 0); //eof
1521+
});
1522+
1523+
let (socket, _) = listener.accept().await.unwrap();
1524+
let socket = TokioIo::new(socket);
1525+
let conn = http1::Builder::new()
1526+
.timer(TokioTimer)
1527+
.header_read_timeout(Duration::from_secs(2))
1528+
.serve_connection(socket, unreachable_service());
1529+
conn.await.expect_err("header timeout");
1530+
}
1531+
15111532
#[tokio::test]
15121533
async fn header_read_timeout_slow_writes_multiple_requests() {
15131534
let (listener, addr) = setup_tcp_listener();

0 commit comments

Comments
 (0)
Please sign in to comment.