Merge pull request #60 from vojta7/bug_fixes

Bug fixes

1 files changed, 34 insertions, 24 deletions

diff --git a/src/file_upload.rs b/src/file_upload.rs
index 273d12c..534083c 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -4,7 +4,7 @@ use actix_web::{
     dev, http::header, multipart, FromRequest, FutureResponse, HttpMessage, HttpRequest,
     HttpResponse, Query,
 };
-use futures::{future, Future, Stream};
+use futures::{future, future::FutureResult, Future, Stream};
 use serde::Deserialize;
 use std::{
     fs,
@@ -101,7 +101,15 @@ fn handle_multipart(
 /// invalid.
 /// This method returns future.
 pub fn upload_file(req: &HttpRequest<crate::MiniserveConfig>) -> FutureResponse<HttpResponse> {
-    let app_root_dir = req.state().path.clone().canonicalize().unwrap();
+    let return_path: String = req.headers()[header::REFERER]
+        .to_str()
+        .unwrap_or("/")
+        .to_owned();
+    let app_root_dir = if let Ok(dir) = req.state().path.canonicalize() {
+        dir
+    } else {
+        return Box::new(create_error_response("Internal server error", &return_path));
+    };
     let path = match Query::<QueryParameters>::extract(req) {
         Ok(query) => {
             if let Ok(stripped_path) = query.path.strip_prefix(Component::RootDir) {
@@ -111,23 +119,17 @@ pub fn upload_file(req: &HttpRequest<crate::MiniserveConfig>) -> FutureResponse<
             }
         }
         Err(_) => {
-            return Box::new(future::ok(
-                HttpResponse::BadRequest().body("Unspecified parameter path"),
+            return Box::new(create_error_response(
+                "Unspecified parameter path",
+                &return_path,
             ))
         }
     };
-    // this is really ugly I will try to think about something smarter
-    let return_path: String = req.headers()[header::REFERER]
-        .clone()
-        .to_str()
-        .unwrap_or("/")
-        .to_owned();
-    let r_p2 = return_path.clone();
 
     // If the target path is under the app root directory, save the file.
-    let target_dir = match &app_root_dir.clone().join(path.clone()).canonicalize() {
+    let target_dir = match &app_root_dir.clone().join(path).canonicalize() {
         Ok(path) if path.starts_with(&app_root_dir) => path.clone(),
-        _ => return Box::new(future::ok(HttpResponse::BadRequest().body("Invalid path"))),
+        _ => return Box::new(create_error_response("Invalid path", &return_path)),
     };
     let overwrite_files = req.state().overwrite_files;
     Box::new(
@@ -136,17 +138,25 @@ pub fn upload_file(req: &HttpRequest<crate::MiniserveConfig>) -> FutureResponse<
             .map(move |item| handle_multipart(item, target_dir.clone(), overwrite_files))
             .flatten()
             .collect()
-            .map(move |_| {
-                HttpResponse::TemporaryRedirect()
-                    .header(header::LOCATION, return_path.to_string())
-                    .finish()
-            })
-            .or_else(move |e| {
-                let error_description = format!("{}", e);
-                future::ok(
-                    HttpResponse::BadRequest()
-                        .body(file_upload_error(&error_description, &r_p2.clone()).into_string()),
-                )
+            .then(move |e| match e {
+                Ok(_) => future::ok(
+                    HttpResponse::SeeOther()
+                        .header(header::LOCATION, return_path.to_string())
+                        .finish(),
+                ),
+                Err(e) => create_error_response(&e.to_string(), &return_path),
             }),
     )
 }
+
+/// Convenience method for creating response errors, if file upload fails.
+fn create_error_response(
+    description: &str,
+    return_path: &str,
+) -> FutureResult<HttpResponse, actix_web::error::Error> {
+    future::ok(
+        HttpResponse::BadRequest()
+            .content_type("text/html; charset=utf-8")
+            .body(file_upload_error(description, return_path).into_string()),
+    )
+}