use a concrete error type in async-h1

Author: jbr

Cargo.toml

@@ -28,6 +28,7 @@ log = "0.4.11"
 pin-project = "1.0.2"
 async-channel = "1.5.1"
 async-dup = "1.2.2"
+thiserror = "1.0.22"
 
 [dev-dependencies]
 pretty_assertions = "0.6.1"

src/client/decode.rs

@@ -1,22 +1,22 @@
 use async_std::io::{BufReader, Read};
 use async_std::prelude::*;
-use http_types::{ensure, ensure_eq, format_err};
+use http_types::content::ContentLength;
 use http_types::{
-    headers::{CONTENT_LENGTH, DATE, TRANSFER_ENCODING},
+    headers::{DATE, TRANSFER_ENCODING},
     Body, Response, StatusCode,
 };
 
 use std::convert::TryFrom;
 
-use crate::chunked::ChunkedDecoder;
 use crate::date::fmt_http_date;
+use crate::{chunked::ChunkedDecoder, Error};
 use crate::{MAX_HEADERS, MAX_HEAD_LENGTH};
 
 const CR: u8 = b'\r';
 const LF: u8 = b'\n';
 
 /// Decode an HTTP response on the client.
-pub async fn decode<R>(reader: R) -> http_types::Result<Response>
+pub async fn decode<R>(reader: R) -> crate::Result<Option<Response>>
 where
     R: Read + Unpin + Send + Sync + 'static,
 {
@@ -29,13 +29,14 @@ where
     loop {
         let bytes_read = reader.read_until(LF, &mut buf).await?;
         // No more bytes are yielded from the stream.
-        assert!(bytes_read != 0, "Empty response"); // TODO: ensure?
+        if bytes_read == 0 {
+            return Ok(None);
+        }
 
         // Prevent CWE-400 DDOS with large HTTP Headers.
-        ensure!(
-            buf.len() < MAX_HEAD_LENGTH,
-            "Head byte length should be less than 8kb"
-        );
+        if buf.len() >= MAX_HEAD_LENGTH {
+            return Err(Error::HeadersTooLong);
+        }
 
         // We've hit the end delimiter of the stream.
         let idx = buf.len() - 1;
@@ -49,17 +50,23 @@ where
 
     // Convert our header buf into an httparse instance, and validate.
     let status = httparse_res.parse(&buf)?;
-    ensure!(!status.is_partial(), "Malformed HTTP head");
+    if status.is_partial() {
+        return Err(Error::PartialHead);
+    }
 
-    let code = httparse_res.code;
-    let code = code.ok_or_else(|| format_err!("No status code found"))?;
+    let code = httparse_res.code.ok_or_else(|| Error::MissingStatusCode)?;
 
     // Convert httparse headers + body into a `http_types::Response` type.
-    let version = httparse_res.version;
-    let version = version.ok_or_else(|| format_err!("No version found"))?;
-    ensure_eq!(version, 1, "Unsupported HTTP version");
+    let version = httparse_res.version.ok_or_else(|| Error::MissingVersion)?;
+
+    if version != 1 {
+        return Err(Error::UnsupportedVersion(version));
+    }
+
+    let status_code =
+        StatusCode::try_from(code).map_err(|_| Error::UnrecognizedStatusCode(code))?;
+    let mut res = Response::new(status_code);
 
-    let mut res = Response::new(StatusCode::try_from(code)?);
     for header in httparse_res.headers.iter() {
         res.append_header(header.name, std::str::from_utf8(header.value)?);
     }
@@ -69,13 +76,13 @@ where
         res.insert_header(DATE, &format!("date: {}\r\n", date)[..]);
     }
 
-    let content_length = res.header(CONTENT_LENGTH);
+    let content_length =
+        ContentLength::from_headers(&res).map_err(|_| Error::MalformedHeader("content-length"))?;
     let transfer_encoding = res.header(TRANSFER_ENCODING);
 
-    ensure!(
-        content_length.is_none() || transfer_encoding.is_none(),
-        "Unexpected Content-Length header"
-    );
+    if content_length.is_some() && transfer_encoding.is_some() {
+        return Err(Error::UnexpectedHeader("content-length"));
+    }
 
     if let Some(encoding) = transfer_encoding {
         if encoding.last().as_str() == "chunked" {
@@ -84,16 +91,16 @@ where
             res.set_body(Body::from_reader(reader, None));
 
             // Return the response.
-            return Ok(res);
+            return Ok(Some(res));
         }
     }
 
     // Check for Content-Length.
-    if let Some(len) = content_length {
-        let len = len.last().as_str().parse::<usize>()?;
-        res.set_body(Body::from_reader(reader.take(len as u64), Some(len)));
+    if let Some(content_length) = content_length {
+        let len = content_length.len();
+        res.set_body(Body::from_reader(reader.take(len), Some(len as usize)));
     }
 
     // Return the response.
-    Ok(res)
+    Ok(Some(res))
 }

src/client/mod.rs

@@ -10,7 +10,7 @@ pub use decode::decode;
 pub use encode::Encoder;
 
 /// Opens an HTTP/1.1 connection to a remote host.
-pub async fn connect<RW>(mut stream: RW, req: Request) -> http_types::Result<Response>
+pub async fn connect<RW>(mut stream: RW, req: Request) -> crate::Result<Option<Response>>
 where
     RW: Read + Write + Send + Sync + Unpin + 'static,
 {

src/error.rs

@@ -0,0 +1,84 @@
+use std::str::Utf8Error;
+
+use http_types::url;
+use thiserror::Error;
+
+/// Concrete errors that occur within async-h1
+#[derive(Error, Debug)]
+#[non_exhaustive]
+pub enum Error {
+    /// [`std::io::Error`]
+    #[error(transparent)]
+    IO(#[from] std::io::Error),
+
+    /// [`url::ParseError`]
+    #[error(transparent)]
+    Url(#[from] url::ParseError),
+
+    /// this error describes a malformed request with a path that does
+    /// not start with / or http:// or https://
+    #[error("unexpected uri format")]
+    UnexpectedURIFormat,
+
+    /// this error describes a http 1.1 request that is missing a Host
+    /// header
+    #[error("mandatory host header missing")]
+    HostHeaderMissing,
+
+    /// this error describes a request that does not specify a path
+    #[error("request path missing")]
+    RequestPathMissing,
+
+    /// [`httparse::Error`]
+    #[error(transparent)]
+    Httparse(#[from] httparse::Error),
+
+    /// an incomplete http head
+    #[error("partial http head")]
+    PartialHead,
+
+    /// we were unable to parse a header
+    #[error("malformed http header {0}")]
+    MalformedHeader(&'static str),
+
+    /// async-h1 doesn't speak this http version
+    /// this error is deprecated
+    #[error("unsupported http version 1.{0}")]
+    UnsupportedVersion(u8),
+
+    /// we were unable to parse this http method
+    #[error("unsupported http method {0}")]
+    UnrecognizedMethod(String),
+
+    /// this request did not have a method
+    #[error("missing method")]
+    MissingMethod,
+
+    /// this request did not have a status code
+    #[error("missing status code")]
+    MissingStatusCode,
+
+    /// we were unable to parse this http method
+    #[error("unrecognized http status code {0}")]
+    UnrecognizedStatusCode(u16),
+
+    /// this request did not have a version, but we expect one
+    /// this error is deprecated
+    #[error("missing version")]
+    MissingVersion,
+
+    /// we expected utf8, but there was an encoding error
+    #[error(transparent)]
+    EncodingError(#[from] Utf8Error),
+
+    /// we received a header that does not make sense in context
+    #[error("unexpected header: {0}")]
+    UnexpectedHeader(&'static str),
+
+    /// for security reasons, we do not allow request headers beyond 8kb.
+    #[error("Head byte length should be less than 8kb")]
+    HeadersTooLong,
+}
+
+/// this crate's result type
+pub type Result<T> = std::result::Result<T, Error>;

src/lib.rs

@@ -116,6 +116,8 @@ use async_std::io::Cursor;
 use body_encoder::BodyEncoder;
 pub use client::connect;
 pub use server::{accept, accept_with_opts, ServerOptions};
+mod error;
+pub use error::{Error, Result};
 
 #[derive(Debug)]
 pub(crate) enum EncoderState {

src/server/decode.rs

@@ -1,14 +1,16 @@
 //! Process HTTP connections on the server.
 
-use std::str::FromStr;
-
 use async_dup::{Arc, Mutex};
 use async_std::io::{BufReader, Read, Write};
 use async_std::{prelude::*, task};
-use http_types::content::ContentLength;
-use http_types::headers::{EXPECT, TRANSFER_ENCODING};
-use http_types::{ensure, ensure_eq, format_err};
-use http_types::{Body, Method, Request, Url};
+use http_types::{
+    content::ContentLength,
+    headers::{EXPECT, TRANSFER_ENCODING},
+    Version,
+};
+use http_types::{Body, Request, Url};
+
+use crate::{Error, Result};
 
 use super::body_reader::BodyReader;
 use crate::chunked::ChunkedDecoder;
@@ -17,14 +19,11 @@ use crate::{MAX_HEADERS, MAX_HEAD_LENGTH};
 
 const LF: u8 = b'\n';
 
-/// The number returned from httparse when the request is HTTP 1.1
-const HTTP_1_1_VERSION: u8 = 1;
-
 const CONTINUE_HEADER_VALUE: &str = "100-continue";
 const CONTINUE_RESPONSE: &[u8] = b"HTTP/1.1 100 Continue\r\n\r\n";
 
 /// Decode an HTTP request on the server.
-pub async fn decode<IO>(mut io: IO) -> http_types::Result<Option<(Request, BodyReader<IO>)>>
+pub async fn decode<IO>(mut io: IO) -> Result<Option<(Request, BodyReader<IO>)>>
 where
     IO: Read + Write + Clone + Send + Sync + Unpin + 'static,
 {
@@ -42,10 +41,9 @@ where
         }
 
         // Prevent CWE-400 DDOS with large HTTP Headers.
-        ensure!(
-            buf.len() < MAX_HEAD_LENGTH,
-            "Head byte length should be less than 8kb"
-        );
+        if buf.len() >= MAX_HEAD_LENGTH {
+            return Err(Error::HeadersTooLong);
+        }
 
         // We've hit the end delimiter of the stream.
         let idx = buf.len() - 1;
@@ -57,44 +55,37 @@ where
     // Convert our header buf into an httparse instance, and validate.
     let status = httparse_req.parse(&buf)?;
 
-    ensure!(!status.is_partial(), "Malformed HTTP head");
-
-    // Convert httparse headers + body into a `http_types::Request` type.
-    let method = httparse_req.method;
-    let method = method.ok_or_else(|| format_err!("No method found"))?;
-
-    let version = httparse_req.version;
-    let version = version.ok_or_else(|| format_err!("No version found"))?;
+    if status.is_partial() {
+        return Err(Error::PartialHead);
+    }
 
-    ensure_eq!(
-        version,
-        HTTP_1_1_VERSION,
-        "Unsupported HTTP version 1.{}",
-        version
-    );
+    let method = httparse_req
+        .method
+        .ok_or_else(|| Error::MissingMethod)?
+        .parse()
+        .map_err(|_| Error::UnrecognizedMethod(httparse_req.method.unwrap().to_string()))?;
 
     let url = url_from_httparse_req(&httparse_req)?;
 
-    let mut req = Request::new(Method::from_str(method)?, url);
+    let mut req = Request::new(method, url);
 
-    req.set_version(Some(http_types::Version::Http1_1));
+    match httparse_req.version {
+        Some(1) => req.set_version(Some(Version::Http1_1)),
+        Some(version) => return Err(Error::UnsupportedVersion(version)),
+        None => return Err(Error::MissingVersion),
+    }
 
     for header in httparse_req.headers.iter() {
         req.append_header(header.name, std::str::from_utf8(header.value)?);
     }
 
-    let content_length = ContentLength::from_headers(&req)?;
+    let content_length =
+        ContentLength::from_headers(&req).map_err(|_| Error::MalformedHeader("content-length"))?;
     let transfer_encoding = req.header(TRANSFER_ENCODING);
 
-    // Return a 400 status if both Content-Length and Transfer-Encoding headers
-    // are set to prevent request smuggling attacks.
-    //
-    // https://tools.ietf.org/html/rfc7230#section-3.3.3
-    http_types::ensure_status!(
-        content_length.is_none() || transfer_encoding.is_none(),
-        400,
-        "Unexpected Content-Length header"
-    );
+    if content_length.is_some() && transfer_encoding.is_some() {
+        return Err(Error::UnexpectedHeader("content-length"));
+    }
 
     // Establish a channel to wait for the body to be read. This
     // allows us to avoid sending 100-continue in situations that
@@ -128,8 +119,8 @@ where
         let reader = BufReader::new(reader);
         req.set_body(Body::from_reader(reader, None));
         return Ok(Some((req, BodyReader::Chunked(reader_clone))));
-    } else if let Some(len) = content_length {
-        let len = len.len();
+    } else if let Some(content_length) = content_length {
+        let len = content_length.len();
         let reader = Arc::new(Mutex::new(reader.take(len)));
         req.set_body(Body::from_reader(
             BufReader::new(ReadNotifier::new(reader.clone(), body_read_sender)),
@@ -141,14 +132,14 @@ where
     }
 }
 
-fn url_from_httparse_req(req: &httparse::Request<'_, '_>) -> http_types::Result<Url> {
-    let path = req.path.ok_or_else(|| format_err!("No uri found"))?;
+fn url_from_httparse_req(req: &httparse::Request<'_, '_>) -> Result<Url> {
+    let path = req.path.ok_or_else(|| Error::RequestPathMissing)?;
 
     let host = req
         .headers
         .iter()
         .find(|x| x.name.eq_ignore_ascii_case("host"))
-        .ok_or_else(|| format_err!("Mandatory Host header missing"))?
+        .ok_or_else(|| Error::HostHeaderMissing)?
         .value;
 
     let host = std::str::from_utf8(host)?;
@@ -160,7 +151,7 @@ fn url_from_httparse_req(req: &httparse::Request<'_, '_>) -> http_types::Result<
     } else if req.method.unwrap().eq_ignore_ascii_case("connect") {
         Ok(Url::parse(&format!("http://{}/", path))?)
     } else {
-        Err(format_err!("unexpected uri format"))
+        Err(Error::UnexpectedURIFormat)
     }
 }