diff --git a/README.md b/README.md index bff3de703..0aafaf0b4 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ If a header is already set or previously inserted, it will not be overwritten. # Fullchain TLS and HTTP Strict Transport Security (HSTS) miniserve --tls-cert fullchain.pem --tls-key my.key --header "Strict-Transport-Security: max-age=31536000; includeSubDomains; preload" /tmp/myshare -If the parameter value has spaces, be sure to wrap it in quotes. +If the parameter value has spaces, be sure to wrap it in quotes. (To achieve an A+ rating at https://www.ssllabs.com/ssltest/, enabling both fullchain TLS and HSTS is necessary.) ### Upload a file using `curl`: @@ -289,6 +289,14 @@ Options: [env: OVERWRITE_FILES=] + -R, --rename-duplicate + Enable renaming files during file upload if duplicate exists + + The renaming will occur by adding numerical suffix to the filename before the final extension. For example file.txt will be uploaded as file-1.txt, the number will + be increased untill a available slot is found. + + [env: RENAME_DUPLICATE_FILES=] + -r, --enable-tar Enable uncompressed tar archive generation @@ -497,7 +505,7 @@ You can provide `-i` multiple times to bind to multiple interfaces at the same t This is mostly a note for me on how to release this thing: -- Make sure `CHANGELOG.md` is up to date. +- Make sure [CHANGELOG.md](./CHANGELOG.md) is up to date. - `cargo release ` - `cargo release --execute ` - Releases will automatically be deployed by GitHub Actions. diff --git a/src/args.rs b/src/args.rs index dd09f65e4..1dbba6d83 100644 --- a/src/args.rs +++ b/src/args.rs @@ -15,6 +15,14 @@ pub enum MediaType { Video, } +#[derive(ValueEnum, Clone, Default, Copy)] +pub enum DuplicateFile { + #[default] + Error, + Overwrite, + Rename, +} + #[derive(Parser)] #[command(name = "miniserve", author, about, version)] pub struct CliArgs { @@ -194,9 +202,20 @@ pub struct CliArgs { )] pub media_type_raw: Option, - /// Enable overriding existing files during file upload - #[arg(short = 'o', long = "overwrite-files", env = "OVERWRITE_FILES")] - pub overwrite_files: bool, + /// What to do if existing files with same name is present during file upload + /// + /// If you enable renaming files, the renaming will occur by + /// adding numerical suffix to the filename before the final + /// extension. For example file.txt will be uploaded as + /// file-1.txt, the number will be increased untill a available + /// slot is found. + #[arg( + short = 'o', + long = "on-duplicate-files", + env = "ON_DUPLICATE_FILES", + default_value = "error" + )] + pub on_duplicate_files: DuplicateFile, /// Enable uncompressed tar archive generation #[arg(short = 'r', long = "enable-tar", env = "MINISERVE_ENABLE_TAR")] diff --git a/src/config.rs b/src/config.rs index f468365ad..6e2a10076 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,7 +12,7 @@ use anyhow::{anyhow, Context, Result}; use rustls_pemfile as pemfile; use crate::{ - args::{parse_auth, CliArgs, MediaType}, + args::{parse_auth, CliArgs, DuplicateFile, MediaType}, auth::RequiredAuth, file_utils::sanitize_path, listing::{SortingMethod, SortingOrder}, @@ -107,8 +107,8 @@ pub struct MiniserveConfig { /// HTML accept attribute value pub uploadable_media_type: Option, - /// Enable upload to override existing files - pub overwrite_files: bool, + /// What to do on upload if filename already exists + pub on_duplicate_files: DuplicateFile, /// If false, creation of uncompressed tar archives is disabled pub tar_enabled: bool, @@ -288,7 +288,7 @@ impl MiniserveConfig { index: args.index, spa: args.spa, pretty_urls: args.pretty_urls, - overwrite_files: args.overwrite_files, + on_duplicate_files: args.on_duplicate_files, show_qrcode: args.qrcode, mkdir_enabled: args.mkdir_enabled, file_upload: args.allowed_upload_dir.is_some(), diff --git a/src/errors.rs b/src/errors.rs index 21f8f12be..88ae0fa33 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -37,7 +37,7 @@ pub enum RuntimeError { MultipartError(String), /// Might occur during file upload - #[error("File already exists, and the overwrite_files option has not been set")] + #[error("File already exists, and the on_duplicate_files option is set to error out")] DuplicateFileError, /// Upload not allowed diff --git a/src/file_op.rs b/src/file_op.rs index 18bdcbe21..bb7644e02 100644 --- a/src/file_op.rs +++ b/src/file_op.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use tokio::fs::File; use tokio::io::AsyncWriteExt; +use crate::args::DuplicateFile; use crate::{ config::MiniserveConfig, errors::RuntimeError, file_utils::contains_symlink, file_utils::sanitize_path, @@ -21,11 +22,33 @@ use crate::{ /// Returns total bytes written to file. async fn save_file( field: actix_multipart::Field, - file_path: PathBuf, - overwrite_files: bool, + mut file_path: PathBuf, + on_duplicate_files: DuplicateFile, ) -> Result { - if !overwrite_files && file_path.exists() { - return Err(RuntimeError::DuplicateFileError); + if file_path.exists() { + match on_duplicate_files { + DuplicateFile::Error => return Err(RuntimeError::DuplicateFileError), + DuplicateFile::Overwrite => (), + DuplicateFile::Rename => { + // extract extension of the file and the file stem without extension + // file.txt => (file, txt) + let file_name = file_path.file_stem().unwrap_or_default().to_string_lossy(); + let file_ext = file_path.extension().map(|s| s.to_string_lossy()); + for i in 1.. { + // increment the number N in {file_name}-{N}.{file_ext} + // format until available name is found (e.g. file-1.txt, file-2.txt, etc) + let fp = if let Some(ext) = &file_ext { + file_path.with_file_name(format!("{}-{}.{}", file_name, i, ext)) + } else { + file_path.with_file_name(format!("{}-{}", file_name, i)) + }; + if !fp.exists() { + file_path = fp; + break; + } + } + } + } } let file = match File::create(&file_path).await { @@ -56,7 +79,7 @@ async fn save_file( async fn handle_multipart( mut field: actix_multipart::Field, path: PathBuf, - overwrite_files: bool, + on_duplicate_files: DuplicateFile, allow_mkdir: bool, allow_hidden_paths: bool, allow_symlinks: bool, @@ -168,7 +191,7 @@ async fn handle_multipart( } } - save_file(field, path.join(filename_path), overwrite_files).await + save_file(field, path.join(filename_path), on_duplicate_files).await } /// Query parameters used by upload and rm APIs @@ -224,7 +247,7 @@ pub async fn upload_file( handle_multipart( field, non_canonicalized_target_dir.clone(), - conf.overwrite_files, + conf.on_duplicate_files, conf.mkdir_enabled, conf.show_hidden, !conf.no_symlinks, diff --git a/tests/upload_files.rs b/tests/upload_files.rs index 77a9dc319..65787f077 100644 --- a/tests/upload_files.rs +++ b/tests/upload_files.rs @@ -7,7 +7,10 @@ use rstest::rstest; use select::document::Document; use select::predicate::{Attr, Text}; use std::fs::create_dir_all; +use std::fs::File; +use std::io::Write; use std::path::Path; +use std::path::PathBuf; #[rstest] fn uploading_files_works(#[with(&["-u"])] server: TestServer) -> Result<(), Error> { @@ -275,3 +278,177 @@ fn set_media_type( Ok(()) } + +#[rstest] +#[case(server(&["-u"]))] +#[case(server(&["-u", "-o", "error"]))] +#[case(server(&["-u", "--on-duplicate-files", "error"]))] +fn uploading_duplicate_file_is_prevented(#[case] server: TestServer) -> Result<(), Error> { + let test_file_name = "duplicate test file.txt"; + let test_file_contents = "Test File Contents"; + + // create the file + let test_file_path = write_file_contents( + server.path().to_path_buf(), + test_file_name, + test_file_contents, + ); + + // Before uploading, make sure the file is there. + let body = reqwest::blocking::get(server.url())?.error_for_status()?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + + // Perform the actual upload. + let upload_action = parsed + .find(Attr("id", "file_submit")) + .next() + .expect("Couldn't find element with id=file_submit") + .attr("action") + .expect("Upload form doesn't have action attribute"); + // Then try to upload anyway + let form = multipart::Form::new(); + let part = multipart::Part::text("this should not be uploaded") + .file_name(test_file_name) + .mime_str("text/plain")?; + let form = form.part("file_to_upload", part); + + let client = Client::new(); + // Ensure uploading fails and returns an error + assert!(client + .post(server.url().join(upload_action)?) + .multipart(form) + .send()? + .error_for_status() + .is_err()); + + // After uploading, uploaded file is still getting listed. + let body = reqwest::blocking::get(server.url())?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + // and assert the contents is the same as before + assert_file_contents(test_file_path, test_file_contents); + + Ok(()) +} + +#[rstest] +fn overwrite_duplicate_file( + #[with(&["-u", "--on-duplicate-files", "overwrite"])] server: TestServer, +) -> Result<(), Error> { + let test_file_name = "duplicate test file.txt"; + let test_file_contents = "Test File Contents"; + let test_file_contents_new = "New Uploaded Test File Contents"; + + // create the file + let test_file_path = write_file_contents( + server.path().to_path_buf(), + test_file_name, + test_file_contents, + ); + + // Before uploading, make sure the file is there. + let body = reqwest::blocking::get(server.url())?.error_for_status()?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + + // Perform the actual upload. + let upload_action = parsed + .find(Attr("id", "file_submit")) + .next() + .expect("Couldn't find element with id=file_submit") + .attr("action") + .expect("Upload form doesn't have action attribute"); + // Then try to upload anyway + let form = multipart::Form::new(); + let part = multipart::Part::text(test_file_contents_new) + .file_name(test_file_name) + .mime_str("text/plain")?; + let form = form.part("file_to_upload", part); + + let client = Client::new(); + client + .post(server.url().join(upload_action)?) + .multipart(form) + .send()? + .error_for_status()?; + + // After uploading, uploaded file is still getting listed. + let body = reqwest::blocking::get(server.url())?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + // and assert the contents is different one from before + assert_file_contents(test_file_path, test_file_contents_new); + + Ok(()) +} + +#[rstest] +fn rename_duplicate_file( + #[with(&["-u", "--on-duplicate-files", "rename"])] server: TestServer, +) -> Result<(), Error> { + let test_file_name = "duplicate test file.txt"; + let test_file_name_new = "duplicate test file-1.txt"; + let test_file_contents = "Test File Contents"; + let test_file_contents_new = "New Uploaded Test File Contents"; + + // create the file + let test_file_path = write_file_contents( + server.path().to_path_buf(), + test_file_name, + test_file_contents, + ); + + // Before uploading, make sure the file is there. + let body = reqwest::blocking::get(server.url())?.error_for_status()?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + + // Perform the actual upload. + let upload_action = parsed + .find(Attr("id", "file_submit")) + .next() + .expect("Couldn't find element with id=file_submit") + .attr("action") + .expect("Upload form doesn't have action attribute"); + // Then try to upload anyway + let form = multipart::Form::new(); + let part = multipart::Part::text(test_file_contents_new) + .file_name(test_file_name) + .mime_str("text/plain")?; + let form = form.part("file_to_upload", part); + + let client = Client::new(); + client + .post(server.url().join(upload_action)?) + .multipart(form) + .send()? + .error_for_status()?; + + // After uploading, make sure old and new files are both listed. + let body = reqwest::blocking::get(server.url())?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + assert!(parsed.find(Text).any(|x| x.text() == test_file_name_new)); + // and assert the contents is same for old and new files + assert_file_contents(server.path().join(&test_file_path), test_file_contents); + assert_file_contents( + server.path().join(&test_file_name_new), + test_file_contents_new, + ); + + Ok(()) +} + +fn write_file_contents(path: PathBuf, filename: &str, contents: &str) -> PathBuf { + let file_path = path.join(filename); + let mut file = File::create(&file_path).unwrap(); + file.write_all(contents.as_bytes()) + .expect("Couldn't write file"); + file_path +} + +fn assert_file_contents(file_path: PathBuf, contents: &str) { + let file_contents = std::fs::read_to_string(&file_path).unwrap(); + assert!(file_contents == contents) +}