From a7787741898e0aef9c7ce4324e615634169c44b5 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Wed, 18 Nov 2020 23:29:00 -0500 Subject: [PATCH 1/6] Added note about short timeouts to set_timeout docs --- src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 066ce28..96acd69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -139,7 +139,12 @@ impl Builder { /// If you would like a timeout for your code itself, please use /// [`tokio::time::Timeout`] to implement it internally. /// - /// The default timeout is 30 seconds. + /// **The default timeout is 30 seconds.** If you are considering changing this, keep + /// in mind that some clients, when recieving a file type not supported for display, + /// will prompt the user how they would like to proceed. While this occurs, the + /// request hangs open. Setting a short timeout may close the prompt before user has + /// a chance to respond. If you are only serving `text/plain` and `text/gemini`, this + /// should not be a problem. pub fn set_timeout(mut self, timeout: Duration) -> Self { self.timeout = timeout; self From 94d7a5ab4fd966d014fcf210055551b6cebf76d3 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 01:25:34 -0500 Subject: [PATCH 2/6] Added complex timeout override option to builder --- src/lib.rs | 160 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 136 insertions(+), 24 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 96acd69..9bd962d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,6 +40,7 @@ pub struct Server { listener: Arc, handler: Handler, timeout: Duration, + complex_timeout: Option, } impl Server { @@ -101,31 +102,116 @@ impl Server { }) .context("Request handler failed")?; - // Use a timeout for sending the response - let fut_send_and_flush = async { - send_response(response, &mut stream).await + self.send_response(response, &mut stream).await .context("Failed to send response")?; + Ok(()) + } + + async fn send_response(&self, mut response: Response, stream: &mut (impl AsyncWrite + Unpin)) -> Result<()> { + let maybe_body = response.take_body(); + let header = response.header(); + + // Okay, I know this method looks really complicated, but I promise it's not. + // There's really only three things this method does: + // + // * Send the response header + // * Send the response body + // * Flush the stream + // + // All the other code is doing one of two things. Either it's + // + // * code to add and handle timeouts (that's what all the async blocks and calls + // to tokio::time::timeout are), or + // * logic to decide whether to use the special case timeout handling (seperate + // timeouts for the header and the body) vs the normal timeout handling (header, + // body, and flush all as one timeout) + // + // The split between the two cases happens at this very first if block. + // Everything in this deep chain of if's and if-let's is for the special case. If + // any one of the ifs fails, the code after the big if block is run, and that's + // the normal case. + // + // Hope this helps! Emi <3 + + if header.status == Status::SUCCESS && maybe_body.is_some() { + // aaaa let me have if-let chaining ;_; + if let "text/plain"|"text/gemini" = header.meta.as_str() { + if let Some(cplx_timeout) = self.complex_timeout { + + + ////////////// Use the special case timeout override ///////////////////////////// + + // Send the header & flush + let fut_send_header = async { + send_response_header(response.header(), stream).await + .context("Failed to write response header")?; + + stream.flush() + .await + .context("Failed to flush response header") + }; + tokio::time::timeout(self.timeout, fut_send_header) + .await + .context("Timed out while sending response header")??; + + // Send the body & flush + let fut_send_body = async { + send_response_body(maybe_body.unwrap(), stream).await + .context("Failed to write response body")?; + + stream.flush() + .await + .context("Failed to flush response body") + }; + tokio::time::timeout(cplx_timeout, fut_send_body) + .await + .context("Timed out while sending response body")??; + + return Ok(()) + } + } + } + + + ///////////// Use the normal timeout ///////////////////////////////////////////// + + let fut_send_response = async { + send_response_header(response.header(), stream).await + .context("Failed to write response header")?; + + if let Some(body) = maybe_body { + send_response_body(body, stream).await + .context("Failed to write response body")?; + } + stream.flush() .await .context("Failed to flush response data") }; - timeout(self.timeout, fut_send_and_flush) + tokio::time::timeout(self.timeout, fut_send_response) .await - .context("Client timed out receiving response data")??; + .context("Timed out while sending response data")??; Ok(()) + + ////////////////////////////////////////////////////////////////////////////////// } } pub struct Builder { addr: A, timeout: Duration, + complex_body_timeout_override: Option, } impl Builder { fn bind(addr: A) -> Self { - Self { addr, timeout: Duration::from_secs(30) } + Self { + addr, + timeout: Duration::from_secs(1), + complex_body_timeout_override: Some(Duration::from_secs(30)), + } } /// Set the timeout on incoming requests @@ -139,17 +225,54 @@ impl Builder { /// If you would like a timeout for your code itself, please use /// [`tokio::time::Timeout`] to implement it internally. /// - /// **The default timeout is 30 seconds.** If you are considering changing this, keep - /// in mind that some clients, when recieving a file type not supported for display, - /// will prompt the user how they would like to proceed. While this occurs, the - /// request hangs open. Setting a short timeout may close the prompt before user has - /// a chance to respond. If you are only serving `text/plain` and `text/gemini`, this - /// should not be a problem. + /// **The default timeout is 1 second.** As somewhat of a workaround for + /// shortcomings of the specification, this timeout, and any timeout set using this + /// method, is overridden in special cases, specifically for MIME types outside of + /// `text/plain` and `text/gemini`, to be 30 seconds. If you would like to change or + /// prevent this, please see + /// [`override_complex_body_timeout`](Self::override_complex_body_timeout()). pub fn set_timeout(mut self, timeout: Duration) -> Self { self.timeout = timeout; self } + /// Override the timeout for complex body types + /// + /// Many clients choose to handle body types which cannot be displayed by prompting + /// the user if they would like to download or open the request body. However, since + /// this prompt occurs in the middle of receiving a request, often the connection + /// times out before the end user is able to respond to the prompt. + /// + /// As a workaround, it is possible to set an override on the request timeout in + /// specific conditions: + /// + /// 1. **Only override the timeout for receiving the body of the request.** This will + /// not override the timeout on sending the request header, nor on receiving the + /// response header. + /// 2. **Only override the timeout for successful responses.** The only bodies which + /// have bodies are successful ones. In all other cases, there's no body to + /// timeout for + /// 3. **Only override the timeout for complex body types.** Almost all clients are + /// able to display `text/plain` and `text/gemini` responses, and will not prompt + /// the user for these response types. This means that there is no reason to + /// expect a client to have a human-length response time for these MIME types. + /// Because of this, responses of this type will not be overridden. + /// + /// This method is used to override the timeout for responses meeting these specific + /// criteria. All other stages of the connection will use the timeout specified in + /// [`set_timeout()`](Self::set_timeout()). + /// + /// If this is set to [`None`], then the client will have the default amount of time + /// to both receive the header and the body. If this is set to [`Some`], the client + /// will have the default amount of time to recieve the header, and an *additional* + /// alotment of time to recieve the body. + /// + /// The default timeout for this is 30 seconds. + pub fn override_complex_body_timeout(mut self, timeout: Option) -> Self { + self.complex_body_timeout_override = timeout; + self + } + pub async fn serve(self, handler: F) -> Result<()> where F: Fn(Request) -> HandlerResponse + Send + Sync + 'static, @@ -165,6 +288,7 @@ impl Builder { listener: Arc::new(listener), handler: Arc::new(handler), timeout: self.timeout, + complex_timeout: self.complex_body_timeout_override, }; server.serve().await @@ -199,18 +323,6 @@ async fn receive_request(stream: &mut (impl AsyncBufRead + Unpin)) -> Result Result<()> { - send_response_header(response.header(), stream).await - .context("Failed to send response header")?; - - if let Some(body) = response.take_body() { - send_response_body(body, stream).await - .context("Failed to send response body")?; - } - - Ok(()) -} - async fn send_response_header(header: &ResponseHeader, stream: &mut (impl AsyncWrite + Unpin)) -> Result<()> { let header = format!( "{status} {meta}\r\n", From 25d575bee7e0a6ca083b4e1e12892924ea96dba7 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 01:30:08 -0500 Subject: [PATCH 3/6] Shorten references to tokio::time::timeout because I forgot I used an import for that woops sorry --- src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9bd962d..d57fc93 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,7 +122,7 @@ impl Server { // All the other code is doing one of two things. Either it's // // * code to add and handle timeouts (that's what all the async blocks and calls - // to tokio::time::timeout are), or + // to timeout are), or // * logic to decide whether to use the special case timeout handling (seperate // timeouts for the header and the body) vs the normal timeout handling (header, // body, and flush all as one timeout) @@ -151,7 +151,7 @@ impl Server { .await .context("Failed to flush response header") }; - tokio::time::timeout(self.timeout, fut_send_header) + timeout(self.timeout, fut_send_header) .await .context("Timed out while sending response header")??; @@ -164,7 +164,7 @@ impl Server { .await .context("Failed to flush response body") }; - tokio::time::timeout(cplx_timeout, fut_send_body) + timeout(cplx_timeout, fut_send_body) .await .context("Timed out while sending response body")??; @@ -189,7 +189,7 @@ impl Server { .await .context("Failed to flush response data") }; - tokio::time::timeout(self.timeout, fut_send_response) + timeout(self.timeout, fut_send_response) .await .context("Timed out while sending response data")??; From df362d1bc351b5c82897899cf004a6f1fc69c9df Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 01:43:57 -0500 Subject: [PATCH 4/6] Fixed bug where incorrect timeout was used. I got the mime types backwards haha pretend you didnt see that --- src/lib.rs | 59 +++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d57fc93..bb4ced9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,48 +128,49 @@ impl Server { // body, and flush all as one timeout) // // The split between the two cases happens at this very first if block. - // Everything in this deep chain of if's and if-let's is for the special case. If - // any one of the ifs fails, the code after the big if block is run, and that's - // the normal case. + // Everything in this if is for the special case. If any one of the ifs fails, + // the code after the big if block is run, and that's the normal case. // // Hope this helps! Emi <3 - if header.status == Status::SUCCESS && maybe_body.is_some() { - // aaaa let me have if-let chaining ;_; - if let "text/plain"|"text/gemini" = header.meta.as_str() { - if let Some(cplx_timeout) = self.complex_timeout { + if header.status == Status::SUCCESS && + maybe_body.is_some() && + header.meta.as_str() != "text/plain" && + header.meta.as_str() != "text/gemini" + { + if let Some(cplx_timeout) = self.complex_timeout { ////////////// Use the special case timeout override ///////////////////////////// - // Send the header & flush - let fut_send_header = async { - send_response_header(response.header(), stream).await - .context("Failed to write response header")?; + // Send the header & flush + let fut_send_header = async { + send_response_header(response.header(), stream).await + .context("Failed to write response header")?; - stream.flush() - .await - .context("Failed to flush response header") - }; - timeout(self.timeout, fut_send_header) + stream.flush() .await - .context("Timed out while sending response header")??; + .context("Failed to flush response header") + }; + timeout(self.timeout, fut_send_header) + .await + .context("Timed out while sending response header")??; - // Send the body & flush - let fut_send_body = async { - send_response_body(maybe_body.unwrap(), stream).await - .context("Failed to write response body")?; + // Send the body & flush + let fut_send_body = async { + send_response_body(maybe_body.unwrap(), stream).await + .context("Failed to write response body")?; - stream.flush() - .await - .context("Failed to flush response body") - }; - timeout(cplx_timeout, fut_send_body) + stream.flush() .await - .context("Timed out while sending response body")??; + .context("Failed to flush response body") + }; + timeout(cplx_timeout, fut_send_body) + .await + .context("Timed out while sending response body")??; - return Ok(()) - } + + return Ok(()) } } From aeeee86aae4e37534899e0d0f73198e494f9a817 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 12:37:41 -0500 Subject: [PATCH 5/6] Updated changelog for complex mime timeout override --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1d7f18..47c5d1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added - `document` API for creating Gemini documents -- preliminary timeout API by [@Alch-Emi](https://github.com/Alch-Emi) +- preliminary timeout API, incl a special case for complex MIMEs by [@Alch-Emi](https://github.com/Alch-Emi) - `Response::success_with_body` by [@Alch-Emi](https://github.com/Alch-Emi) - `redirect_temporary_lossy` for `Response` and `ResponseHeader` - `bad_request_lossy` for `Response` and `ResponseHeader` From 46ab84ba04ceba67ac7c06cfcbced8f933b146b4 Mon Sep 17 00:00:00 2001 From: panicbit Date: Thu, 19 Nov 2020 19:29:17 +0100 Subject: [PATCH 6/6] streamline send_response --- src/lib.rs | 115 +++++++++++++++++++--------------------------------- src/util.rs | 8 ++++ 2 files changed, 49 insertions(+), 74 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b8ed975..b8e00d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,6 +21,7 @@ use tokio_rustls::{rustls, TlsAcceptor}; use rustls::*; use anyhow::*; use lazy_static::lazy_static; +use crate::util::opt_timeout; pub mod types; pub mod util; @@ -113,91 +114,46 @@ impl Server { let maybe_body = response.take_body(); let header = response.header(); - // Okay, I know this method looks really complicated, but I promise it's not. - // There's really only three things this method does: - // - // * Send the response header - // * Send the response body - // * Flush the stream - // - // All the other code is doing one of two things. Either it's - // - // * code to add and handle timeouts (that's what all the async blocks and calls - // to timeout are), or - // * logic to decide whether to use the special case timeout handling (seperate - // timeouts for the header and the body) vs the normal timeout handling (header, - // body, and flush all as one timeout) - // - // The split between the two cases happens at this very first if block. - // Everything in this if is for the special case. If any one of the ifs fails, - // the code after the big if block is run, and that's the normal case. - // - // Hope this helps! Emi <3 - - if header.status == Status::SUCCESS && + let use_complex_timeout = + header.status.is_success() && maybe_body.is_some() && header.meta.as_str() != "text/plain" && - header.meta.as_str() != "text/gemini" - { - if let Some(cplx_timeout) = self.complex_timeout { + header.meta.as_str() != "text/gemini" && + self.complex_timeout.is_some(); + let send_general_timeout; + let send_header_timeout; + let send_body_timeout; - ////////////// Use the special case timeout override ///////////////////////////// - - // Send the header & flush - let fut_send_header = async { - send_response_header(response.header(), stream).await - .context("Failed to write response header")?; - - stream.flush() - .await - .context("Failed to flush response header") - }; - timeout(self.timeout, fut_send_header) - .await - .context("Timed out while sending response header")??; - - // Send the body & flush - let fut_send_body = async { - send_response_body(maybe_body.unwrap(), stream).await - .context("Failed to write response body")?; - - stream.flush() - .await - .context("Failed to flush response body") - }; - timeout(cplx_timeout, fut_send_body) - .await - .context("Timed out while sending response body")??; - - - return Ok(()) - } + if use_complex_timeout { + send_general_timeout = None; + send_header_timeout = Some(self.timeout); + send_body_timeout = self.complex_timeout; + } else { + send_general_timeout = Some(self.timeout); + send_header_timeout = None; + send_body_timeout = None; } - - ///////////// Use the normal timeout ///////////////////////////////////////////// - - let fut_send_response = async { - send_response_header(response.header(), stream).await + opt_timeout(send_general_timeout, async { + // Send the header + opt_timeout(send_header_timeout, send_response_header(response.header(), stream)) + .await + .context("Timed out while sending response header")? .context("Failed to write response header")?; - if let Some(body) = maybe_body { - send_response_body(body, stream).await - .context("Failed to write response body")?; - } - - stream.flush() + // Send the body + opt_timeout(send_body_timeout, maybe_send_response_body(maybe_body, stream)) .await - .context("Failed to flush response data") - }; - timeout(self.timeout, fut_send_response) - .await - .context("Timed out while sending response data")??; + .context("Timed out while sending response body")? + .context("Failed to write response body")?; + + Ok::<_,Error>(()) + }) + .await + .context("Timed out while sending response data")??; Ok(()) - - ////////////////////////////////////////////////////////////////////////////////// } } @@ -377,6 +333,15 @@ async fn send_response_header(header: &ResponseHeader, stream: &mut (impl AsyncW ); stream.write_all(header.as_bytes()).await?; + stream.flush().await?; + + Ok(()) +} + +async fn maybe_send_response_body(maybe_body: Option, stream: &mut (impl AsyncWrite + Unpin)) -> Result<()> { + if let Some(body) = maybe_body { + send_response_body(body, stream).await?; + } Ok(()) } @@ -387,6 +352,8 @@ async fn send_response_body(body: Body, stream: &mut (impl AsyncWrite + Unpin)) Body::Reader(mut reader) => { io::copy(&mut reader, stream).await?; }, } + stream.flush().await?; + Ok(()) } diff --git a/src/util.rs b/src/util.rs index c48b8bf..5c623aa 100644 --- a/src/util.rs +++ b/src/util.rs @@ -14,6 +14,7 @@ use crate::types::Response; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::task::Poll; use futures_core::future::Future; +use tokio::time; #[cfg(feature="serve_dir")] pub async fn serve_file>(path: P, mime: &Mime) -> Result { @@ -155,3 +156,10 @@ impl Future for HandlerCatchUnwind { } } } + +pub(crate) async fn opt_timeout(duration: Option, future: impl Future) -> Result { + match duration { + Some(duration) => time::timeout(duration, future).await, + None => Ok(future.await), + } +}