new flag: Enable renaming during file upload if duplicate exists

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 <version>`
 - `cargo release --execute <version>`
 - Releases will automatically be deployed by GitHub Actions.

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<String>,
 
-    /// 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")]

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<String>,
 
-    /// 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(),

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

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<u64, RuntimeError> {
-    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,

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)
+}