diff --git a/CHANGELOG.md b/CHANGELOG.md index ebdfaf9..2387c30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +# Unreleased + +## Added + +- Implemented `Eq` for `common::headers::Encoding`, `common::headers::MediaType`, + `common::headers::Headers`, `common::HttpHeaderError`, `common::Body`, `common::Version`, + `common::RequestError`, `request::Uri`, `request::RequestLine`, `response::StatusCode`, + `response::ResponseHeaders` + +## Changed + +- Mark `HttpServer::new_from_fd` as `unsafe` as the correctness of the unsafe code + in this method relies on an invariant the caller has to uphold + # v0.1.0 - micro-http v0.1.0 first release. diff --git a/src/common/headers.rs b/src/common/headers.rs index 9d5c921..9306951 100644 --- a/src/common/headers.rs +++ b/src/common/headers.rs @@ -78,7 +78,7 @@ impl Header { /// invalidate our request as we don't support the full set of HTTP/1.1 specification. /// Such header entries are "Transfer-Encoding: identity; q=0", which means a compression /// algorithm is applied to the body of the request, or "Expect: 103-checkpoint". -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct Headers { /// The `Content-Length` header field tells us how many bytes we need to receive /// from the source after the headers. @@ -310,7 +310,7 @@ impl Headers { } /// Wrapper over supported AcceptEncoding. -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Encoding {} impl Encoding { @@ -367,7 +367,7 @@ impl Encoding { } /// Wrapper over supported Media Types. -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum MediaType { /// Media Type: "text/plain". PlainText, @@ -450,8 +450,8 @@ mod tests { fn test_default() { let headers = Headers::default(); assert_eq!(headers.content_length(), 0); - assert_eq!(headers.chunked(), false); - assert_eq!(headers.expect(), false); + assert!(!headers.chunked()); + assert!(!headers.expect()); assert_eq!(headers.accept(), MediaType::PlainText); assert_eq!(headers.custom_entries(), &HashMap::default()); } diff --git a/src/common/mod.rs b/src/common/mod.rs index ccdfb5e..322e65e 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -15,7 +15,7 @@ pub mod ascii { } ///Errors associated with a header that is invalid. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum HttpHeaderError { /// The header is misformatted. InvalidFormat(String), @@ -64,7 +64,7 @@ impl Display for HttpHeaderError { } /// Errors associated with parsing the HTTP Request from a u8 slice. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum RequestError { /// No request was pending while the request body was being parsed. BodyWithoutPendingRequest, @@ -195,7 +195,7 @@ impl Display for ServerError { /// assert_eq!(body.raw(), b"This is a test body."); /// assert_eq!(body.len(), 20); /// ``` -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Body { /// Body of the HTTP message as bytes. pub body: Vec, @@ -281,7 +281,7 @@ impl Method { /// let version = Version::try_from(b"http/1.1"); /// assert!(version.is_err()); /// ``` -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Version { /// HTTP/1.0 Http10, diff --git a/src/connection.rs b/src/connection.rs index a7386f6..3bd0d59 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -176,7 +176,7 @@ impl HttpConnection { iov_len: buf.len(), }]; - // Safe because we have mutably borrowed buf and it's safe to write + // SAFETY: Safe because we have mutably borrowed buf and it's safe to write // arbitrary data to a slice. let (read_count, fd_count) = unsafe { self.stream @@ -189,7 +189,7 @@ impl HttpConnection { fds.iter() .take(fd_count) .map(|fd| { - // Safe because all fds are owned by us after they have been + // SAFETY: Safe because all fds are owned by us after they have been // received through the socket. unsafe { File::from_raw_fd(*fd) } }) @@ -1048,11 +1048,11 @@ mod tests { let mut file1 = TempFile::new().unwrap().into_file(); let mut file2 = TempFile::new().unwrap().into_file(); let mut file3 = TempFile::new().unwrap().into_file(); - file1.write(b"foo").unwrap(); + file1.write_all(b"foo").unwrap(); file1.seek(SeekFrom::Start(0)).unwrap(); - file2.write(b"bar").unwrap(); + file2.write_all(b"bar").unwrap(); file2.seek(SeekFrom::Start(0)).unwrap(); - file3.write(b"foobar").unwrap(); + file3.write_all(b"foobar").unwrap(); file3.seek(SeekFrom::Start(0)).unwrap(); // Send 2 file descriptors along with 3 bytes of data. diff --git a/src/request.rs b/src/request.rs index 146ca3f..bff34f5 100644 --- a/src/request.rs +++ b/src/request.rs @@ -26,7 +26,7 @@ pub(crate) fn find(bytes: &[u8], sequence: &[u8]) -> Option { /// Wrapper over HTTP URIs. /// /// The `Uri` can not be used directly and it is only accessible from an HTTP Request. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Uri { string: String, } @@ -85,7 +85,7 @@ impl Uri { } /// Wrapper over an HTTP Request Line. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct RequestLine { method: Method, uri: Uri, @@ -505,8 +505,8 @@ mod tests { assert_eq!(request.uri(), &Uri::new("http://localhost/home")); assert_eq!(request.http_version(), Version::Http11); assert_eq!(request.method(), Method::Patch); - assert_eq!(request.headers.chunked(), true); - assert_eq!(request.headers.expect(), true); + assert!(request.headers.chunked()); + assert!(request.headers.expect()); assert_eq!(request.headers.content_length(), 26); assert_eq!( request.body.unwrap().body, @@ -539,8 +539,8 @@ mod tests { assert_eq!(request.uri(), &Uri::new("http://localhost/")); assert_eq!(request.http_version(), Version::Http10); assert_eq!(request.method(), Method::Get); - assert_eq!(request.headers.chunked(), false); - assert_eq!(request.headers.expect(), false); + assert!(!request.headers.chunked()); + assert!(!request.headers.expect()); assert_eq!(request.headers.content_length(), 0); assert!(request.body.is_none()); diff --git a/src/response.rs b/src/response.rs index 3be39ac..f06dae4 100644 --- a/src/response.rs +++ b/src/response.rs @@ -12,7 +12,7 @@ use crate::Method; /// /// The status code is defined as specified in the /// [RFC](https://tools.ietf.org/html/rfc7231#section-6). -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum StatusCode { /// 100, Continue Continue, @@ -84,7 +84,7 @@ impl StatusLine { /// Wrapper over the list of headers associated with a HTTP Response. /// When creating a ResponseHeaders object, the content type is initialized to `text/plain`. /// The content type can be updated with a call to `set_content_type`. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct ResponseHeaders { content_length: i32, content_type: MediaType, diff --git a/src/router.rs b/src/router.rs index 1a94185..a2171c3 100644 --- a/src/router.rs +++ b/src/router.rs @@ -92,7 +92,7 @@ impl HttpRoutes { request.uri().get_abs_path() ); let mut response = match self.routes.get(&path) { - Some(route) => route.handle_request(&request, &argument), + Some(route) => route.handle_request(request, argument), None => Response::new(Version::Http11, StatusCode::NotFound), }; diff --git a/src/server.rs b/src/server.rs index 770de46..5e2083f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -133,7 +133,7 @@ impl ClientConnection { let mut error_response = Response::new(Version::Http11, StatusCode::BadRequest); error_response.set_body(Body::new(format!( "{{ \"error\": \"{}\nAll previous unanswered requests will be dropped.\" }}", - inner.to_string() + inner ))); self.connection.enqueue_response(error_response); } @@ -285,16 +285,17 @@ impl HttpServer { /// Constructor for `HttpServer`. /// - /// Note that this function requires the socket_fd to be solely owned + /// Returns the newly formed `HttpServer`. + /// + /// # Safety + /// This function requires the socket_fd to be solely owned /// and not be associated with another File in the caller as it uses /// the unsafe `UnixListener::from_raw_fd method`. /// - /// Returns the newly formed `HttpServer`. - /// /// # Errors /// Returns an `IOError` when `epoll::create` fails. - pub fn new_from_fd(socket_fd: RawFd) -> Result { - let socket = unsafe { UnixListener::from_raw_fd(socket_fd) }; + pub unsafe fn new_from_fd(socket_fd: RawFd) -> Result { + let socket = UnixListener::from_raw_fd(socket_fd); let epoll = epoll::Epoll::new().map_err(ServerError::IOError)?; Ok(HttpServer { socket, @@ -639,6 +640,8 @@ impl HttpServer { #[cfg(test)] mod tests { + #![allow(clippy::undocumented_unsafe_blocks)] + use super::*; use std::io::{Read, Write}; use std::net::Shutdown; @@ -764,7 +767,7 @@ mod tests { let socket_listener = UnixListener::bind(path_to_socket.as_path()).unwrap(); let socket_fd = socket_listener.into_raw_fd(); - let mut server = HttpServer::new_from_fd(socket_fd).unwrap(); + let mut server = unsafe { HttpServer::new_from_fd(socket_fd).unwrap() }; server.start_server().unwrap(); // Test one incoming connection.