From f3308c044d402dfad448bbc0497b14c69a8f22f2 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Mon, 6 Nov 2023 10:49:15 -0500 Subject: [PATCH] feat(lib): missing Timer will warn or panic If a default timeout is set, and no Timer, a warning will be emitted. If a timeout is configured by the user, and no Timer is set, it will panic. Closes #3393 --- src/common/time.rs | 64 ++++++++++++++++++---------------------- src/server/conn/http1.rs | 27 ++++++++++++----- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/common/time.rs b/src/common/time.rs index 9dff12534d..a8d3cc9c85 100644 --- a/src/common/time.rs +++ b/src/common/time.rs @@ -1,4 +1,7 @@ -#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))] +#[cfg(any( + all(any(feature = "client", feature = "server"), feature = "http2"), + all(feature = "server", feature = "http1"), +))] use std::time::Duration; use std::{fmt, sync::Arc}; use std::{pin::Pin, time::Instant}; @@ -13,46 +16,19 @@ pub(crate) enum Time { Empty, } +#[cfg(all(feature = "server", feature = "http1"))] +#[derive(Clone, Copy, Debug)] +pub(crate) enum Dur { + Default(Option), + Configured(Option), +} + impl fmt::Debug for Time { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Time").finish() } } -/* -pub(crate) fn timeout(tim: Tim, future: F, duration: Duration) -> HyperTimeout { - HyperTimeout { sleep: tim.sleep(duration), future: future } -} - -pin_project_lite::pin_project! { - pub(crate) struct HyperTimeout { - sleep: Box, - #[pin] - future: F - } -} - -pub(crate) struct Timeout; - -impl Future for HyperTimeout where F: Future { - - type Output = Result; - - fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll{ - let mut this = self.project(); - if let Poll::Ready(v) = this.future.poll(ctx) { - return Poll::Ready(Ok(v)); - } - - if let Poll::Ready(_) = Pin::new(&mut this.sleep).poll(ctx) { - return Poll::Ready(Err(Timeout)); - } - - return Poll::Pending; - } -} -*/ - impl Time { #[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))] pub(crate) fn sleep(&self, duration: Duration) -> Pin> { @@ -82,4 +58,22 @@ impl Time { Time::Timer(ref t) => t.reset(sleep, new_deadline), } } + + #[cfg(all(feature = "server", feature = "http1"))] + pub(crate) fn check(&self, dur: Dur, name: &'static str) -> Option { + match dur { + Dur::Default(Some(dur)) => match self { + Time::Empty => { + warn!("timeout `{}` has default, but no timer set", name,); + None + } + Time::Timer(..) => Some(dur), + }, + Dur::Configured(Some(dur)) => match self { + Time::Empty => panic!("timeout `{}` set, but no timer set", name,), + Time::Timer(..) => Some(dur), + }, + Dur::Default(None) | Dur::Configured(None) => None, + } + } } diff --git a/src/server/conn/http1.rs b/src/server/conn/http1.rs index c3a4f724ff..272101e716 100644 --- a/src/server/conn/http1.rs +++ b/src/server/conn/http1.rs @@ -15,7 +15,10 @@ use bytes::Bytes; use crate::body::{Body, Incoming as IncomingBody}; use crate::proto; use crate::service::HttpService; -use crate::{common::time::Time, rt::Timer}; +use crate::{ + common::time::{Dur, Time}, + rt::Timer, +}; type Http1Dispatcher = proto::h1::Dispatcher< proto::h1::dispatch::Server, @@ -70,7 +73,7 @@ pub struct Builder { h1_keep_alive: bool, h1_title_case_headers: bool, h1_preserve_header_case: bool, - h1_header_read_timeout: Option, + h1_header_read_timeout: Dur, h1_writev: Option, max_buf_size: Option, pipeline_flush: bool, @@ -237,7 +240,7 @@ impl Builder { h1_keep_alive: true, h1_title_case_headers: false, h1_preserve_header_case: false, - h1_header_read_timeout: None, + h1_header_read_timeout: Dur::Default(None), h1_writev: None, max_buf_size: None, pipeline_flush: false, @@ -293,8 +296,8 @@ impl Builder { /// transmit the entire header within this time, the connection is closed. /// /// Default is None. - pub fn header_read_timeout(&mut self, read_timeout: Duration) -> &mut Self { - self.h1_header_read_timeout = Some(read_timeout); + pub fn header_read_timeout(&mut self, read_timeout: impl Into>) -> &mut Self { + self.h1_header_read_timeout = Dur::Configured(read_timeout.into()); self } @@ -355,6 +358,11 @@ impl Builder { /// This returns a Future that must be polled in order for HTTP to be /// driven on the connection. /// + /// # Panics + /// + /// If a timeout option has been configured, but a `timer` has not been + /// provided, calling `serve_connection` will panic. + /// /// # Example /// /// ``` @@ -400,9 +408,12 @@ impl Builder { if self.h1_preserve_header_case { conn.set_preserve_header_case(); } - if let Some(header_read_timeout) = self.h1_header_read_timeout { - conn.set_http1_header_read_timeout(header_read_timeout); - } + if let Some(dur) = self + .timer + .check(self.h1_header_read_timeout, "header_read_timeout") + { + conn.set_http1_header_read_timeout(dur); + }; if let Some(writev) = self.h1_writev { if writev { conn.set_write_strategy_queue();