From 8a8d7bb5b104adc1c04275b90c33ac0e8fafba77 Mon Sep 17 00:00:00 2001 From: cihodar Date: Mon, 10 Aug 2020 14:17:05 +0300 Subject: [PATCH] Made InvalidHeader error more verbose. Changed the way we handle TransferEncoding and added new test cases. Signed-off-by: cihodar Signed-off-by: YUAN LYU --- src/common/headers.rs | 99 ++++++++++++++++++++++++++++------- src/common/mod.rs | 118 +++++++++++++++++++++++++++++++++++++----- src/connection.rs | 31 ++++++++--- src/lib.rs | 2 +- src/request.rs | 6 +-- src/server.rs | 6 +-- 6 files changed, 217 insertions(+), 45 deletions(-) diff --git a/src/common/headers.rs b/src/common/headers.rs index c6861c3..c3546f0 100644 --- a/src/common/headers.rs +++ b/src/common/headers.rs @@ -3,6 +3,7 @@ use std::result::Result; +use crate::HttpHeaderError; use crate::RequestError; /// Wrapper over an HTTP Header type. @@ -51,7 +52,9 @@ impl Header { "transfer-encoding" => Ok(Self::TransferEncoding), "server" => Ok(Self::Server), "accept" => Ok(Self::Accept), - _ => Err(RequestError::InvalidHeader), + invalid_key => Err(RequestError::HeaderError(HttpHeaderError::UnsupportedName( + invalid_key.to_string(), + ))), } } else { Err(RequestError::InvalidRequest) @@ -129,7 +132,9 @@ impl Headers { Ok(headers_str) => { let entry = headers_str.splitn(2, ':').collect::>(); if entry.len() != 2 { - return Err(RequestError::InvalidHeader); + return Err(RequestError::HeaderError(HttpHeaderError::InvalidFormat( + entry[0].to_string(), + ))); } if let Ok(head) = Header::try_from(entry[0].as_bytes()) { match head { @@ -138,12 +143,22 @@ impl Headers { self.content_length = content_length; Ok(()) } - Err(_) => Err(RequestError::InvalidHeader), + Err(_) => { + Err(RequestError::HeaderError(HttpHeaderError::InvalidValue( + entry[0].to_string(), + entry[1].to_string(), + ))) + } }, Header::ContentType => { match MediaType::try_from(entry[1].trim().as_bytes()) { Ok(_) => Ok(()), - Err(_) => Err(RequestError::UnsupportedHeader), + Err(_) => Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + entry[0].to_string(), + entry[1].to_string(), + ), + )), } } Header::Accept => match MediaType::try_from(entry[1].trim().as_bytes()) { @@ -151,30 +166,52 @@ impl Headers { self.accept = accept_type; Ok(()) } - Err(_) => Err(RequestError::UnsupportedHeader), + Err(_) => Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + entry[0].to_string(), + entry[1].to_string(), + ), + )), }, Header::TransferEncoding => match entry[1].trim() { "chunked" => { self.chunked = true; Ok(()) } - "identity; q=0" => Err(RequestError::InvalidHeader), - _ => Err(RequestError::UnsupportedHeader), + "identity" => Ok(()), + _ => Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + entry[0].to_string(), + entry[1].to_string(), + ), + )), }, Header::Expect => match entry[1].trim() { "100-continue" => { self.expect = true; Ok(()) } - _ => Err(RequestError::InvalidHeader), + _ => Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + entry[0].to_string(), + entry[1].to_string(), + ), + )), }, Header::Server => Ok(()), } } else { - Err(RequestError::UnsupportedHeader) + Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + entry[0].to_string(), + entry[1].to_string(), + ), + )) } } - _ => Err(RequestError::InvalidHeader), + Err(utf8_err) => Err(RequestError::HeaderError( + HttpHeaderError::InvalidUtf8String(utf8_err), + )), } } @@ -231,7 +268,10 @@ impl Headers { break; } match headers.parse_header_line(header_line.as_bytes()) { - Ok(_) | Err(RequestError::UnsupportedHeader) => continue, + Ok(_) + | Err(RequestError::HeaderError(HttpHeaderError::UnsupportedValue(_, _))) => { + continue + } Err(e) => return Err(e), }; } @@ -411,19 +451,29 @@ mod tests { // Invalid header syntax. assert_eq!( header.parse_header_line(b"Expect"), - Err(RequestError::InvalidHeader) + Err(RequestError::HeaderError(HttpHeaderError::InvalidFormat( + "Expect".to_string() + ))) ); // Invalid content length. assert_eq!( header.parse_header_line(b"Content-Length: five"), - Err(RequestError::InvalidHeader) + Err(RequestError::HeaderError(HttpHeaderError::InvalidValue( + "Content-Length".to_string(), + " five".to_string() + ))) ); // Invalid transfer encoding. assert_eq!( header.parse_header_line(b"Transfer-Encoding: gzip"), - Err(RequestError::UnsupportedHeader) + Err(RequestError::HeaderError( + HttpHeaderError::UnsupportedValue( + "Transfer-Encoding".to_string(), + " gzip".to_string() + ) + )) ); // Invalid expect. @@ -431,7 +481,10 @@ mod tests { header .parse_header_line(b"Expect: 102-processing") .unwrap_err(), - RequestError::InvalidHeader + RequestError::HeaderError(HttpHeaderError::UnsupportedValue( + "Expect".to_string(), + " 102-processing".to_string() + )) ); // Unsupported media type. @@ -439,14 +492,19 @@ mod tests { header .parse_header_line(b"Content-Type: application/json-patch") .unwrap_err(), - RequestError::UnsupportedHeader + RequestError::HeaderError(HttpHeaderError::UnsupportedValue( + "Content-Type".to_string(), + " application/json-patch".to_string() + )) ); // Invalid input format. let input: [u8; 10] = [130, 140, 150, 130, 140, 150, 130, 140, 150, 160]; assert_eq!( header.parse_header_line(&input[..]).unwrap_err(), - RequestError::InvalidHeader + RequestError::HeaderError(HttpHeaderError::InvalidUtf8String( + String::from_utf8(input.to_vec()).unwrap_err().utf8_error() + )) ); // Test valid transfer encoding. @@ -480,7 +538,10 @@ mod tests { // Invalid content length. assert_eq!( header.parse_header_line(b"Content-Length: -1"), - Err(RequestError::InvalidHeader) + Err(RequestError::HeaderError(HttpHeaderError::InvalidValue( + "Content-Length".to_string(), + " -1".to_string() + ))) ); } @@ -530,7 +591,7 @@ mod tests { // Bad header. assert_eq!( Header::try_from(b"Encoding").unwrap_err(), - RequestError::InvalidHeader + RequestError::HeaderError(HttpHeaderError::UnsupportedName("encoding".to_string())) ); // Invalid encoding. diff --git a/src/common/mod.rs b/src/common/mod.rs index 2dec180..7b2fb54 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::fmt::{Display, Error, Formatter}; +use std::str::Utf8Error; pub mod headers; @@ -13,6 +14,55 @@ pub mod ascii { pub const CRLF_LEN: usize = 2; } +///Errors associated with a header that is invalid. +#[derive(Debug, PartialEq)] +pub enum HttpHeaderError { + /// The content length specified is longer than the limit imposed by Micro Http. + SizeLimitExceeded(String), + /// The header specified is not supported. + UnsupportedName(String), + /// The value for the specified header is not supported. + UnsupportedValue(String, String), + /// The specified header contains illegal characters. + InvalidUtf8String(Utf8Error), + /// The requested feature is not currently supported. + UnsupportedFeature(String, String), + /// The header is misformatted. + InvalidFormat(String), + ///The value specified is not valid. + InvalidValue(String, String), +} + +impl Display for HttpHeaderError { + fn fmt(&self, f: &mut Formatter) -> Result<(), Error> { + match self { + Self::SizeLimitExceeded(inner) => { + write!(f, "Invalid content length. Header: {}", inner) + } + Self::UnsupportedName(inner) => write!(f, "Unsupported header name. Key: {}", inner), + Self::UnsupportedValue(header_key, header_value) => write!( + f, + "Unsupported value. Key:{}; Value:{}", + header_key, header_value + ), + Self::InvalidUtf8String(header_key) => { + write!(f, "Header contains invalid characters. Key: {}", header_key) + } + Self::UnsupportedFeature(header_key, header_value) => write!( + f, + "Unsupported feature. Key: {}; Value: {}", + header_key, header_value + ), + Self::InvalidFormat(header_key) => { + write!(f, "Header is incorrectly formatted. Key: {}", header_key) + } + Self::InvalidValue(header_name, value) => { + write!(f, "Invalid value. Key:{}; Value:{}", header_name, value) + } + } + } +} + /// Errors associated with parsing the HTTP Request from a u8 slice. #[derive(Debug, PartialEq)] pub enum RequestError { @@ -26,10 +76,8 @@ pub enum RequestError { InvalidUri(&'static str), /// The HTTP Version in the Request is not supported or it is invalid. InvalidHttpVersion(&'static str), - /// The header specified may be valid, but is not supported by this HTTP implementation. - UnsupportedHeader, - /// Header specified is invalid. - InvalidHeader, + /// Header specified is either invalid or not supported by this HTTP implementation. + HeaderError(HttpHeaderError), /// The Request is invalid and cannot be served. InvalidRequest, /// Overflow occurred when parsing a request. @@ -52,8 +100,7 @@ impl Display for RequestError { Self::InvalidHttpMethod(inner) => write!(f, "Invalid HTTP Method: {}", inner), Self::InvalidUri(inner) => write!(f, "Invalid URI: {}", inner), Self::InvalidHttpVersion(inner) => write!(f, "Invalid HTTP Version: {}", inner), - Self::UnsupportedHeader => write!(f, "Unsupported header."), - Self::InvalidHeader => write!(f, "Invalid header."), + Self::HeaderError(inner) => write!(f, "Invalid header. Reason: {}", inner), Self::InvalidRequest => write!(f, "Invalid request."), Self::Overflow => write!(f, "Overflow occurred when parsing a request."), Self::Underflow => write!(f, "Underflow occurred when parsing a request."), @@ -340,10 +387,6 @@ mod tests { format!("{}", RequestError::HeadersWithoutPendingRequest), "No request was pending while the request headers were being parsed." ); - assert_eq!( - format!("{}", RequestError::InvalidHeader), - "Invalid header." - ); assert_eq!( format!("{}", RequestError::InvalidHttpMethod("test")), "Invalid HTTP Method: test" @@ -368,9 +411,60 @@ mod tests { format!("{}", RequestError::Underflow), "Underflow occurred when parsing a request." ); + } + + #[test] + fn test_display_header_error() { assert_eq!( - format!("{}", RequestError::UnsupportedHeader), - "Unsupported header." + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::SizeLimitExceeded("test".to_string())) + ), + "Invalid header. Reason: Invalid content length. Header: test" + ); + assert_eq!( + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::UnsupportedName("test".to_string())) + ), + "Invalid header. Reason: Unsupported header name. Key: test" + ); + assert_eq!( + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::UnsupportedValue( + "test".to_string(), + "test".to_string() + )) + ), + "Invalid header. Reason: Unsupported value. Key:test; Value:test" + ); + let value = String::from_utf8(vec![0, 159]); + assert_eq!( + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::InvalidUtf8String( + value.unwrap_err().utf8_error() + )) + ), + "Invalid header. Reason: Header contains invalid characters. Key: invalid utf-8 sequence of 1 bytes from index 1" + ); + assert_eq!( + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::UnsupportedFeature( + "test".to_string(), + "test".to_string() + )) + ), + "Invalid header. Reason: Unsupported feature. Key: test; Value: test" + ); + assert_eq!( + format!( + "{}", + RequestError::HeaderError(HttpHeaderError::InvalidFormat("test".to_string())) + ), + "Invalid header. Reason: Header is incorrectly formatted. Key: test" ); } diff --git a/src/connection.rs b/src/connection.rs index 2a5b03b..a5b41e7 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -6,7 +6,7 @@ use std::io::{Read, Write}; use crate::common::ascii::{CR, CRLF_LEN, LF}; use crate::common::Body; -pub use crate::common::{ConnectionError, RequestError}; +pub use crate::common::{ConnectionError, HttpHeaderError, RequestError}; use crate::headers::Headers; use crate::request::{find, Request, RequestLine}; use crate::response::{Response, StatusCode}; @@ -270,7 +270,8 @@ impl HttpConnection { let line = &self.buffer[*line_start_index..line_end_index]; match request.headers.parse_header_line(line) { // If a header is unsupported we ignore it. - Ok(_) | Err(RequestError::UnsupportedHeader) => {} + Ok(_) + | Err(RequestError::HeaderError(HttpHeaderError::UnsupportedValue(_, _))) => {} // If parsing the header invalidates the request, we propagate // the error. Err(e) => return Err(ConnectionError::ParseError(e)), @@ -288,7 +289,10 @@ impl HttpConnection { // line end sequence. if *line_start_index == 0 && end_cursor == BUFFER_SIZE { // Header line is longer than BUFFER_SIZE bytes, so it is invalid. - return Err(ConnectionError::ParseError(RequestError::InvalidHeader)); + let utf8_string = String::from_utf8_lossy(&self.buffer); + return Err(ConnectionError::ParseError(RequestError::HeaderError( + HttpHeaderError::SizeLimitExceeded(utf8_string.to_string()), + ))); } // Move the incomplete header line from the end of the buffer to // the beginning, so that we can append the rest of the line and @@ -610,7 +614,10 @@ mod tests { let request_error = conn.try_read().unwrap_err(); assert_eq!( request_error, - ConnectionError::ParseError(RequestError::InvalidHeader) + ConnectionError::ParseError(RequestError::HeaderError(HttpHeaderError::InvalidValue( + "Content-Length".to_string(), + " alpha".to_string() + ))) ); } @@ -642,6 +649,7 @@ mod tests { headers: Headers::new(1400, true, true), body: Some(Body::new(request_body)), }; + assert_eq!(request, expected_request); } @@ -681,9 +689,13 @@ mod tests { request_body.write_all(b"\r\n\r\n").unwrap(); sender.write_all(request_body.as_slice()).unwrap(); assert!(conn.try_read().is_ok()); + + let expected_msg = &format!("head: {}", String::from_utf8(request_body).unwrap())[..1024]; assert_eq!( conn.try_read().unwrap_err(), - ConnectionError::ParseError(RequestError::InvalidHeader) + ConnectionError::ParseError(RequestError::HeaderError( + HttpHeaderError::SizeLimitExceeded(expected_msg.to_string()) + )) ); } @@ -890,7 +902,10 @@ mod tests { .unwrap(); assert_eq!( conn.try_read().unwrap_err(), - ConnectionError::ParseError(RequestError::InvalidHeader) + ConnectionError::ParseError(RequestError::HeaderError(HttpHeaderError::InvalidValue( + "Content-Length".to_string(), + " -1".to_string() + ))) ); } @@ -1022,7 +1037,9 @@ mod tests { }); assert_eq!( conn.parse_headers(&mut 0, BUFFER_SIZE).unwrap_err(), - ConnectionError::ParseError(RequestError::InvalidHeader) + ConnectionError::ParseError(RequestError::HeaderError(HttpHeaderError::InvalidFormat( + "\0".to_string() + ))) ); // OK case: incomplete header line. diff --git a/src/lib.rs b/src/lib.rs index 628169e..16ca0ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -121,4 +121,4 @@ pub use crate::response::{Response, StatusCode}; pub use crate::server::{HttpServer, ServerError, ServerRequest, ServerResponse}; pub use crate::common::headers::{Headers, MediaType}; -pub use crate::common::{Body, Method, Version}; +pub use crate::common::{Body, HttpHeaderError, Method, Version}; diff --git a/src/request.rs b/src/request.rs index b470588..2971f5a 100644 --- a/src/request.rs +++ b/src/request.rs @@ -4,6 +4,7 @@ use std::str::from_utf8; use crate::common::ascii::{CR, CRLF_LEN, LF, SP}; +pub use crate::common::HttpHeaderError; pub use crate::common::RequestError; use crate::common::{Body, Method, Version}; use crate::headers::Headers; @@ -480,14 +481,13 @@ mod tests { Request::try_from(b"PATCH http://localhost/home HTTP/1.1\r\n").unwrap_err(); // Test for an invalid encoding. - let request = Request::try_from( + assert!(Request::try_from( b"PATCH http://localhost/home HTTP/1.1\r\n\ Expect: 100-continue\r\n\ Transfer-Encoding: identity; q=0\r\n\ Content-Length: 26\r\n\r\nthis is not\n\r\na json \nbody", ) - .unwrap_err(); - assert_eq!(request, RequestError::InvalidHeader); + .is_ok()); // Test for an invalid content length. let request = Request::try_from( diff --git a/src/server.rs b/src/server.rs index 720f04e..64949dd 100644 --- a/src/server.rs +++ b/src/server.rs @@ -764,14 +764,14 @@ mod tests { assert!(server.requests().unwrap().is_empty()); assert!(server.requests().unwrap().is_empty()); - let mut buf: [u8; 198] = [0; 198]; + let mut buf: [u8; 255] = [0; 255]; assert!(socket.read(&mut buf[..]).unwrap() > 0); let error_message = b"HTTP/1.1 400 \r\n\ Server: Firecracker API\r\n\ Connection: keep-alive\r\n\ Content-Type: application/json\r\n\ - Content-Length: 80\r\n\r\n{ \"error\": \"Invalid header.\n\ - All previous unanswered requests will be dropped.\" }"; + Content-Length: 136\r\n\r\n{ \"error\": \"Invalid header. \ + Reason: Invalid value. Key:Content-Length; Value: alpha\nAll previous unanswered requests will be dropped.\" }"; assert_eq!(&buf[..], &error_message[..]); }