From 637b6ae1784ae76d495498cea89ede419881cbf7 Mon Sep 17 00:00:00 2001 From: Niklas Date: Thu, 12 Nov 2020 17:17:46 +0100 Subject: [PATCH 1/9] feat(http server): configurable request body limit --- benches/benches.rs | 2 +- examples/http.rs | 3 +- src/client/http/client.rs | 20 +----- src/client/http/mod.rs | 3 +- src/client/http/transport.rs | 71 +++++---------------- src/http/raw/core.rs | 2 +- src/http/raw/tests.rs | 5 +- src/http/server.rs | 5 +- src/http/tests.rs | 3 +- src/http/transport/background.rs | 85 ++++++------------------- src/http/transport/mod.rs | 23 ++++--- src/types/http.rs | 103 +++++++++++++++++++++++++++++++ src/types/mod.rs | 4 ++ 13 files changed, 171 insertions(+), 158 deletions(-) create mode 100644 src/types/http.rs diff --git a/benches/benches.rs b/benches/benches.rs index bda3fe1c78..04510e3861 100644 --- a/benches/benches.rs +++ b/benches/benches.rs @@ -17,7 +17,7 @@ fn concurrent_tasks() -> Vec { } async fn http_server(tx: Sender) { - let server = HttpServer::new("127.0.0.1:0").await.unwrap(); + let server = HttpServer::new("127.0.0.1:0", HttpConfig { max_request_body_size: u32::MAX }).await.unwrap(); let mut say_hello = server.register_method("say_hello".to_string()).unwrap(); tx.send(*server.local_addr()).unwrap(); loop { diff --git a/examples/http.rs b/examples/http.rs index 4eb4b3708c..ee4938e6bb 100644 --- a/examples/http.rs +++ b/examples/http.rs @@ -52,9 +52,8 @@ async fn main() -> Result<(), Box> { } async fn run_server(server_started_tx: Sender<()>, url: &str) { - let server = HttpServer::new(url).await.unwrap(); + let server = HttpServer::new(url, HttpConfig::default()).await.unwrap(); let mut say_hello = server.register_method("say_hello".to_string()).unwrap(); - server_started_tx.send(()).unwrap(); loop { let r = say_hello.next().await; diff --git a/src/client/http/client.rs b/src/client/http/client.rs index 3874640b1d..584c4d94c7 100644 --- a/src/client/http/client.rs +++ b/src/client/http/client.rs @@ -1,18 +1,9 @@ use crate::client::http::transport::HttpTransportClient; use crate::types::client::{Error, Mismatch}; +use crate::types::http::HttpConfig; use crate::types::jsonrpc::{self, JsonValue}; use std::sync::atomic::{AtomicU64, Ordering}; -/// Default maximum request body size (10 MB). -const DEFAULT_MAX_BODY_SIZE_TEN_MB: u32 = 10 * 1024 * 1024; - -/// HTTP configuration. -#[derive(Copy, Clone)] -pub struct HttpConfig { - /// Maximum request body size in bytes. - pub max_request_body_size: u32, -} - /// JSON-RPC HTTP Client that provides functionality to perform method calls and notifications. /// /// WARNING: The async methods must be executed on [Tokio 0.2](https://docs.rs/tokio/0.2.22/tokio). @@ -23,19 +14,12 @@ pub struct HttpClient { request_id: AtomicU64, } -impl Default for HttpConfig { - fn default() -> Self { - Self { max_request_body_size: DEFAULT_MAX_BODY_SIZE_TEN_MB } - } -} - impl HttpClient { /// Initializes a new HTTP client. /// /// Fails when the URL is invalid. pub fn new(target: impl AsRef, config: HttpConfig) -> Result { - let transport = HttpTransportClient::new(target, config.max_request_body_size) - .map_err(|e| Error::TransportError(Box::new(e)))?; + let transport = HttpTransportClient::new(target, config).map_err(|e| Error::TransportError(Box::new(e)))?; Ok(Self { transport, request_id: AtomicU64::new(0) }) } diff --git a/src/client/http/mod.rs b/src/client/http/mod.rs index 31a7121236..f98e1c4209 100644 --- a/src/client/http/mod.rs +++ b/src/client/http/mod.rs @@ -4,5 +4,6 @@ mod transport; #[cfg(test)] mod tests; -pub use client::{HttpClient, HttpConfig}; +pub use crate::types::http::HttpConfig; +pub use client::HttpClient; pub use transport::HttpTransportClient; diff --git a/src/client/http/transport.rs b/src/client/http/transport.rs index b85b890ab3..27388be3af 100644 --- a/src/client/http/transport.rs +++ b/src/client/http/transport.rs @@ -6,8 +6,10 @@ // that we need to be guaranteed that hyper doesn't re-use an existing connection if we ever reset // the JSON-RPC request id to a value that might have already been used. -use crate::types::jsonrpc; -use futures::StreamExt; +use crate::types::{ + http::{read_http_body, HttpConfig}, + jsonrpc, +}; use thiserror::Error; const CONTENT_TYPE_JSON: &str = "application/json"; @@ -20,15 +22,15 @@ pub struct HttpTransportClient { /// HTTP client, client: hyper::Client, /// Configurable max request body size - max_request_body_size: u32, + config: HttpConfig, } impl HttpTransportClient { /// Initializes a new HTTP client. - pub fn new(target: impl AsRef, max_request_body_size: u32) -> Result { - let target = url::Url::parse(target.as_ref()).map_err(|e| Error::Url(format!("Invalid URL: {}", e)))?; + pub fn new(target: impl AsRef, config: HttpConfig) -> Result { + let target = url::Url::parse(target.as_ref()).map_err(|e| Error::Url(format!("Invalid URL: {}", e).into()))?; if target.scheme() == "http" { - Ok(HttpTransportClient { client: hyper::Client::new(), target, max_request_body_size }) + Ok(HttpTransportClient { client: hyper::Client::new(), target, config }) } else { Err(Error::Url("URL scheme not supported, expects 'http'".into())) } @@ -39,7 +41,7 @@ impl HttpTransportClient { let body = jsonrpc::to_vec(&request).map_err(Error::Serialization)?; log::debug!("send: {}", request); - if body.len() > self.max_request_body_size as usize { + if body.len() > self.config.max_request_body_size as usize { return Err(Error::RequestTooLarge); } @@ -70,22 +72,8 @@ impl HttpTransportClient { request: jsonrpc::Request, ) -> Result { let response = self.send_request(request).await?; - let body_size = read_content_length(response.headers()).unwrap_or(0); - let mut body_fut: hyper::Body = response.into_body(); - - if body_size > self.max_request_body_size { - return Err(Error::RequestTooLarge); - } - - let mut body = Vec::with_capacity(body_size as usize); - - while let Some(chunk) = body_fut.next().await { - let chunk = chunk.map_err(|e| Error::Http(Box::new(e)))?; - if chunk.len() + body.len() > self.max_request_body_size as usize { - return Err(Error::RequestTooLarge); - } - body.extend_from_slice(&chunk); - } + let (parts, body) = response.into_parts(); + let body = read_http_body(&parts.headers, body, self.config).await.map_err(|e| Error::Http(Box::new(e)))?; // Note that we don't check the Content-Type of the request. This is deemed // unnecessary, as a parsing error while happen anyway. @@ -95,23 +83,6 @@ impl HttpTransportClient { } } -// Read `content_length` from HTTP Header. -// -// Returns `Some(val)` if `content_length` contains exactly one value. -// None otherwise. -fn read_content_length(header: &hyper::header::HeaderMap) -> Option { - let values = header.get_all("content-length"); - let mut iter = values.iter(); - let content_length = iter.next()?; - if iter.next().is_some() { - return None; - } - - // HTTP Content-Length indicates number of bytes in decimal. - let length = content_length.to_str().ok()?; - u32::from_str_radix(length, 10).ok() -} - /// Error that can happen during a request. #[derive(Debug, Error)] pub enum Error { @@ -150,20 +121,22 @@ pub enum Error { #[cfg(test)] mod tests { - use super::{read_content_length, Error, HttpTransportClient}; + use super::{Error, HttpTransportClient}; + use crate::types::http::HttpConfig; use crate::types::jsonrpc::{Call, Id, MethodCall, Params, Request, Version}; #[test] fn invalid_http_url_rejected() { - let err = HttpTransportClient::new("ws://localhost:9933", 1337).unwrap_err(); + let err = HttpTransportClient::new("ws://localhost:9933", HttpConfig::default()).unwrap_err(); assert!(matches!(err, Error::Url(_))); } #[tokio::test] async fn request_limit_works() { let eighty_bytes_limit = 80; - let client = HttpTransportClient::new("http://localhost:9933", eighty_bytes_limit).unwrap(); - assert_eq!(client.max_request_body_size, eighty_bytes_limit); + let client = + HttpTransportClient::new("http://localhost:9933", HttpConfig { max_request_body_size: 80 }).unwrap(); + assert_eq!(client.config.max_request_body_size, eighty_bytes_limit); let request = Request::Single(Call::MethodCall(MethodCall { jsonrpc: Version::V2, @@ -176,14 +149,4 @@ mod tests { let response = client.send_request(request).await.unwrap_err(); assert!(matches!(response, Error::RequestTooLarge)); } - - #[test] - fn read_content_length_works() { - let mut header = hyper::header::HeaderMap::new(); - header.insert(hyper::header::CONTENT_LENGTH, "177".parse().unwrap()); - assert_eq!(read_content_length(&header), Some(177)); - - header.append(hyper::header::CONTENT_LENGTH, "999".parse().unwrap()); - assert_eq!(read_content_length(&header), None); - } } diff --git a/src/http/raw/core.rs b/src/http/raw/core.rs index dd3b7e14fb..547d9be852 100644 --- a/src/http/raw/core.rs +++ b/src/http/raw/core.rs @@ -146,7 +146,7 @@ impl From for RawServer { impl<'a> RawServerRequest<'a> { /// Returns the id of the request. /// - /// If this request object is dropped, you can retreive it again later by calling + /// If this request object is dropped, you can retrieve it again later by calling /// [`request_by_id`](crate::raw::RawServer::request_by_id). pub fn id(&self) -> RawServerRequestId { RawServerRequestId { inner: self.inner.id() } diff --git a/src/http/raw/tests.rs b/src/http/raw/tests.rs index a33898e3ab..313e65274a 100644 --- a/src/http/raw/tests.rs +++ b/src/http/raw/tests.rs @@ -28,13 +28,14 @@ use crate::client::HttpTransportClient; use crate::http::{HttpRawServer, HttpRawServerEvent, HttpTransportServer}; +use crate::types::http::HttpConfig; use crate::types::jsonrpc::{self, Call, MethodCall, Notification, Params, Request, Version}; use serde_json::Value; async fn connection_context() -> (HttpTransportClient, HttpRawServer) { - let server = HttpTransportServer::new(&"127.0.0.1:0".parse().unwrap()).await.unwrap(); + let server = HttpTransportServer::new(&"127.0.0.1:0".parse().unwrap(), HttpConfig::default()).await.unwrap(); let uri = format!("http://{}", server.local_addr()); - let client = HttpTransportClient::new(&uri, 10 * 1024 * 1024).unwrap(); + let client = HttpTransportClient::new(&uri, HttpConfig::default()).unwrap(); (client, server.into()) } diff --git a/src/http/server.rs b/src/http/server.rs index f58e0f7829..e2c4fb0d59 100644 --- a/src/http/server.rs +++ b/src/http/server.rs @@ -26,6 +26,7 @@ use crate::http::raw::{RawServer, RawServerEvent, RawServerRequestId}; use crate::http::transport::HttpTransportServer; +use crate::types::http::HttpConfig; use crate::types::jsonrpc::{self, JsonValue}; use crate::types::server::Error; @@ -111,9 +112,9 @@ enum FrontToBack { impl Server { /// Initializes a new server based upon this raw server. - pub async fn new(url: &str) -> Result> { + pub async fn new(url: &str, config: HttpConfig) -> Result> { let sockaddr = url.parse()?; - let transport_server = HttpTransportServer::new(&sockaddr).await?; + let transport_server = HttpTransportServer::new(&sockaddr, config).await?; let local_addr = *transport_server.local_addr(); // We use an unbounded channel because the only exchanged messages concern registering diff --git a/src/http/tests.rs b/src/http/tests.rs index 6e40bcdc71..95cb2bca46 100644 --- a/src/http/tests.rs +++ b/src/http/tests.rs @@ -1,6 +1,7 @@ #![cfg(test)] use crate::http::HttpServer; +use crate::types::http::HttpConfig; use crate::types::jsonrpc::JsonValue; use futures::channel::oneshot::{self, Sender}; use futures::future::FutureExt; @@ -10,7 +11,7 @@ use jsonrpsee_test_utils::types::{Id, StatusCode}; use std::net::SocketAddr; async fn server(server_started_tx: Sender) { - let server = HttpServer::new("127.0.0.1:0").await.unwrap(); + let server = HttpServer::new("127.0.0.1:0", HttpConfig::default()).await.unwrap(); let mut hello = server.register_method("say_hello".to_owned()).unwrap(); let mut add = server.register_method("add".to_owned()).unwrap(); let mut notif = server.register_notification("notif".to_owned(), false).unwrap(); diff --git a/src/http/transport/background.rs b/src/http/transport/background.rs index 54f5b640b5..7531a5ed15 100644 --- a/src/http/transport/background.rs +++ b/src/http/transport/background.rs @@ -26,7 +26,10 @@ use crate::http::server_utils::access_control::AccessControl; use crate::http::transport::response; -use crate::types::jsonrpc; +use crate::types::{ + http::{self, HttpConfig}, + jsonrpc, +}; use futures::{channel::mpsc, channel::oneshot, prelude::*}; use hyper::service::{make_service_fn, service_fn}; use hyper::Error; @@ -52,13 +55,17 @@ impl BackgroundHttp { /// /// In addition to `Self`, also returns the local address the server ends up listening on, /// which might be different than the one passed as parameter. - pub async fn bind(addr: &SocketAddr) -> Result<(BackgroundHttp, SocketAddr), Box> { - Self::bind_with_acl(addr, AccessControl::default()).await + pub async fn bind( + addr: &SocketAddr, + config: HttpConfig, + ) -> Result<(BackgroundHttp, SocketAddr), Box> { + Self::bind_with_acl(addr, AccessControl::default(), config).await } pub async fn bind_with_acl( addr: &SocketAddr, access_control: AccessControl, + config: HttpConfig, ) -> Result<(BackgroundHttp, SocketAddr), Box> { let (tx, rx) = mpsc::channel(4); @@ -69,7 +76,7 @@ impl BackgroundHttp { Ok::<_, Error>(service_fn(move |req| { let mut tx = tx.clone(); let access_control = access_control.clone(); - async move { Ok::<_, Error>(process_request(req, &mut tx, &access_control).await) } + async move { Ok::<_, Error>(process_request(req, &mut tx, &access_control, config).await) } })) } }); @@ -124,6 +131,7 @@ async fn process_request( request: hyper::Request, fg_process_tx: &mut mpsc::Sender, access_control: &AccessControl, + config: HttpConfig, ) -> hyper::Response { // Process access control if access_control.deny_host(&request) { @@ -136,18 +144,17 @@ async fn process_request( return response::invalid_allow_headers(); } - /* - // Read metadata - let metadata = self.jsonrpc_handler.extractor.read_metadata(&request); - */ - // Proceed match *request.method() { // Validate the ContentType header // to prevent Cross-Origin XHRs with text/plain hyper::Method::POST if is_json(request.headers().get("content-type")) => { - let request = match body_to_request(request.into_body()).await { - Ok(b) => b, + let (parts, body) = request.into_parts(); + let request = match http::read_http_body(&parts.headers, body, config).await { + Ok(body) => match jsonrpc::from_slice(&body) { + Ok(response) => response, + Err(_e) => return response::parse_error(), + }, Err(e) => match (e.kind(), e.into_inner()) { (io::ErrorKind::InvalidData, _) => return response::parse_error(), (io::ErrorKind::UnexpectedEof, _) => return response::parse_error(), @@ -187,59 +194,3 @@ fn is_json(content_type: Option<&hyper::header::HeaderValue>) -> bool { _ => false, } } - -/// Converts a `hyper` body into a structured JSON object. -/// -/// Enforces a size limit on the body. -async fn body_to_request(mut body: hyper::Body) -> Result { - let mut json_body = Vec::new(); - while let Some(chunk) = body.next().await { - let chunk = match chunk { - Ok(c) => c, - Err(err) => return Err(io::Error::new(io::ErrorKind::Other, err.to_string())), // TODO: - }; - json_body.extend_from_slice(&chunk); - if json_body.len() >= 16384 { - // TODO: some limit - return Err(io::Error::new(io::ErrorKind::Other, "request too large")); - } - } - - Ok(serde_json::from_slice(&json_body)?) -} - -#[cfg(test)] -mod tests { - use super::body_to_request; - - #[test] - fn body_to_request_works() { - let s = r#"[{"a":"hello"}]"#; - let expected: super::jsonrpc::Request = serde_json::from_str(s).unwrap(); - let req = futures::executor::block_on(async move { - let body = hyper::Body::from(s); - body_to_request(body).await.unwrap() - }); - assert_eq!(req, expected); - } - - #[test] - fn body_to_request_size_limit_json() { - let huge_body = - serde_json::to_vec(&(0..32768).map(|_| serde_json::Value::from("test")).collect::>()).unwrap(); - - futures::executor::block_on(async move { - let body = hyper::Body::from(huge_body); - assert!(body_to_request(body).await.is_err()); - }); - } - - #[test] - fn body_to_request_size_limit_garbage() { - let huge_body = (0..100_000).map(|_| rand::random::()).collect::>(); - futures::executor::block_on(async move { - let body = hyper::Body::from(huge_body); - assert!(body_to_request(body).await.is_err()); - }); - } -} diff --git a/src/http/transport/mod.rs b/src/http/transport/mod.rs index 8b630a23c6..8481b5e1e9 100644 --- a/src/http/transport/mod.rs +++ b/src/http/transport/mod.rs @@ -28,6 +28,7 @@ mod background; mod response; use crate::http::server_utils::access_control::AccessControl; +use crate::types::http::HttpConfig; use crate::types::jsonrpc; use fnv::FnvHashMap; @@ -68,7 +69,7 @@ pub struct HttpTransportServer { /// Next identifier to use when inserting an element in `requests`. next_request_id: u64, - /// The identifier is lineraly increasing and is never leaked on the wire or outside of this + /// The identifier is linearly increasing and is never leaked on the wire or outside of this /// module. Therefore there is no risk of hash collision and using a `FnvHashMap` is safe. requests: FnvHashMap>>, } @@ -83,8 +84,11 @@ impl HttpTransportServer { // > starting to listen on a port is an asynchronous operation, but the hyper library // > hides this to us. In order to be future-proof, this function is async, so that we // > might switch out to a different library later without breaking the API. - pub async fn new(addr: &SocketAddr) -> Result> { - let (background_thread, local_addr) = background::BackgroundHttp::bind(addr).await?; + pub async fn new( + addr: &SocketAddr, + config: HttpConfig, + ) -> Result> { + let (background_thread, local_addr) = background::BackgroundHttp::bind(addr, config).await?; Ok(HttpTransportServer { background_thread, local_addr, requests: Default::default(), next_request_id: 0 }) } @@ -92,9 +96,10 @@ impl HttpTransportServer { pub async fn bind_with_acl( addr: &SocketAddr, access_control: AccessControl, + config: HttpConfig, ) -> Result> { - let (background_thread, local_addr) = background::BackgroundHttp::bind_with_acl(addr, access_control).await?; - + let (background_thread, local_addr) = + background::BackgroundHttp::bind_with_acl(addr, access_control, config).await?; Ok(HttpTransportServer { background_thread, local_addr, requests: Default::default(), next_request_id: 0 }) } @@ -149,7 +154,7 @@ impl HttpTransportServer { /// You can pass `None` in order to destroy the request object without sending back anything. /// /// The implementation blindly sends back the response and doesn't check whether there is any - /// correspondance with the request in terms of logic. For example, `respond` will accept + /// correspondence with the request in terms of logic. For example, `respond` will accept /// sending back a batch of six responses even if the original request was a single /// notification. /// @@ -197,14 +202,14 @@ impl HttpTransportServer { #[cfg(test)] mod tests { - use super::HttpTransportServer; + use super::{HttpConfig, HttpTransportServer}; #[test] fn error_if_port_occupied() { futures::executor::block_on(async move { let addr = "127.0.0.1:0".parse().unwrap(); - let server1 = HttpTransportServer::new(&addr).await.unwrap(); - assert!(HttpTransportServer::new(server1.local_addr()).await.is_err()); + let server1 = HttpTransportServer::new(&addr, HttpConfig::default()).await.unwrap(); + assert!(HttpTransportServer::new(server1.local_addr(), HttpConfig::default()).await.is_err()); }); } } diff --git a/src/types/http.rs b/src/types/http.rs new file mode 100644 index 0000000000..31c0a78eee --- /dev/null +++ b/src/types/http.rs @@ -0,0 +1,103 @@ +//! Shared HTTP types + +use futures::StreamExt; +use std::io::{Error, ErrorKind}; + +/// Default maximum request body size (10 MB). +const DEFAULT_MAX_BODY_SIZE_TEN_MB: u32 = 10 * 1024 * 1024; + +/// HTTP configuration. +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct HttpConfig { + /// Maximum request body size in bytes. + pub max_request_body_size: u32, +} + +impl Default for HttpConfig { + fn default() -> Self { + Self { max_request_body_size: DEFAULT_MAX_BODY_SIZE_TEN_MB } + } +} + +/// Read response body from a received request with configured `HTTP` settings. +// TODO: move somewhere else!!! +pub async fn read_http_body( + header: &hyper::HeaderMap, + mut body: hyper::Body, + config: HttpConfig, +) -> Result, Error> { + let body_size = read_http_content_length(&header).unwrap_or(0); + + if body_size > config.max_request_body_size { + return Err(Error::new( + ErrorKind::Other, + format!("HTTP request body too large, got: {} max: {}", body_size, config.max_request_body_size), + )); + } + + let mut received_data = Vec::with_capacity(body_size as usize); + + while let Some(chunk) = body.next().await { + let chunk = chunk.map_err(|e| Error::new(ErrorKind::Other, e.to_string()))?; + let body_length = chunk.len() + received_data.len(); + if body_length > config.max_request_body_size as usize { + return Err(Error::new( + ErrorKind::Other, + format!("HTTP request body too large, got: {} max: {}", body_length, config.max_request_body_size), + )); + } + received_data.extend_from_slice(&chunk); + } + Ok(received_data) +} + +// Read `content_length` from HTTP Header. +// +// Returns `Some(val)` if `content_length` contains exactly one value. +// None otherwise. +fn read_http_content_length(headers: &hyper::header::HeaderMap) -> Option { + let values = headers.get_all("content-length"); + let mut iter = values.iter(); + let content_length = iter.next()?; + if iter.next().is_some() { + return None; + } + + // HTTP Content-Length indicates number of bytes in decimal. + let length = content_length.to_str().ok()?; + u32::from_str_radix(length, 10).ok() +} + +#[cfg(test)] +mod tests { + use super::{read_http_body, read_http_content_length, HttpConfig}; + use crate::types::jsonrpc; + + #[tokio::test] + async fn body_to_request_works() { + let s = r#"[{"a":"hello"}]"#; + let expected: jsonrpc::Request = serde_json::from_str(s).unwrap(); + let body = hyper::Body::from(s.to_owned()); + let headers = hyper::header::HeaderMap::new(); + let bytes = read_http_body(&headers, body, HttpConfig::default()).await.unwrap(); + let req: jsonrpc::Request = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(req, expected); + } + + #[tokio::test] + async fn body_to_bytes_size_limit_works() { + let headers = hyper::header::HeaderMap::new(); + let body = hyper::Body::from(vec![0; 128]); + assert!(read_http_body(&headers, body, HttpConfig { max_request_body_size: 127 }).await.is_err()); + } + + #[test] + fn read_content_length_works() { + let mut headers = hyper::header::HeaderMap::new(); + headers.insert(hyper::header::CONTENT_LENGTH, "177".parse().unwrap()); + assert_eq!(read_http_content_length(&headers), Some(177)); + + headers.append(hyper::header::CONTENT_LENGTH, "999".parse().unwrap()); + assert_eq!(read_http_content_length(&headers), None); + } +} diff --git a/src/types/mod.rs b/src/types/mod.rs index cdbff64019..5c523583ce 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -6,3 +6,7 @@ pub mod client; /// Shared types for server implementation. pub mod server; + +/// Shared types for HTTP +#[cfg(feature = "http")] +pub mod http; From 8ef17a8cff9c3c7bdf5d6c4ee69f7c1a8b83fd92 Mon Sep 17 00:00:00 2001 From: Niklas Date: Tue, 17 Nov 2020 21:00:44 +0100 Subject: [PATCH 2/9] refactor(crate reorg): to have shared http helpers. * Merge client and server errors. * Move `http_server_utils` to `utils/http` * Minor cleanup --- examples/http.rs | 2 +- src/client/http/client.rs | 2 +- src/client/http/tests.rs | 2 +- src/client/http/transport.rs | 24 ++-- src/client/ws/client.rs | 13 +- src/http/mod.rs | 2 - src/http/server.rs | 20 +-- src/http/server_utils/utils.rs | 32 ----- src/http/tests.rs | 6 +- src/http/transport/background.rs | 19 +-- src/http/transport/mod.rs | 5 +- src/lib.rs | 2 + src/types/{client.rs => error.rs} | 26 ++-- src/types/http.rs | 86 ------------- src/types/mod.rs | 7 +- src/types/server.rs | 10 -- .../http}/access_control.rs | 12 +- src/{http/server_utils => utils/http}/cors.rs | 9 +- .../server_utils => utils/http}/hosts.rs | 2 +- src/utils/http/hyper_helpers.rs | 117 ++++++++++++++++++ .../server_utils => utils/http}/matcher.rs | 0 src/{http/server_utils => utils/http}/mod.rs | 2 +- src/utils/mod.rs | 2 + src/ws/server.rs | 24 ++-- src/ws/tests.rs | 10 +- 25 files changed, 222 insertions(+), 214 deletions(-) delete mode 100644 src/http/server_utils/utils.rs rename src/types/{client.rs => error.rs} (66%) delete mode 100644 src/types/server.rs rename src/{http/server_utils => utils/http}/access_control.rs (93%) rename src/{http/server_utils => utils/http}/cors.rs (99%) rename src/{http/server_utils => utils/http}/hosts.rs (99%) create mode 100644 src/utils/http/hyper_helpers.rs rename src/{http/server_utils => utils/http}/matcher.rs (100%) rename src/{http/server_utils => utils/http}/mod.rs (98%) create mode 100644 src/utils/mod.rs diff --git a/examples/http.rs b/examples/http.rs index ee4938e6bb..12a2559f94 100644 --- a/examples/http.rs +++ b/examples/http.rs @@ -52,7 +52,7 @@ async fn main() -> Result<(), Box> { } async fn run_server(server_started_tx: Sender<()>, url: &str) { - let server = HttpServer::new(url, HttpConfig::default()).await.unwrap(); + let server = HttpServer::new(url, HttpConfig { max_request_body_size: 30 }).await.unwrap(); let mut say_hello = server.register_method("say_hello".to_string()).unwrap(); server_started_tx.send(()).unwrap(); loop { diff --git a/src/client/http/client.rs b/src/client/http/client.rs index 584c4d94c7..3ea85ee8e8 100644 --- a/src/client/http/client.rs +++ b/src/client/http/client.rs @@ -1,5 +1,5 @@ use crate::client::http::transport::HttpTransportClient; -use crate::types::client::{Error, Mismatch}; +use crate::types::error::{Error, Mismatch}; use crate::types::http::HttpConfig; use crate::types::jsonrpc::{self, JsonValue}; use std::sync::atomic::{AtomicU64, Ordering}; diff --git a/src/client/http/tests.rs b/src/client/http/tests.rs index b9e02d657f..70cc394417 100644 --- a/src/client/http/tests.rs +++ b/src/client/http/tests.rs @@ -1,5 +1,5 @@ use crate::client::{HttpClient, HttpConfig}; -use crate::types::client::Error; +use crate::types::error::Error; use crate::types::jsonrpc::{self, ErrorCode, JsonValue, Params}; use jsonrpsee_test_utils::helpers::*; diff --git a/src/client/http/transport.rs b/src/client/http/transport.rs index 27388be3af..bc5f10ee8e 100644 --- a/src/client/http/transport.rs +++ b/src/client/http/transport.rs @@ -6,10 +6,8 @@ // that we need to be guaranteed that hyper doesn't re-use an existing connection if we ever reset // the JSON-RPC request id to a value that might have already been used. -use crate::types::{ - http::{read_http_body, HttpConfig}, - jsonrpc, -}; +use crate::types::{error::GenericTransportError, http::HttpConfig, jsonrpc}; +use crate::utils::http::hyper_helpers; use thiserror::Error; const CONTENT_TYPE_JSON: &str = "application/json"; @@ -28,7 +26,7 @@ pub struct HttpTransportClient { impl HttpTransportClient { /// Initializes a new HTTP client. pub fn new(target: impl AsRef, config: HttpConfig) -> Result { - let target = url::Url::parse(target.as_ref()).map_err(|e| Error::Url(format!("Invalid URL: {}", e).into()))?; + let target = url::Url::parse(target.as_ref()).map_err(|e| Error::Url(format!("Invalid URL: {}", e)))?; if target.scheme() == "http" { Ok(HttpTransportClient { client: hyper::Client::new(), target, config }) } else { @@ -73,7 +71,7 @@ impl HttpTransportClient { ) -> Result { let response = self.send_request(request).await?; let (parts, body) = response.into_parts(); - let body = read_http_body(&parts.headers, body, self.config).await.map_err(|e| Error::Http(Box::new(e)))?; + let body = hyper_helpers::read_response_to_body(&parts.headers, body, self.config).await?; // Note that we don't check the Content-Type of the request. This is deemed // unnecessary, as a parsing error while happen anyway. @@ -115,10 +113,22 @@ pub enum Error { ParseError(#[source] serde_json::error::Error), /// Request body too large. - #[error("The request body was to large")] + #[error("The request body was too large")] RequestTooLarge, } +impl From> for Error +where + T: std::error::Error + Send + Sync + 'static, +{ + fn from(err: GenericTransportError) -> Self { + match err { + GenericTransportError::::TooLarge => Self::RequestTooLarge, + GenericTransportError::::Inner(e) => Self::Http(Box::new(e)), + } + } +} + #[cfg(test)] mod tests { use super::{Error, HttpTransportClient}; diff --git a/src/client/ws/client.rs b/src/client/ws/client.rs index b152eaf16c..7e6df64330 100644 --- a/src/client/ws/client.rs +++ b/src/client/ws/client.rs @@ -25,7 +25,7 @@ // DEALINGS IN THE SOFTWARE. use crate::client::ws::{RawClient, RawClientEvent, RawClientRequestId, WsTransportClient}; -use crate::types::client::Error; +use crate::types::error::Error; use crate::types::jsonrpc::{self, JsonValue}; use futures::{ @@ -123,7 +123,7 @@ impl Client { let method = method.into(); let params = params.into(); log::trace!("[frontend]: send notification: method={:?}, params={:?}", method, params); - self.to_back.clone().send(FrontToBack::Notification { method, params }).await.map_err(Error::InternalChannel) + self.to_back.clone().send(FrontToBack::Notification { method, params }).await.map_err(Error::Internal) } /// Perform a request towards the server. @@ -139,7 +139,11 @@ impl Client { let params = params.into(); log::trace!("[frontend]: send request: method={:?}, params={:?}", method, params); let (send_back_tx, send_back_rx) = oneshot::channel(); - self.to_back.clone().send(FrontToBack::StartRequest { method, params, send_back: send_back_tx }).await?; + self.to_back + .clone() + .send(FrontToBack::StartRequest { method, params, send_back: send_back_tx }) + .await + .map_err(Error::Internal)?; // TODO: send a `ChannelClosed` message if we close the channel unexpectedly @@ -181,7 +185,8 @@ impl Client { params: params.into(), send_back: send_back_tx, }) - .await?; + .await + .map_err(Error::Internal)?; let notifs_rx = match send_back_rx.await { Ok(Ok(v)) => v, diff --git a/src/http/mod.rs b/src/http/mod.rs index df775bd8d3..5465a820ad 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -26,8 +26,6 @@ mod raw; mod server; -#[allow(unused)] -mod server_utils; mod transport; #[cfg(test)] diff --git a/src/http/server.rs b/src/http/server.rs index e2c4fb0d59..5b42b3e80e 100644 --- a/src/http/server.rs +++ b/src/http/server.rs @@ -26,9 +26,9 @@ use crate::http::raw::{RawServer, RawServerEvent, RawServerRequestId}; use crate::http::transport::HttpTransportServer; +use crate::types::error::Error; use crate::types::http::HttpConfig; use crate::types::jsonrpc::{self, JsonValue}; -use crate::types::server::Error; use futures::{channel::mpsc, future::Either, pin_mut, prelude::*}; use parking_lot::Mutex; @@ -50,7 +50,7 @@ pub struct Server { local_addr: SocketAddr, /// Channel to send requests to the background task. to_back: mpsc::UnboundedSender, - /// List of methods (for RPC queries, subscriptions, and unsubscriptions) that have been + /// List of methods (for RPC queries and and notifications) that have been /// registered. Serves no purpose except to check for duplicates. registered_methods: Arc>>, /// Next unique ID used when registering a subscription. @@ -112,8 +112,8 @@ enum FrontToBack { impl Server { /// Initializes a new server based upon this raw server. - pub async fn new(url: &str, config: HttpConfig) -> Result> { - let sockaddr = url.parse()?; + pub async fn new(url: impl AsRef, config: HttpConfig) -> Result> { + let sockaddr = url.as_ref().parse()?; let transport_server = HttpTransportServer::new(&sockaddr, config).await?; let local_addr = *transport_server.local_addr(); @@ -130,7 +130,7 @@ impl Server { Ok(Server { local_addr, to_back, - registered_methods: Arc::new(Mutex::new(Default::default())), + registered_methods: Arc::new(Mutex::new(HashSet::new())), next_subscription_unique_id: Arc::new(atomic::AtomicUsize::new(0)), }) } @@ -156,7 +156,7 @@ impl Server { allow_losses: bool, ) -> Result { if !self.registered_methods.lock().insert(method_name.clone()) { - return Err(Error::AlreadyRegistered(method_name)); + return Err(Error::MethodAlreadyRegistered(method_name)); } log::trace!("[frontend]: register_notification={}", method_name); @@ -164,7 +164,7 @@ impl Server { self.to_back .unbounded_send(FrontToBack::RegisterNotifications { name: method_name, handler: tx, allow_losses }) - .map_err(|e| Error::InternalChannel(e.into_send_error()))?; + .map_err(|e| Error::Internal(e.into_send_error()))?; Ok(RegisteredNotification { queries_rx: rx }) } @@ -181,7 +181,7 @@ impl Server { /// Returns an error if the method name was already registered. pub fn register_method(&self, method_name: String) -> Result { if !self.registered_methods.lock().insert(method_name.clone()) { - return Err(Error::AlreadyRegistered(method_name)); + return Err(Error::MethodAlreadyRegistered(method_name)); } log::trace!("[frontend]: register_method={}", method_name); @@ -189,7 +189,7 @@ impl Server { self.to_back .unbounded_send(FrontToBack::RegisterMethod { name: method_name, handler: tx }) - .map_err(|e| Error::InternalChannel(e.into_send_error()))?; + .map_err(|e| Error::Internal(e.into_send_error()))?; Ok(RegisteredMethod { to_back: self.to_back.clone(), queries_rx: rx }) } @@ -231,7 +231,7 @@ impl IncomingRequest { self.to_back .send(FrontToBack::AnswerRequest { request_id: self.request_id, answer: response.into() }) .await - .map_err(Error::InternalChannel) + .map_err(Error::Internal) } } diff --git a/src/http/server_utils/utils.rs b/src/http/server_utils/utils.rs deleted file mode 100644 index ee5dea513e..0000000000 --- a/src/http/server_utils/utils.rs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2019 Parity Technologies (UK) Ltd. -// -// Permission is hereby granted, free of charge, to any -// person obtaining a copy of this software and associated -// documentation files (the "Software"), to deal in the -// Software without restriction, including without -// limitation the rights to use, copy, modify, merge, -// publish, distribute, sublicense, and/or sell copies of -// the Software, and to permit persons to whom the Software -// is furnished to do so, subject to the following -// conditions: -// -// The above copyright notice and this permission notice -// shall be included in all copies or substantial portions -// of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF -// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED -// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A -// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT -// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY -// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR -// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -// DEALINGS IN THE SOFTWARE. - -//! Utility methods relying on hyper - -/// Extracts string value of a single header in request. -pub fn read_header<'a>(req: &'a hyper::Request, header_name: &str) -> Option<&'a str> { - req.headers().get(header_name).and_then(|v| v.to_str().ok()) -} diff --git a/src/http/tests.rs b/src/http/tests.rs index 95cb2bca46..91d2ba6966 100644 --- a/src/http/tests.rs +++ b/src/http/tests.rs @@ -40,9 +40,9 @@ async fn server(server_started_tx: Sender) { pin_mut!(hello_fut, add_fut, notif_fut); select! { - say_hello = hello_fut => (), - add = add_fut => (), - notif = notif_fut => (), + _say_hello = hello_fut => (), + _add = add_fut => (), + _notif = notif_fut => (), complete => (), }; } diff --git a/src/http/transport/background.rs b/src/http/transport/background.rs index 7531a5ed15..3e4221bab8 100644 --- a/src/http/transport/background.rs +++ b/src/http/transport/background.rs @@ -24,16 +24,13 @@ // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use crate::http::server_utils::access_control::AccessControl; use crate::http::transport::response; -use crate::types::{ - http::{self, HttpConfig}, - jsonrpc, -}; +use crate::types::{error::GenericTransportError, http::HttpConfig, jsonrpc}; +use crate::utils::http::{access_control::AccessControl, hyper_helpers}; use futures::{channel::mpsc, channel::oneshot, prelude::*}; use hyper::service::{make_service_fn, service_fn}; use hyper::Error; -use std::{error, io, net::SocketAddr, thread}; +use std::{error, net::SocketAddr, thread}; /// Background thread that serves HTTP requests. pub(super) struct BackgroundHttp { @@ -150,17 +147,13 @@ async fn process_request( // to prevent Cross-Origin XHRs with text/plain hyper::Method::POST if is_json(request.headers().get("content-type")) => { let (parts, body) = request.into_parts(); - let request = match http::read_http_body(&parts.headers, body, config).await { + let request = match hyper_helpers::read_response_to_body(&parts.headers, body, config).await { Ok(body) => match jsonrpc::from_slice(&body) { Ok(response) => response, Err(_e) => return response::parse_error(), }, - Err(e) => match (e.kind(), e.into_inner()) { - (io::ErrorKind::InvalidData, _) => return response::parse_error(), - (io::ErrorKind::UnexpectedEof, _) => return response::parse_error(), - (_, Some(inner)) => return response::internal_error(inner.to_string()), - (kind, None) => return response::internal_error(format!("{:?}", kind)), - }, + Err(GenericTransportError::TooLarge) => return response::too_large("The request was too large"), + Err(GenericTransportError::Inner(e)) => return response::internal_error(e.to_string()), }; let (tx, rx) = oneshot::channel(); diff --git a/src/http/transport/mod.rs b/src/http/transport/mod.rs index 8481b5e1e9..c37cb7851d 100644 --- a/src/http/transport/mod.rs +++ b/src/http/transport/mod.rs @@ -25,11 +25,12 @@ // DEALINGS IN THE SOFTWARE. mod background; +#[allow(unused)] mod response; -use crate::http::server_utils::access_control::AccessControl; use crate::types::http::HttpConfig; use crate::types::jsonrpc; +use crate::utils::http::access_control::AccessControl; use fnv::FnvHashMap; use futures::{channel::oneshot, prelude::*}; @@ -89,7 +90,7 @@ impl HttpTransportServer { config: HttpConfig, ) -> Result> { let (background_thread, local_addr) = background::BackgroundHttp::bind(addr, config).await?; - Ok(HttpTransportServer { background_thread, local_addr, requests: Default::default(), next_request_id: 0 }) + Ok(HttpTransportServer { background_thread, local_addr, requests: FnvHashMap::default(), next_request_id: 0 }) } /// Tries to start an HTTP server that listens on the given address with an access control list. diff --git a/src/lib.rs b/src/lib.rs index 8488afc45a..75062e0f07 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,8 @@ pub mod client; pub mod http; /// Shared types. pub mod types; +/// Utils. +pub mod utils; /// JSONRPC 2.0 WebSocket server. #[cfg(feature = "ws")] pub mod ws; diff --git a/src/types/client.rs b/src/types/error.rs similarity index 66% rename from src/types/client.rs rename to src/types/error.rs index 9dc1ee35ba..4f3647b999 100644 --- a/src/types/client.rs +++ b/src/types/error.rs @@ -1,6 +1,5 @@ use crate::types::jsonrpc::{self, JsonValue}; use std::fmt; - /// Convenience type for displaying errors. #[derive(Clone, Debug, PartialEq)] pub struct Mismatch { @@ -16,29 +15,42 @@ impl fmt::Display for Mismatch { } } -/// Error produced by the client. +/// Error type. #[derive(Debug, thiserror::Error)] pub enum Error { - /// Networking error or error on the low-level protocol layer (e.g. missing field, - /// invalid ID, etc.). + /// Networking error or error on the low-level protocol layer. #[error("Networking or low-level protocol error: {0}")] TransportError(#[source] Box), - /// Request error. - #[error("Server responded to our request with an error: {0:?}")] + /// JSON-RPC request error. + #[error("JSON-RPC request error: {0:?}")] Request(#[source] jsonrpc::Error), /// Subscription error. #[error("Subscription to subscribe_method: {0} with unsubscribe_method: {1} failed")] Subscription(String, String), /// Frontend/backend channel error. #[error("Frontend/backend channel error: {0}")] - InternalChannel(#[from] futures::channel::mpsc::SendError), + Internal(#[source] futures::channel::mpsc::SendError), /// Failed to parse the data that the server sent back to us. #[error("Parse error: {0}")] ParseError(#[source] jsonrpc::ParseError), /// Invalid id in response to a request. #[error("Invalid ID in response: {0}")] InvalidRequestId(Mismatch), + /// Method was already registered. + #[error("Method: {0} already registered")] + MethodAlreadyRegistered(String), #[error("Custom error: {0}")] /// Custom error. Custom(String), } + +/// Generic transport error. +#[derive(Debug, thiserror::Error)] +pub enum GenericTransportError { + /// Request was too large. + #[error("The request was too big")] + TooLarge, + /// Concrete transport error. + #[error("Transport error: {0}")] + Inner(T), +} diff --git a/src/types/http.rs b/src/types/http.rs index 31c0a78eee..a6558559b4 100644 --- a/src/types/http.rs +++ b/src/types/http.rs @@ -1,8 +1,5 @@ //! Shared HTTP types -use futures::StreamExt; -use std::io::{Error, ErrorKind}; - /// Default maximum request body size (10 MB). const DEFAULT_MAX_BODY_SIZE_TEN_MB: u32 = 10 * 1024 * 1024; @@ -18,86 +15,3 @@ impl Default for HttpConfig { Self { max_request_body_size: DEFAULT_MAX_BODY_SIZE_TEN_MB } } } - -/// Read response body from a received request with configured `HTTP` settings. -// TODO: move somewhere else!!! -pub async fn read_http_body( - header: &hyper::HeaderMap, - mut body: hyper::Body, - config: HttpConfig, -) -> Result, Error> { - let body_size = read_http_content_length(&header).unwrap_or(0); - - if body_size > config.max_request_body_size { - return Err(Error::new( - ErrorKind::Other, - format!("HTTP request body too large, got: {} max: {}", body_size, config.max_request_body_size), - )); - } - - let mut received_data = Vec::with_capacity(body_size as usize); - - while let Some(chunk) = body.next().await { - let chunk = chunk.map_err(|e| Error::new(ErrorKind::Other, e.to_string()))?; - let body_length = chunk.len() + received_data.len(); - if body_length > config.max_request_body_size as usize { - return Err(Error::new( - ErrorKind::Other, - format!("HTTP request body too large, got: {} max: {}", body_length, config.max_request_body_size), - )); - } - received_data.extend_from_slice(&chunk); - } - Ok(received_data) -} - -// Read `content_length` from HTTP Header. -// -// Returns `Some(val)` if `content_length` contains exactly one value. -// None otherwise. -fn read_http_content_length(headers: &hyper::header::HeaderMap) -> Option { - let values = headers.get_all("content-length"); - let mut iter = values.iter(); - let content_length = iter.next()?; - if iter.next().is_some() { - return None; - } - - // HTTP Content-Length indicates number of bytes in decimal. - let length = content_length.to_str().ok()?; - u32::from_str_radix(length, 10).ok() -} - -#[cfg(test)] -mod tests { - use super::{read_http_body, read_http_content_length, HttpConfig}; - use crate::types::jsonrpc; - - #[tokio::test] - async fn body_to_request_works() { - let s = r#"[{"a":"hello"}]"#; - let expected: jsonrpc::Request = serde_json::from_str(s).unwrap(); - let body = hyper::Body::from(s.to_owned()); - let headers = hyper::header::HeaderMap::new(); - let bytes = read_http_body(&headers, body, HttpConfig::default()).await.unwrap(); - let req: jsonrpc::Request = serde_json::from_slice(&bytes).unwrap(); - assert_eq!(req, expected); - } - - #[tokio::test] - async fn body_to_bytes_size_limit_works() { - let headers = hyper::header::HeaderMap::new(); - let body = hyper::Body::from(vec![0; 128]); - assert!(read_http_body(&headers, body, HttpConfig { max_request_body_size: 127 }).await.is_err()); - } - - #[test] - fn read_content_length_works() { - let mut headers = hyper::header::HeaderMap::new(); - headers.insert(hyper::header::CONTENT_LENGTH, "177".parse().unwrap()); - assert_eq!(read_http_content_length(&headers), Some(177)); - - headers.append(hyper::header::CONTENT_LENGTH, "999".parse().unwrap()); - assert_eq!(read_http_content_length(&headers), None); - } -} diff --git a/src/types/mod.rs b/src/types/mod.rs index 5c523583ce..5f41eaf82b 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1,11 +1,8 @@ /// JSON-RPC 2.0 specification related types. pub mod jsonrpc; -/// Shared types for client implementation. -pub mod client; - -/// Shared types for server implementation. -pub mod server; +/// Shared error type. +pub mod error; /// Shared types for HTTP #[cfg(feature = "http")] diff --git a/src/types/server.rs b/src/types/server.rs deleted file mode 100644 index 8ea9002054..0000000000 --- a/src/types/server.rs +++ /dev/null @@ -1,10 +0,0 @@ -/// Server Error. -#[derive(Debug, thiserror::Error)] -pub enum Error { - /// The method, notification or subscription is already registered. - #[error("{0} is already registered")] - AlreadyRegistered(String), - /// Frontend/backend channel error. - #[error("Frontend/backend channel error: {0}")] - InternalChannel(#[from] futures::channel::mpsc::SendError), -} diff --git a/src/http/server_utils/access_control.rs b/src/utils/http/access_control.rs similarity index 93% rename from src/http/server_utils/access_control.rs rename to src/utils/http/access_control.rs index 780f93c163..6eac03edcd 100644 --- a/src/http/server_utils/access_control.rs +++ b/src/utils/http/access_control.rs @@ -26,9 +26,9 @@ //! Access control based on http headers -use crate::http::server_utils::cors::{AccessControlAllowHeaders, AccessControlAllowOrigin}; -use crate::http::server_utils::hosts::{AllowHosts, Host}; -use crate::http::server_utils::{cors, hosts, utils}; +use crate::utils::http::cors::{AccessControlAllowHeaders, AccessControlAllowOrigin}; +use crate::utils::http::hosts::{AllowHosts, Host}; +use crate::utils::http::{cors, hosts, hyper_helpers}; use hyper::{self, header}; /// Define access on control on http layer @@ -44,14 +44,14 @@ pub struct AccessControl { impl AccessControl { /// Validate incoming request by http HOST pub fn deny_host(&self, request: &hyper::Request) -> bool { - !hosts::is_host_valid(utils::read_header(request, "host"), &self.allow_hosts) + !hosts::is_host_valid(hyper_helpers::read_header(request, "host"), &self.allow_hosts) } /// Validate incoming request by CORS origin pub fn deny_cors_origin(&self, request: &hyper::Request) -> bool { let header = cors::get_cors_allow_origin( - utils::read_header(request, "origin"), - utils::read_header(request, "host"), + hyper_helpers::read_header(request, "origin"), + hyper_helpers::read_header(request, "host"), &self.cors_allow_origin, ) .map(|origin| { diff --git a/src/http/server_utils/cors.rs b/src/utils/http/cors.rs similarity index 99% rename from src/http/server_utils/cors.rs rename to src/utils/http/cors.rs index 28255a6fb5..92df7ca7de 100644 --- a/src/http/server_utils/cors.rs +++ b/src/utils/http/cors.rs @@ -26,8 +26,8 @@ //! CORS handling utility functions -use crate::http::server_utils::hosts::{Host, Port}; -use crate::http::server_utils::matcher::{Matcher, Pattern}; +use crate::utils::http::hosts::{Host, Port}; +use crate::utils::http::matcher::{Matcher, Pattern}; use lazy_static::lazy_static; use std::collections::HashSet; use std::{fmt, ops}; @@ -322,10 +322,9 @@ lazy_static! { #[cfg(test)] mod tests { - use std::iter; - use super::*; - use crate::http::server_utils::hosts::Host; + use crate::utils::http::hosts::Host; + use std::iter; #[test] fn should_parse_origin() { diff --git a/src/http/server_utils/hosts.rs b/src/utils/http/hosts.rs similarity index 99% rename from src/http/server_utils/hosts.rs rename to src/utils/http/hosts.rs index c91a9fd881..1a3cf4b1f0 100644 --- a/src/http/server_utils/hosts.rs +++ b/src/utils/http/hosts.rs @@ -26,7 +26,7 @@ //! Host header validation. -use crate::http::server_utils::matcher::{Matcher, Pattern}; +use crate::utils::http::matcher::{Matcher, Pattern}; use std::collections::HashSet; use std::net::SocketAddr; diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs new file mode 100644 index 0000000000..51e58407f4 --- /dev/null +++ b/src/utils/http/hyper_helpers.rs @@ -0,0 +1,117 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +//! Utility methods relying on hyper + +use crate::types::error::GenericTransportError; +use crate::types::http::HttpConfig; +use futures::StreamExt; + +/// Read a hyper response with configured `HTTP` settings. +/// +/// Returns `Ok(bytes)` if the body was in valid size range. +/// Returns `Err` if the body was too large or the body couldn't be read. +// +// TODO: split into two functions. +pub async fn read_response_to_body( + headers: &hyper::HeaderMap, + mut body: hyper::Body, + config: HttpConfig, +) -> Result, GenericTransportError> { + let body_size = read_http_content_length(&headers).unwrap_or(0); + + if body_size > config.max_request_body_size { + return Err(GenericTransportError::TooLarge); + } + + let mut received_data = Vec::with_capacity(body_size as usize); + + while let Some(chunk) = body.next().await { + let chunk = chunk.map_err(|e| GenericTransportError::Inner(e))?; + let body_length = chunk.len() + received_data.len(); + if body_length > config.max_request_body_size as usize { + return Err(GenericTransportError::TooLarge); + } + received_data.extend_from_slice(&chunk); + } + Ok(received_data) +} + +/// Read `content_length` from HTTP Header. +/// +/// Returns `Some(val)` if `content_length` contains exactly one value. +/// None otherwise. +fn read_http_content_length(headers: &hyper::header::HeaderMap) -> Option { + let values = headers.get_all("content-length"); + let mut iter = values.iter(); + let content_length = iter.next()?; + if iter.next().is_some() { + return None; + } + + // HTTP Content-Length indicates number of bytes in decimal. + let length = content_length.to_str().ok()?; + u32::from_str_radix(length, 10).ok() +} + +/// Extracts string value of a single header in request. +pub fn read_header<'a>(req: &'a hyper::Request, header_name: &str) -> Option<&'a str> { + req.headers().get(header_name).and_then(|v| v.to_str().ok()) +} + +#[cfg(test)] +mod tests { + use super::{read_http_content_length, read_response_to_body, HttpConfig}; + use crate::types::jsonrpc; + + #[tokio::test] + async fn body_to_request_works() { + let s = r#"[{"a":"hello"}]"#; + let expected: jsonrpc::Request = serde_json::from_str(s).unwrap(); + let body = hyper::Body::from(s.to_owned()); + let headers = hyper::header::HeaderMap::new(); + let bytes = read_response_to_body(&headers, body, HttpConfig::default()).await.unwrap(); + let req: jsonrpc::Request = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(req, expected); + } + + #[tokio::test] + async fn body_to_bytes_size_limit_works() { + let headers = hyper::header::HeaderMap::new(); + let body = hyper::Body::from(vec![0; 128]); + assert!(read_response_to_body(&headers, body, HttpConfig { max_request_body_size: 127 }).await.is_err()); + } + + #[test] + fn read_content_length_works() { + let mut headers = hyper::header::HeaderMap::new(); + headers.insert(hyper::header::CONTENT_LENGTH, "177".parse().unwrap()); + assert_eq!(read_http_content_length(&headers), Some(177)); + + headers.append(hyper::header::CONTENT_LENGTH, "999".parse().unwrap()); + assert_eq!(read_http_content_length(&headers), None); + } +} diff --git a/src/http/server_utils/matcher.rs b/src/utils/http/matcher.rs similarity index 100% rename from src/http/server_utils/matcher.rs rename to src/utils/http/matcher.rs diff --git a/src/http/server_utils/mod.rs b/src/utils/http/mod.rs similarity index 98% rename from src/http/server_utils/mod.rs rename to src/utils/http/mod.rs index 1648bc08e4..62c09485e0 100644 --- a/src/http/server_utils/mod.rs +++ b/src/utils/http/mod.rs @@ -29,7 +29,7 @@ pub mod access_control; pub mod cors; pub mod hosts; -pub mod utils; +pub mod hyper_helpers; mod matcher; diff --git a/src/utils/mod.rs b/src/utils/mod.rs new file mode 100644 index 0000000000..d56a2d3919 --- /dev/null +++ b/src/utils/mod.rs @@ -0,0 +1,2 @@ +#[cfg(feature = "http")] +pub mod http; diff --git a/src/ws/server.rs b/src/ws/server.rs index e7521041ee..020b48316f 100644 --- a/src/ws/server.rs +++ b/src/ws/server.rs @@ -24,8 +24,8 @@ // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +use crate::types::error::Error; use crate::types::jsonrpc::{self, JsonValue}; -use crate::types::server::Error; use crate::ws::raw::{RawServer, RawServerEvent, RawServerRequestId, RawServerSubscriptionId}; use crate::ws::transport::WsTransportServer; @@ -141,8 +141,8 @@ enum FrontToBack { impl Server { /// Initializes a new server. - pub async fn new(url: &str) -> Result> { - let sockaddr = url.parse()?; + pub async fn new(url: impl AsRef) -> Result> { + let sockaddr: SocketAddr = url.as_ref().parse()?; let transport_server = WsTransportServer::builder(sockaddr).build().await?; let local_addr = *transport_server.local_addr(); @@ -185,7 +185,7 @@ impl Server { allow_losses: bool, ) -> Result { if !self.registered_methods.lock().insert(method_name.clone()) { - return Err(Error::AlreadyRegistered(method_name)); + return Err(Error::MethodAlreadyRegistered(method_name)); } log::trace!("[frontend]: register_notification={}", method_name); @@ -193,7 +193,7 @@ impl Server { self.to_back .unbounded_send(FrontToBack::RegisterNotifications { name: method_name, handler: tx, allow_losses }) - .map_err(|e| Error::InternalChannel(e.into_send_error()))?; + .map_err(|e| Error::Internal(e.into_send_error()))?; Ok(RegisteredNotification { queries_rx: rx }) } @@ -210,7 +210,7 @@ impl Server { /// Returns an error if the method name was already registered. pub fn register_method(&self, method_name: String) -> Result { if !self.registered_methods.lock().insert(method_name.clone()) { - return Err(Error::AlreadyRegistered(method_name)); + return Err(Error::MethodAlreadyRegistered(method_name)); } log::trace!("[frontend]: register_method={}", method_name); @@ -218,7 +218,7 @@ impl Server { self.to_back .unbounded_send(FrontToBack::RegisterMethod { name: method_name, handler: tx }) - .map_err(|e| Error::InternalChannel(e.into_send_error()))?; + .map_err(|e| Error::Internal(e.into_send_error()))?; Ok(RegisteredMethod { to_back: self.to_back.clone(), queries_rx: rx }) } @@ -241,11 +241,11 @@ impl Server { // This means that if the strings are equal this will be slower than just comparing the // strings. if !registered_methods.insert(subscribe_method_name.clone()) { - return Err(Error::AlreadyRegistered(subscribe_method_name)); + return Err(Error::MethodAlreadyRegistered(subscribe_method_name)); } if !registered_methods.insert(unsubscribe_method_name.clone()) { registered_methods.remove(&subscribe_method_name); - return Err(Error::AlreadyRegistered(unsubscribe_method_name)); + return Err(Error::MethodAlreadyRegistered(unsubscribe_method_name)); } } @@ -262,7 +262,7 @@ impl Server { subscribe_method: subscribe_method_name, unsubscribe_method: unsubscribe_method_name, }) - .map_err(|e| Error::InternalChannel(e.into_send_error()))?; + .map_err(|e| Error::Internal(e.into_send_error()))?; Ok(RegisteredSubscription { to_back: self.to_back.clone(), unique_id }) } @@ -299,7 +299,7 @@ impl RegisteredSubscription { self.to_back .send(FrontToBack::SendOutNotif { unique_id: self.unique_id, notification: value }) .await - .map_err(Error::InternalChannel) + .map_err(Error::Internal) } } @@ -314,7 +314,7 @@ impl IncomingRequest { self.to_back .send(FrontToBack::AnswerRequest { request_id: self.request_id, answer: response.into() }) .await - .map_err(Error::InternalChannel) + .map_err(Error::Internal) } } diff --git a/src/ws/tests.rs b/src/ws/tests.rs index fd4db4c9fc..01b9b85590 100644 --- a/src/ws/tests.rs +++ b/src/ws/tests.rs @@ -1,8 +1,8 @@ #![cfg(test)] use crate::client::{WsClient, WsSubscription}; +use crate::types::error::Error; use crate::types::jsonrpc::{JsonValue, Params}; -use crate::types::server::Error; use crate::ws::WsServer; use std::net::SocketAddr; @@ -68,9 +68,9 @@ pub async fn server(server_started: Sender) { pin_mut!(hello_fut, add_fut, notif_fut); select! { - say_hello = hello_fut => (), - add = add_fut => (), - notif = notif_fut => (), + _say_hello = hello_fut => (), + _add = add_fut => (), + _notif = notif_fut => (), complete => (), }; } @@ -236,7 +236,7 @@ async fn register_same_subscribe_unsubscribe_is_err() { let server = WsServer::new("127.0.0.1:0").await.unwrap(); assert!(matches!( server.register_subscription("subscribe_hello".to_owned(), "subscribe_hello".to_owned()), - Err(Error::AlreadyRegistered(_)) + Err(Error::MethodAlreadyRegistered(_)) )); } From e824c8d3fa9c572e45df718f5af064579733de87 Mon Sep 17 00:00:00 2001 From: Niklas Date: Wed, 18 Nov 2020 14:11:57 +0100 Subject: [PATCH 3/9] fix nits --- examples/http.rs | 2 +- src/utils/http/access_control.rs | 6 ++-- src/utils/http/hyper_helpers.rs | 52 ++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/examples/http.rs b/examples/http.rs index 12a2559f94..ee4938e6bb 100644 --- a/examples/http.rs +++ b/examples/http.rs @@ -52,7 +52,7 @@ async fn main() -> Result<(), Box> { } async fn run_server(server_started_tx: Sender<()>, url: &str) { - let server = HttpServer::new(url, HttpConfig { max_request_body_size: 30 }).await.unwrap(); + let server = HttpServer::new(url, HttpConfig::default()).await.unwrap(); let mut say_hello = server.register_method("say_hello".to_string()).unwrap(); server_started_tx.send(()).unwrap(); loop { diff --git a/src/utils/http/access_control.rs b/src/utils/http/access_control.rs index 6eac03edcd..fdfac7e53c 100644 --- a/src/utils/http/access_control.rs +++ b/src/utils/http/access_control.rs @@ -44,14 +44,14 @@ pub struct AccessControl { impl AccessControl { /// Validate incoming request by http HOST pub fn deny_host(&self, request: &hyper::Request) -> bool { - !hosts::is_host_valid(hyper_helpers::read_header(request, "host"), &self.allow_hosts) + !hosts::is_host_valid(hyper_helpers::read_header(request.headers(), "host"), &self.allow_hosts) } /// Validate incoming request by CORS origin pub fn deny_cors_origin(&self, request: &hyper::Request) -> bool { let header = cors::get_cors_allow_origin( - hyper_helpers::read_header(request, "origin"), - hyper_helpers::read_header(request, "host"), + hyper_helpers::read_header(request.headers(), "origin"), + hyper_helpers::read_header(request.headers(), "host"), &self.cors_allow_origin, ) .map(|origin| { diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index 51e58407f4..3890c83e50 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -34,16 +34,16 @@ use futures::StreamExt; /// /// Returns `Ok(bytes)` if the body was in valid size range. /// Returns `Err` if the body was too large or the body couldn't be read. -// -// TODO: split into two functions. pub async fn read_response_to_body( headers: &hyper::HeaderMap, mut body: hyper::Body, config: HttpConfig, ) -> Result, GenericTransportError> { - let body_size = read_http_content_length(&headers).unwrap_or(0); + // NOTE(niklasad1): Bigger values than `u64::MAX` will be regarded as zero here and that's very unlikely to occur. + // Thus, in those causes we rely on the while loop to allocate instead of `pre-allocate`. + let body_size = read_header_content_length(&headers).unwrap_or(0); - if body_size > config.max_request_body_size { + if body_size > config.max_request_body_size as u64 { return Err(GenericTransportError::TooLarge); } @@ -60,31 +60,32 @@ pub async fn read_response_to_body( Ok(received_data) } -/// Read `content_length` from HTTP Header. +/// Read `content_length` from HTTP Header that fits into `u64` /// -/// Returns `Some(val)` if `content_length` contains exactly one value. -/// None otherwise. -fn read_http_content_length(headers: &hyper::header::HeaderMap) -> Option { - let values = headers.get_all("content-length"); - let mut iter = values.iter(); - let content_length = iter.next()?; - if iter.next().is_some() { - return None; - } - +/// NOTE: There's no specific hard limit on `content_length` in specification, so bigger values than `u64::MAX` won't be parsed successfully. +fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option { + let length = read_header(headers, "content-length")?; // HTTP Content-Length indicates number of bytes in decimal. - let length = content_length.to_str().ok()?; - u32::from_str_radix(length, 10).ok() + u64::from_str_radix(length, 10).ok() } /// Extracts string value of a single header in request. -pub fn read_header<'a>(req: &'a hyper::Request, header_name: &str) -> Option<&'a str> { - req.headers().get(header_name).and_then(|v| v.to_str().ok()) +/// +/// Returns `Some(val)` if the header contains exactly one value. +/// None otherwise. +pub fn read_header<'a>(headers: &'a hyper::header::HeaderMap, header_name: &str) -> Option<&'a str> { + let mut iter = headers.get_all(header_name).iter(); + let val = iter.next()?; + if iter.next().is_none() { + val.to_str().ok() + } else { + None + } } #[cfg(test)] mod tests { - use super::{read_http_content_length, read_response_to_body, HttpConfig}; + use super::{read_header_content_length, read_response_to_body, HttpConfig}; use crate::types::jsonrpc; #[tokio::test] @@ -109,9 +110,16 @@ mod tests { fn read_content_length_works() { let mut headers = hyper::header::HeaderMap::new(); headers.insert(hyper::header::CONTENT_LENGTH, "177".parse().unwrap()); - assert_eq!(read_http_content_length(&headers), Some(177)); + assert_eq!(read_header_content_length(&headers), Some(177)); headers.append(hyper::header::CONTENT_LENGTH, "999".parse().unwrap()); - assert_eq!(read_http_content_length(&headers), None); + assert_eq!(read_header_content_length(&headers), None); + } + + #[test] + fn read_content_length_too_big_value() { + let mut headers = hyper::header::HeaderMap::new(); + headers.insert(hyper::header::CONTENT_LENGTH, "18446744073709551616".parse().unwrap()); + assert_eq!(read_header_content_length(&headers), None); } } From 574f7047d513cb074ba3b864a8890d1f577854d9 Mon Sep 17 00:00:00 2001 From: Niklas Date: Wed, 18 Nov 2020 14:32:38 +0100 Subject: [PATCH 4/9] fix(hyper helper): u64 -> u32 --- src/utils/http/hyper_helpers.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index 3890c83e50..10c513859f 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -39,11 +39,11 @@ pub async fn read_response_to_body( mut body: hyper::Body, config: HttpConfig, ) -> Result, GenericTransportError> { - // NOTE(niklasad1): Bigger values than `u64::MAX` will be regarded as zero here and that's very unlikely to occur. + // NOTE(niklasad1): Bigger values than `u32::MAX` will be regarded as zero here and that's very unlikely to occur. // Thus, in those causes we rely on the while loop to allocate instead of `pre-allocate`. let body_size = read_header_content_length(&headers).unwrap_or(0); - if body_size > config.max_request_body_size as u64 { + if body_size > config.max_request_body_size { return Err(GenericTransportError::TooLarge); } @@ -60,13 +60,13 @@ pub async fn read_response_to_body( Ok(received_data) } -/// Read `content_length` from HTTP Header that fits into `u64` +/// Read `content_length` from HTTP Header that fits into `u32` /// -/// NOTE: There's no specific hard limit on `content_length` in specification, so bigger values than `u64::MAX` won't be parsed successfully. -fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option { +/// NOTE: There's no specific hard limit on `content_length` in HTTP specification, thus this method might reject valid `content_length` +fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option { let length = read_header(headers, "content-length")?; // HTTP Content-Length indicates number of bytes in decimal. - u64::from_str_radix(length, 10).ok() + u32::from_str_radix(length, 10).ok() } /// Extracts string value of a single header in request. From fbf28d2879a70cce37adf1a255712247a938f987 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 18 Nov 2020 16:20:02 +0100 Subject: [PATCH 5/9] Update src/utils/http/hyper_helpers.rs Co-authored-by: David --- src/utils/http/hyper_helpers.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index 10c513859f..323cdeb573 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -74,9 +74,9 @@ fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option /// Returns `Some(val)` if the header contains exactly one value. /// None otherwise. pub fn read_header<'a>(headers: &'a hyper::header::HeaderMap, header_name: &str) -> Option<&'a str> { - let mut iter = headers.get_all(header_name).iter(); - let val = iter.next()?; - if iter.next().is_none() { + let mut values = headers.get_all(header_name).iter(); + let val = values.next()?; + if values.next().is_none() { val.to_str().ok() } else { None From 8e1dda7190faa823db2a602a5ae5d8bfc30486dd Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 18 Nov 2020 16:20:16 +0100 Subject: [PATCH 6/9] Update src/utils/http/hyper_helpers.rs Co-authored-by: David --- src/utils/http/hyper_helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index 323cdeb573..ea182a2b4d 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -73,7 +73,7 @@ fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option /// /// Returns `Some(val)` if the header contains exactly one value. /// None otherwise. -pub fn read_header<'a>(headers: &'a hyper::header::HeaderMap, header_name: &str) -> Option<&'a str> { +pub fn read_header_value<'a>(headers: &'a hyper::header::HeaderMap, header_name: &str) -> Option<&'a str> { let mut values = headers.get_all(header_name).iter(); let val = values.next()?; if values.next().is_none() { From c77bcaebea68ad62f8c531c8bbdeffb6ebdc4a34 Mon Sep 17 00:00:00 2001 From: Niklas Date: Wed, 18 Nov 2020 17:22:22 +0100 Subject: [PATCH 7/9] fix: grumbles --- src/utils/http/access_control.rs | 11 ++++------- src/utils/http/hyper_helpers.rs | 20 +++++++++++++------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/utils/http/access_control.rs b/src/utils/http/access_control.rs index fdfac7e53c..f4eefef144 100644 --- a/src/utils/http/access_control.rs +++ b/src/utils/http/access_control.rs @@ -44,14 +44,14 @@ pub struct AccessControl { impl AccessControl { /// Validate incoming request by http HOST pub fn deny_host(&self, request: &hyper::Request) -> bool { - !hosts::is_host_valid(hyper_helpers::read_header(request.headers(), "host"), &self.allow_hosts) + !hosts::is_host_valid(hyper_helpers::read_header_value(request.headers(), "host"), &self.allow_hosts) } /// Validate incoming request by CORS origin pub fn deny_cors_origin(&self, request: &hyper::Request) -> bool { let header = cors::get_cors_allow_origin( - hyper_helpers::read_header(request.headers(), "origin"), - hyper_helpers::read_header(request.headers(), "host"), + hyper_helpers::read_header_value(request.headers(), "origin"), + hyper_helpers::read_header_value(request.headers(), "host"), &self.cors_allow_origin, ) .map(|origin| { @@ -70,10 +70,7 @@ impl AccessControl { /// Validate incoming request by CORS header pub fn deny_cors_header(&self, request: &hyper::Request) -> bool { let headers = request.headers().keys().map(|name| name.as_str()); - let requested_headers = request - .headers() - .get_all("access-control-request-headers") - .iter() + let requested_headers = hyper_helpers::read_header_values(request.headers(), "access-control-request-headers") .filter_map(|val| val.to_str().ok()) .flat_map(|val| val.split(", ")) .flat_map(|val| val.split(',')); diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index ea182a2b4d..357b1fe013 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -60,19 +60,17 @@ pub async fn read_response_to_body( Ok(received_data) } -/// Read `content_length` from HTTP Header that fits into `u32` +/// Read the `Content-Length` HTTP Header. Must fit into a `u32`; returns `None` otherwise. /// -/// NOTE: There's no specific hard limit on `content_length` in HTTP specification, thus this method might reject valid `content_length` +/// NOTE: There's no specific hard limit on `Content_length` in HTTP specification. +/// Thus this method might reject valid `content_length` fn read_header_content_length(headers: &hyper::header::HeaderMap) -> Option { - let length = read_header(headers, "content-length")?; + let length = read_header_value(headers, "content-length")?; // HTTP Content-Length indicates number of bytes in decimal. u32::from_str_radix(length, 10).ok() } -/// Extracts string value of a single header in request. -/// -/// Returns `Some(val)` if the header contains exactly one value. -/// None otherwise. +/// Returns a string value when there is exactly one value for the given header. pub fn read_header_value<'a>(headers: &'a hyper::header::HeaderMap, header_name: &str) -> Option<&'a str> { let mut values = headers.get_all(header_name).iter(); let val = values.next()?; @@ -83,6 +81,14 @@ pub fn read_header_value<'a>(headers: &'a hyper::header::HeaderMap, header_name: } } +/// Returns an iterator of all values for a given a header name +pub fn read_header_values<'a>( + headers: &'a hyper::header::HeaderMap, + header_name: &str, +) -> hyper::header::ValueIter<'a, hyper::header::HeaderValue> { + headers.get_all(header_name).iter() +} + #[cfg(test)] mod tests { use super::{read_header_content_length, read_response_to_body, HttpConfig}; From e73118e804b6f591afe9338e8d054aaa704f45f7 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 20 Nov 2020 14:32:29 +0100 Subject: [PATCH 8/9] Update src/utils/http/hyper_helpers.rs Co-authored-by: David --- src/utils/http/hyper_helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/http/hyper_helpers.rs b/src/utils/http/hyper_helpers.rs index 357b1fe013..cb4b2cf066 100644 --- a/src/utils/http/hyper_helpers.rs +++ b/src/utils/http/hyper_helpers.rs @@ -39,8 +39,8 @@ pub async fn read_response_to_body( mut body: hyper::Body, config: HttpConfig, ) -> Result, GenericTransportError> { - // NOTE(niklasad1): Bigger values than `u32::MAX` will be regarded as zero here and that's very unlikely to occur. - // Thus, in those causes we rely on the while loop to allocate instead of `pre-allocate`. + // NOTE(niklasad1): Values bigger than `u32::MAX` will be turned into zero here. This is unlikely to occur in practise + // and for that case we fallback to allocating in the while-loop below instead of pre-allocating. let body_size = read_header_content_length(&headers).unwrap_or(0); if body_size > config.max_request_body_size { From 6903d2a1bf48b58e261cb711d48ebc00bf9e10b1 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 20 Nov 2020 14:32:37 +0100 Subject: [PATCH 9/9] Update src/http/server.rs Co-authored-by: David --- src/http/server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http/server.rs b/src/http/server.rs index 5b42b3e80e..6e88257483 100644 --- a/src/http/server.rs +++ b/src/http/server.rs @@ -50,7 +50,7 @@ pub struct Server { local_addr: SocketAddr, /// Channel to send requests to the background task. to_back: mpsc::UnboundedSender, - /// List of methods (for RPC queries and and notifications) that have been + /// List of methods (for RPC queries and notifications) that have been /// registered. Serves no purpose except to check for duplicates. registered_methods: Arc>>, /// Next unique ID used when registering a subscription.