Skip to content

Commit 25a4abf

Browse files
seanmonstarseanmonstar
committed
fix(server): start header read timeout immediately
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.
1 parent f9aa697 commit 25a4abf

File tree

5 files changed

+71
-159
lines changed

5 files changed

+71
-159
lines changed

‎src/proto/h1/conn.rs‎

Lines changed: 49 additions & 13 deletions
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‎

Lines changed: 1 addition & 37 deletions
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‎

Lines changed: 0 additions & 15 deletions
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,

0 commit comments

Comments
 (0)