From b025536692c81565fd4358ec5ee84cb4132475d8 Mon Sep 17 00:00:00 2001 From: Jose Quintana Date: Fri, 2 Jul 2021 12:22:57 +0200 Subject: [PATCH] refactor: minor code syntax and docs improvements --- src/compression.rs | 2 +- src/control_headers.rs | 4 ++-- src/error_page.rs | 8 ++++---- src/handler.rs | 23 +++++++++++------------ src/security_headers.rs | 2 +- src/server.rs | 19 ++++++++++--------- src/signals.rs | 12 ++++++------ src/static_files.rs | 20 +++++++++++++------- 8 files changed, 48 insertions(+), 42 deletions(-) diff --git a/src/compression.rs b/src/compression.rs index e2aa353..d224963 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -15,7 +15,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use tokio_util::io::{ReaderStream, StreamReader}; -use crate::error::Result; +use crate::Result; /// Contains a fixed list of common text-based MIME types in order to apply compression. pub const TEXT_MIME_TYPES: [&str; 16] = [ diff --git a/src/control_headers.rs b/src/control_headers.rs index 4641583..19122a9 100644 --- a/src/control_headers.rs +++ b/src/control_headers.rs @@ -1,5 +1,5 @@ // An arbitrary `Cache-Control` headers functionality for incoming requests based on a set of file types. -// Note: Since it's a ad-hoc feature it could be subject to change. +// Note: Since it's an ad-hoc feature it could be subject to change in the future. // See https://github.com/joseluisq/static-web-server/issues/30 use headers::{CacheControl, HeaderMapExt}; @@ -13,7 +13,7 @@ const CACHE_EXT_ONE_YEAR: [&str; 30] = [ ]; /// It appends a `Cache-Control` header to a response if that one is part of a set of file types. -pub fn with_cache_control(ext: &str, resp: &mut Response) { +pub fn append_headers(ext: &str, resp: &mut Response) { // Default max-age value in seconds (one day) let mut max_age = 60 * 60 * 24_u64; diff --git a/src/error_page.rs b/src/error_page.rs index 88cbde9..b4bc64e 100644 --- a/src/error_page.rs +++ b/src/error_page.rs @@ -2,16 +2,16 @@ use headers::{AcceptRanges, ContentLength, ContentType, HeaderMapExt}; use hyper::{Body, Method, Response, StatusCode}; use once_cell::sync::OnceCell; -use crate::error::Result; +use crate::Result; pub static PAGE_404: OnceCell = OnceCell::new(); pub static PAGE_50X: OnceCell = OnceCell::new(); /// It returns a HTTP error response which also handles available `404` or `50x` HTML content. -pub fn get_error_response(method: &Method, status_code: &StatusCode) -> Result> { +pub fn error_response(method: &Method, status_code: &StatusCode) -> Result> { tracing::warn!(method = ?method, status = status_code.as_u16(), error = ?status_code.to_owned()); - // Check for 4xx and 50x status codes and handle their corresponding HTML content + // Check for 4xx/50x status codes and handle their corresponding HTML content let mut error_page_content = String::new(); let status_code = match status_code { // 4xx @@ -57,7 +57,7 @@ pub fn get_error_response(method: &Method, status_code: &StatusCode) -> Result { - // HTML content for error status codes 50x and their HTML content + // HTML content check for status codes 50x error_page_content = match PAGE_50X.get() { Some(s) => s.to_owned(), None => { diff --git a/src/handler.rs b/src/handler.rs index c755d46..d7a80fe 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -2,10 +2,10 @@ use http::StatusCode; use hyper::{Body, Request, Response}; use std::{future::Future, path::PathBuf, sync::Arc}; -use crate::{compression, control_headers, cors, security_headers, static_files}; -use crate::{error_page, Error, Result}; +use crate::{compression, control_headers, cors, error_page, security_headers, static_files}; +use crate::{Error, Result}; -// It defines options for a request handler. +/// It defines options for a request handler. pub struct RequestHandlerOpts { pub root_dir: PathBuf, pub compression: bool, @@ -14,12 +14,13 @@ pub struct RequestHandlerOpts { pub security_headers: bool, } -// It defines the main request handler for Hyper service request. +/// It defines the main request handler used by the Hyper service request. pub struct RequestHandler { pub opts: RequestHandlerOpts, } impl RequestHandler { + /// Main entry point for incoming requests. pub fn handle<'a>( &'a self, req: &'a mut Request, @@ -41,19 +42,17 @@ impl RequestHandler { } Err(e) => { tracing::debug!("cors error kind: {:?}", e); - return error_page::get_error_response(method, &StatusCode::FORBIDDEN); + return error_page::error_response(method, &StatusCode::FORBIDDEN); } }; } // Static files - match static_files::handle_request(method, headers, root_dir, uri_path, dir_listing) - .await - { + match static_files::handle(method, headers, root_dir, uri_path, dir_listing).await { Ok(mut resp) => { - // Append Security Headers + // Append security headers if self.opts.security_headers { - security_headers::with_security_headers(&mut resp); + security_headers::append_headers(&mut resp); } // Auto compression based on the `Accept-Encoding` header @@ -63,11 +62,11 @@ impl RequestHandler { // Append `Cache-Control` headers for web assets let ext = uri_path.to_lowercase(); - control_headers::with_cache_control(&ext, &mut resp); + control_headers::append_headers(&ext, &mut resp); Ok(resp) } - Err(status) => error_page::get_error_response(method, &status), + Err(status) => error_page::error_response(method, &status), } } } diff --git a/src/security_headers.rs b/src/security_headers.rs index c9f82f5..73ff7bb 100644 --- a/src/security_headers.rs +++ b/src/security_headers.rs @@ -6,7 +6,7 @@ use hyper::{Body, Response}; /// It appends security headers like `Strict-Transport-Security: max-age=63072000; includeSubDomains; preload` (2 years max-age), ///`X-Frame-Options: DENY`, `X-XSS-Protection: 1; mode=block` and `Content-Security-Policy: frame-ancestors 'self'`. -pub fn with_security_headers(resp: &mut Response) { +pub fn append_headers(resp: &mut Response) { // Strict-Transport-Security (HSTS) resp.headers_mut().insert( STRICT_TRANSPORT_SECURITY, diff --git a/src/server.rs b/src/server.rs index adf6942..6bf08cf 100644 --- a/src/server.rs +++ b/src/server.rs @@ -6,8 +6,7 @@ use structopt::StructOpt; use crate::handler::{RequestHandler, RequestHandlerOpts}; use crate::tls::{TlsAcceptor, TlsConfigBuilder}; -use crate::Result; -use crate::{config::Config, service::RouterService}; +use crate::{config::Config, service::RouterService, Result}; use crate::{cors, error_page, helpers, logger, signals}; /// Define a multi-thread HTTP or HTTP/2 web server. @@ -49,14 +48,13 @@ impl Server { Ok(()) } - /// Run the inner Hyper `HyperServer` forever on the current thread with the given configuration. + /// Run the inner Hyper `HyperServer` (HTTP1/HTTP2) forever on the current thread + // using the given configuration. async fn start_server(self) -> Result { let opts = &self.opts; logger::init(&opts.log_level)?; - tracing::info!("runtime worker threads: {}", self.threads); - let (tcplistener, addr_string); match opts.fd { Some(fd) => { @@ -89,6 +87,10 @@ impl Server { .set(helpers::read_file_content(opts.page50x.as_ref())) .expect("page 50x is not initialized"); + // Number of worker threads option + let threads = self.threads; + tracing::info!("runtime worker threads: {}", self.threads); + // Security Headers option let security_headers = opts.security_headers; tracing::info!("security headers: enabled={}", security_headers); @@ -101,10 +103,7 @@ impl Server { let dir_listing = opts.directory_listing; tracing::info!("directory listing: enabled={}", dir_listing); - // Spawn a new Tokio asynchronous server task with its given options - let threads = self.threads; - - // CORS support + // CORS option let cors = cors::new(opts.cors_allow_origins.trim().to_owned()); // Create a service router for Hyper @@ -118,6 +117,8 @@ impl Server { }, }); + // Spawn a new Tokio asynchronous task with its given options + if opts.http2 { // HTTP/2 + TLS diff --git a/src/signals.rs b/src/signals.rs index a0a75f3..49941dd 100644 --- a/src/signals.rs +++ b/src/signals.rs @@ -3,19 +3,19 @@ use std::sync::mpsc::channel; use crate::{Context, Result}; -/// It waits for a `Ctrl-C` signal. +/// It waits for a `Ctrl-C` incoming signal. pub fn wait_for_ctrl_c() -> Result { let (tx, rx) = channel(); - ctrlc::set_handler(move || tx.send(()).expect("Could not send signal on channel.")) - .with_context(|| "Error setting Ctrl-C handler.".to_owned())?; + ctrlc::set_handler(move || tx.send(()).expect("could not send signal on channel")) + .with_context(|| "error setting Ctrl-C handler".to_owned())?; - tracing::info!("Press Ctrl+C to shutdown server."); + tracing::info!("press Ctrl+C to shutdown server"); rx.recv() - .with_context(|| "Could not receive signal from channel.".to_owned())?; + .with_context(|| "could not receive signal from channel".to_owned())?; - tracing::warn!("Ctrl+C signal caught, shutting down server execution."); + tracing::warn!("Ctrl+C signal caught, shutting down server execution"); Ok(()) } diff --git a/src/static_files.rs b/src/static_files.rs index d1ab6d2..7c12831 100644 --- a/src/static_files.rs +++ b/src/static_files.rs @@ -24,11 +24,11 @@ use tokio::fs::File as TkFile; use tokio::io::AsyncSeekExt; use tokio_util::io::poll_read_buf; -use crate::error::Result; +use crate::Result; -/// Entry point to handle web server requests which map to specific files +/// Entry point to handle incoming requests which map to specific files /// on file system and return a file response. -pub async fn handle_request( +pub async fn handle( method: &Method, headers: &HeaderMap, base: &Path, @@ -72,6 +72,8 @@ pub async fn handle_request( file_reply(headers, (filepath, meta, auto_index)).await } +/// Convert an incoming uri into a valid and sanitized path then returns a tuple +// with the path as well as its file metadata and an auto index check if it's a directory. fn path_from_tail( base: PathBuf, tail: &str, @@ -96,6 +98,9 @@ fn path_from_tail( }) } +/// Provides directory listing support for the current request. +/// Note that this function is a highly dependent on `path_from_tail()` +// function which must be called first. See `handle()` more for details. fn directory_listing<'a>( method: &'a Method, current_path: &'a str, @@ -103,10 +108,11 @@ fn directory_listing<'a>( ) -> impl Future, StatusCode>> + Send + 'a { let is_head = method == Method::HEAD; - // Note: it's safe to call `parent()` since `filepath` value - // always maps to a file on root directory boundaries. - // See `path_from_tail()` function which sanitizes - // the requested path before to be delegated here. + // Note: it's safe to call `parent()` here since `filepath` + // value always refer to a path with file ending and under + // a root directory boundary. + // See `path_from_tail()` function which sanitizes the requested + // path before to be delegated here. let parent = filepath.parent().unwrap_or(filepath); tokio::fs::read_dir(parent).then(move |res| match res { -- libgit2 1.7.2