Merge pull request #582 from aliemjay/web-v4

migrate to actix-web v4.0-beta

5 files changed, 75 insertions, 61 deletions

diff --git a/src/auth.rs b/src/auth.rs
index 1a913d5..7c77758 100644
--- a/src/auth.rs
+++ b/src/auth.rs
@@ -1,8 +1,9 @@
-use actix_web::dev::ServiceRequest;
+use actix_web::dev::{Service, ServiceRequest, ServiceResponse};
 use actix_web::http::{header, StatusCode};
-use actix_web::{HttpRequest, HttpResponse, Result};
-use actix_web_httpauth::extractors::basic::BasicAuth;
+use actix_web::{HttpRequest, HttpResponse};
+use futures::future::Either;
 use sha2::{Digest, Sha256, Sha512};
+use std::future::{ready, Future};
 
 use crate::errors::{self, ContextualError};
 use crate::renderer;
@@ -14,12 +15,16 @@ pub struct BasicAuthParams {
     pub password: String,
 }
 
-impl From<BasicAuth> for BasicAuthParams {
-    fn from(auth: BasicAuth) -> Self {
-        Self {
+impl BasicAuthParams {
+    fn try_from_request(req: &HttpRequest) -> actix_web::Result<Self> {
+        use actix_web::http::header::Header;
+        use actix_web_httpauth::headers::authorization::{Authorization, Basic};
+
+        let auth = Authorization::<Basic>::parse(req)?.into_scheme();
+        Ok(Self {
             username: auth.user_id().to_string(),
             password: auth.password().unwrap_or(&"".into()).to_string(),
-        }
+        })
     }
 }
 
@@ -72,25 +77,48 @@ pub fn get_hash<T: Digest>(text: &str) -> Vec<u8> {
     hasher.finalize().to_vec()
 }
 
-pub async fn handle_auth(req: ServiceRequest, cred: BasicAuth) -> Result<ServiceRequest> {
+/// When authentication succedes, return the request to be passed to downstream services.
+/// Otherwise, return an error response
+fn handle_auth(req: ServiceRequest) -> Result<ServiceRequest, ServiceResponse> {
     let (req, pl) = req.into_parts();
     let required_auth = &req.app_data::<crate::MiniserveConfig>().unwrap().auth;
 
-    if match_auth(cred.into(), required_auth) {
-        Ok(ServiceRequest::from_parts(req, pl).unwrap_or_else(|_| unreachable!()))
-    } else {
-        Err(HttpResponse::Unauthorized()
-            .header(
-                header::WWW_AUTHENTICATE,
-                header::HeaderValue::from_static("Basic realm=\"miniserve\""),
-            )
-            .body(build_unauthorized_response(
-                &req,
-                ContextualError::InvalidHttpCredentials,
-                true,
-                StatusCode::UNAUTHORIZED,
-            ))
-            .into())
+    if required_auth.is_empty() {
+        // auth is disabled by configuration
+        return Ok(ServiceRequest::from_parts(req, pl));
+    } else if let Ok(cred) = BasicAuthParams::try_from_request(&req) {
+        if match_auth(cred, required_auth) {
+            return Ok(ServiceRequest::from_parts(req, pl));
+        }
+    }
+
+    // auth failed; render and return the error response
+    let resp = HttpResponse::Unauthorized()
+        .append_header((
+            header::WWW_AUTHENTICATE,
+            header::HeaderValue::from_static("Basic realm=\"miniserve\""),
+        ))
+        .body(build_unauthorized_response(
+            &req,
+            ContextualError::InvalidHttpCredentials,
+            true,
+            StatusCode::UNAUTHORIZED,
+        ));
+
+    Err(ServiceResponse::new(req, resp))
+}
+
+pub fn auth_middleware<S>(
+    req: ServiceRequest,
+    srv: &S,
+) -> impl Future<Output = actix_web::Result<ServiceResponse>> + 'static
+where
+    S: Service<ServiceRequest, Response = ServiceResponse, Error = actix_web::Error>,
+    S::Future: 'static,
+{
+    match handle_auth(req) {
+        Ok(req) => Either::Left(srv.call(req)),
+        Err(resp) => Either::Right(ready(Ok(resp))),
     }
 }
 
diff --git a/src/file_upload.rs b/src/file_upload.rs
index 3edb09e..5faa67f 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -205,7 +205,7 @@ pub fn upload_file(
             .then(move |e| match e {
                 Ok(_) => future::ok(
                     HttpResponse::SeeOther()
-                        .header(header::LOCATION, return_path)
+                        .append_header((header::LOCATION, return_path))
                         .finish(),
                 ),
                 Err(e) => create_error_response(
diff --git a/src/listing.rs b/src/listing.rs
index 0bcc5ef..33a0342 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -2,7 +2,7 @@ use actix_web::body::Body;
 use actix_web::dev::ServiceResponse;
 use actix_web::http::StatusCode;
 use actix_web::web::Query;
-use actix_web::{HttpRequest, HttpResponse, Result};
+use actix_web::{HttpRequest, HttpResponse};
 use bytesize::ByteSize;
 use percent_encoding::{percent_decode_str, utf8_percent_encode};
 use qrcodegen::{QrCode, QrCodeEcc};
@@ -142,7 +142,7 @@ impl Breadcrumb {
     }
 }
 
-pub async fn file_handler(req: HttpRequest) -> Result<actix_files::NamedFile> {
+pub async fn file_handler(req: HttpRequest) -> actix_web::Result<actix_files::NamedFile> {
     let path = &req.app_data::<crate::MiniserveConfig>().unwrap().path;
     actix_files::NamedFile::open(path).map_err(Into::into)
 }
@@ -169,25 +169,10 @@ pub fn directory_listing(
     dirs_first: bool,
     hide_version_footer: bool,
     title: Option<String>,
-) -> Result<ServiceResponse, io::Error> {
+) -> io::Result<ServiceResponse> {
     use actix_web::dev::BodyEncoding;
     let serve_path = req.path();
 
-    // In case the current path is a directory, we want to make sure that the current URL ends
-    // on a slash ("/").
-    if !serve_path.ends_with('/') {
-        let query = match req.query_string() {
-            "" => String::new(),
-            _ => format!("?{}", req.query_string()),
-        };
-        return Ok(ServiceResponse::new(
-            req.clone(),
-            HttpResponse::MovedPermanently()
-                .header("Location", format!("{}/{}", serve_path, query))
-                .body("301"),
-        ));
-    }
-
     let base = Path::new(serve_path);
     let random_route_abs = format!("/{}", random_route.clone().unwrap_or_default());
     let is_root = base.parent().is_none() || Path::new(&req.path()) == Path::new(&random_route_abs);
@@ -243,7 +228,7 @@ pub fn directory_listing(
     if let Some(url) = query_params.qrcode {
         let res = match QrCode::encode_text(&url, QrCodeEcc::Medium) {
             Ok(qr) => HttpResponse::Ok()
-                .header("Content-Type", "image/svg+xml")
+                .append_header(("Content-Type", "image/svg+xml"))
                 .body(qr_to_svg_string(&qr, 2)),
             Err(err) => {
                 log::error!("URL is invalid (too long?): {:?}", err);
@@ -376,7 +361,7 @@ pub fn directory_listing(
         // We will create the archive in a separate thread, and stream the content using a pipe.
         // The pipe is made of a futures channel, and an adapter to implement the `Write` trait.
         // Include 10 messages of buffer for erratic connection speeds.
-        let (tx, rx) = futures::channel::mpsc::channel::<Result<actix_web::web::Bytes, ()>>(10);
+        let (tx, rx) = futures::channel::mpsc::channel::<io::Result<actix_web::web::Bytes>>(10);
         let pipe = crate::pipe::Pipe::new(tx);
 
         // Start the actual archive creation in a separate thread.
@@ -392,11 +377,11 @@ pub fn directory_listing(
             HttpResponse::Ok()
                 .content_type(archive_method.content_type())
                 .encoding(archive_method.content_encoding())
-                .header("Content-Transfer-Encoding", "binary")
-                .header(
+                .append_header(("Content-Transfer-Encoding", "binary"))
+                .append_header((
                     "Content-Disposition",
                     format!("attachment; filename={:?}", file_name),
-                )
+                ))
                 .body(actix_web::body::BodyStream::new(rx)),
         ))
     } else {
diff --git a/src/main.rs b/src/main.rs
index c60e153..036d796 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -10,7 +10,6 @@ use actix_web::{
     Responder,
 };
 use actix_web::{middleware, App, HttpRequest, HttpResponse};
-use actix_web_httpauth::middleware::HttpAuthentication;
 use anyhow::Result;
 use log::{error, warn};
 use structopt::clap::crate_version;
@@ -213,10 +212,11 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
         App::new()
             .wrap(configure_header(&inside_config.clone()))
             .app_data(inside_config.clone())
-            .wrap(middleware::Condition::new(
-                !inside_config.auth.is_empty(),
-                HttpAuthentication::basic(auth::handle_auth),
-            ))
+            // we should use `actix_web_httpauth::middleware::HttpAuthentication`
+            // but it is unfortuantrly broken
+            // see: https://github.com/actix/actix-extras/issues/127
+            // TODO replace this when fixed upstream
+            .wrap_fn(auth::auth_middleware)
             .wrap(middleware::Logger::default())
             .route(
                 &format!("/{}", inside_config.favicon_route),
@@ -345,6 +345,7 @@ fn configure_app(app: &mut web::ServiceConfig, conf: &MiniserveConfig) {
                     )
                 })
                 .prefer_utf8(true)
+                .redirect_to_slash_directory()
                 .default_handler(web::to(error_404));
             Some(files)
         }
@@ -417,14 +418,14 @@ async fn error_404(req: HttpRequest) -> HttpResponse {
 
 async fn favicon() -> impl Responder {
     let logo = include_str!("../data/logo.svg");
-    web::HttpResponse::Ok()
-        .set(ContentType(mime::IMAGE_SVG))
+    HttpResponse::Ok()
+        .insert_header(ContentType(mime::IMAGE_SVG))
         .message_body(logo.into())
 }
 
 async fn css() -> impl Responder {
     let css = include_str!(concat!(env!("OUT_DIR"), "/style.css"));
-    web::HttpResponse::Ok()
-        .set(ContentType(mime::TEXT_CSS))
+    HttpResponse::Ok()
+        .insert_header(ContentType(mime::TEXT_CSS))
         .message_body(css.into())
 }
diff --git a/src/pipe.rs b/src/pipe.rs
index 374a45f..6bf32c2 100644
--- a/src/pipe.rs
+++ b/src/pipe.rs
@@ -3,19 +3,19 @@ use actix_web::web::{Bytes, BytesMut};
 use futures::channel::mpsc::Sender;
 use futures::executor::block_on;
 use futures::sink::SinkExt;
-use std::io::{Error, ErrorKind, Result, Write};
+use std::io::{self, Error, ErrorKind, Write};
 
 /// Adapter to implement the `std::io::Write` trait on a `Sender<Bytes>` from a futures channel.
 ///
 /// It uses an intermediate buffer to transfer packets.
 pub struct Pipe {
-    dest: Sender<std::result::Result<Bytes, ()>>,
+    dest: Sender<io::Result<Bytes>>,
     bytes: BytesMut,
 }
 
 impl Pipe {
     /// Wrap the given sender in a `Pipe`.
-    pub fn new(destination: Sender<std::result::Result<Bytes, ()>>) -> Self {
+    pub fn new(destination: Sender<io::Result<Bytes>>) -> Self {
         Pipe {
             dest: destination,
             bytes: BytesMut::new(),
@@ -30,7 +30,7 @@ impl Drop for Pipe {
 }
 
 impl Write for Pipe {
-    fn write(&mut self, buf: &[u8]) -> Result<usize> {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         // We are given a slice of bytes we do not own, so we must start by copying it.
         self.bytes.extend_from_slice(buf);
 
@@ -42,7 +42,7 @@ impl Write for Pipe {
         Ok(buf.len())
     }
 
-    fn flush(&mut self) -> Result<()> {
+    fn flush(&mut self) -> io::Result<()> {
         block_on(self.dest.flush()).map_err(|e| Error::new(ErrorKind::UnexpectedEof, e))
     }
 }