Clear all clippy warnings

Additionally, HttpServer::new_from_fd has been marked as unsafe,
since the correctness of the unsafe call within the function
relies on the caller upholding an invariant

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>

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.

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());
     }

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<u8>,
@@ -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,

src/connection.rs

@@ -176,7 +176,7 @@ impl<T: Read + Write + ScmSocket> HttpConnection<T> {
             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<T: Read + Write + ScmSocket> HttpConnection<T> {
             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.

src/request.rs

@@ -26,7 +26,7 @@ pub(crate) fn find(bytes: &[u8], sequence: &[u8]) -> Option<usize> {
 /// 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());
 

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,

src/router.rs

@@ -92,7 +92,7 @@ impl<T: Send> HttpRoutes<T> {
             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),
         };
 

src/server.rs

@@ -133,7 +133,7 @@ impl<T: Read + Write + ScmSocket> ClientConnection<T> {
                 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<Self> {
-        let socket = unsafe { UnixListener::from_raw_fd(socket_fd) };
+    pub unsafe fn new_from_fd(socket_fd: RawFd) -> Result<Self> {
+        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.