[0.31.1] Improve input qualification in FileSpec::try_from (fixes #194).

Author: emabee

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this
 project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.31.1] - 2025-06-22
+
+Improve input qualification in `FileSpec::try_from` (fixes #194).
+
 ## [0.31.0] - 2025-06-16
 
 Add format configuration capabilities to `flexi_logger::trc::setup_tracing`

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "flexi_logger"
-version = "0.31.0"
+version = "0.31.1"
 authors = ["emabee <[email protected]>"]
 categories = ["development-tools::debugging"]
 description = """

src/flexi_error.rs

@@ -26,6 +26,10 @@ pub enum FlexiLoggerError {
     #[error("Log file cannot be written because the specified path is a directory")]
     OutputBadFile,
 
+    /// Invalid input for deriving `FileSpec`.
+    #[error("Invalid input for deriving FileSpec")]
+    BadFileSpec(&'static str),
+
     /// Spawning the cleanup thread failed.
     ///
     /// This error can safely be avoided with `Logger::cleanup_in_background_thread(false)`.

src/parameters/file_spec.rs

@@ -1,10 +1,10 @@
-use crate::writers::file_log_writer::InfixFilter;
-use crate::{DeferredNow, FlexiLoggerError};
+use crate::{writers::file_log_writer::InfixFilter, DeferredNow, FlexiLoggerError};
 use std::{
     ffi::{OsStr, OsString},
     ops::Add,
     path::{Path, PathBuf},
 };
+
 /// Builder object for specifying the name and path of the log output file.
 ///
 /// The filename is built from several partially components, using this pattern:
@@ -91,23 +91,56 @@ impl FileSpec {
     /// # Errors
     ///
     /// [`FlexiLoggerError::OutputBadFile`] if the given path exists and is a folder.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the basename of the given path has no filename
+    /// [`FlexiLoggerError::BadFileSpec`] if deriving the `FileSpec` from the given path fails.
+    #[allow(clippy::missing_panics_doc)]
     pub fn try_from<P: Into<PathBuf>>(p: P) -> Result<Self, FlexiLoggerError> {
-        let p: PathBuf = p.into();
-        if p.is_dir() {
-            Err(FlexiLoggerError::OutputBadFile)
+        let input: PathBuf = p.into();
+
+        if input.is_dir() {
+            Err(FlexiLoggerError::BadFileSpec("File path is a directory"))
         } else {
-            Ok(FileSpec {
-                directory: p.parent().unwrap(/*cannot fail*/).to_path_buf(),
-                basename: p.file_stem().unwrap(/*ok*/).to_string_lossy().to_string(),
-                o_discriminant: None,
-                o_suffix: p.extension().map(|s| s.to_string_lossy().to_string()),
-                timestamp_cfg: TimestampCfg::No,
-                use_utc: false,
-            })
+            let input_as_str = input.as_os_str().to_string_lossy();
+            if input_as_str.is_empty() {
+                Err(FlexiLoggerError::BadFileSpec("File path is empty"))
+            } else if input_as_str.ends_with('/')
+                || input_as_str.ends_with("/.")
+                || input_as_str.ends_with("/..")
+            {
+                Err(FlexiLoggerError::BadFileSpec(
+                    "Path ends with '/' or '/.' or '/..'",
+                ))
+            } else if input
+                .file_name()
+                .ok_or(FlexiLoggerError::OutputBadFile)?
+                .to_string_lossy()
+                .starts_with('.')
+                && input.extension().is_none()
+            {
+                Err(FlexiLoggerError::BadFileSpec(
+                    "File name cannot start with '.' without an extension",
+                ))
+            } else {
+                match input.parent() {
+                    None => Err(FlexiLoggerError::BadFileSpec(
+                        "File path has no parent directory",
+                    )),
+                    Some(parent) => {
+                        let filespec = FileSpec {
+                            directory: if parent.as_os_str().is_empty() {
+                                PathBuf::from(".")
+                            } else {
+                                parent.to_path_buf()
+                            },
+                            basename: input.file_stem().unwrap(/*ok*/).to_string_lossy().to_string(),
+                            o_discriminant: None,
+                            o_suffix: input.extension().map(|s| s.to_string_lossy().to_string()),
+                            timestamp_cfg: TimestampCfg::No,
+                            use_utc: false,
+                        };
+                        Ok(filespec)
+                    }
+                }
+            }
         }
     }
 
@@ -473,6 +506,27 @@ mod test {
         assert_file_spec(&path, &PathBuf::from("."), true, "log");
     }
 
+    #[test]
+    fn issue_194() {
+        assert!(dbg!(FileSpec::try_from("")).is_err());
+        assert!(dbg!(FileSpec::try_from(".")).is_err());
+        assert!(dbg!(FileSpec::try_from("..")).is_err());
+        assert!(dbg!(FileSpec::try_from("/Users")).is_err());
+        assert!(dbg!(FileSpec::try_from("/Users/")).is_err());
+        assert!(dbg!(FileSpec::try_from("./f/")).is_err());
+        assert!(dbg!(FileSpec::try_from("./f/.")).is_err());
+        assert!(dbg!(FileSpec::try_from("./f/..")).is_err());
+        assert!(dbg!(FileSpec::try_from(".log")).is_err());
+        assert!(dbg!(FileSpec::try_from("./.log")).is_err());
+        assert!(dbg!(FileSpec::try_from("./f/.log")).is_err());
+
+        let filespec = FileSpec::try_from("test.log").unwrap();
+        std::fs::create_dir_all(filespec.get_directory()).unwrap();
+        assert!(std::fs::metadata(filespec.get_directory())
+            .unwrap()
+            .is_dir());
+    }
+
     // todo: does not support suppress_timestamp & suppress_basename & use discriminant
     fn assert_file_spec(path: &Path, folder: &Path, with_timestamp: bool, suffix: &str) {
         // check folder

src/writers/file_log_writer.rs

@@ -2,6 +2,7 @@
 mod builder;
 mod config;
 mod infix_filter;
+mod rotation_config;
 mod state;
 mod state_handle;
 mod threads;
@@ -10,7 +11,7 @@ pub use self::builder::{ArcFileLogWriter, FileLogWriterBuilder, FileLogWriterHan
 pub use self::config::FileLogWriterConfig;
 pub(crate) use infix_filter::InfixFilter;
 
-use self::{config::RotationConfig, state::State, state_handle::StateHandle};
+use self::{rotation_config::RotationConfig, state::State, state_handle::StateHandle};
 use crate::{
     writers::LogWriter, DeferredNow, EffectiveWriteMode, FileSpec, FlexiLoggerError,
     FormatFunction, LogfileSelector,

src/writers/file_log_writer/builder.rs

@@ -1,10 +1,11 @@
-use crate::flexi_error::FlexiLoggerError;
-use crate::formats::default_format;
-use crate::{Cleanup, Criterion, FileSpec, FormatFunction, Naming, WriteMode};
-use std::path::{Path, PathBuf};
-use std::sync::Arc;
+use crate::{
+    default_format,
+    writers::{FileLogWriter, FileLogWriterConfig, LogWriter},
+    Cleanup, Criterion, FileSpec, FlexiLoggerError, FormatFunction, Naming, WriteMode,
+};
+use std::{path::PathBuf, sync::Arc};
 
-use super::{FileLogWriter, FileLogWriterConfig, LogWriter, RotationConfig, State};
+use super::{RotationConfig, State};
 
 /// Builder for [`FileLogWriter`].
 #[allow(clippy::struct_excessive_bools, clippy::module_name_repetitions)]
@@ -219,11 +220,10 @@ impl FileLogWriterBuilder {
     }
 
     pub(super) fn try_build_state(&self) -> Result<State, FlexiLoggerError> {
-        // make sure the folder exists or create it
-        let dir = self.file_spec.get_directory();
-        let p_directory = Path::new(&dir);
-        std::fs::create_dir_all(p_directory)?;
-        if !std::fs::metadata(p_directory)?.is_dir() {
+        // make sure the folder exists, or create it
+        let parent = self.file_spec.get_directory();
+        std::fs::create_dir_all(&parent)?;
+        if !std::fs::metadata(&parent)?.is_dir() {
             return Err(FlexiLoggerError::OutputBadDirectory);
         }
 

src/writers/file_log_writer/config.rs

@@ -1,17 +1,6 @@
-use crate::{Cleanup, Criterion, FileSpec, Naming, WriteMode};
+use crate::{FileSpec, WriteMode};
 use std::path::PathBuf;
 
-/// Describes how rotation should work
-#[derive(Clone, Debug)]
-pub struct RotationConfig {
-    // Defines if rotation should be based on size or date
-    pub(crate) criterion: Criterion,
-    // Defines if rotated files should be numbered or get a date-based name
-    pub(crate) naming: Naming,
-    // Defines the cleanup strategy
-    pub(crate) cleanup: Cleanup,
-}
-
 /// Configuration of a `FileLogWriter`.
 #[derive(Debug, Clone)]
 pub struct FileLogWriterConfig {

src/writers/file_log_writer/rotation_config.rs

@@ -0,0 +1,12 @@
+use crate::{Cleanup, Criterion, Naming};
+
+// Describes how rotation should work
+#[derive(Clone, Debug)]
+pub(super) struct RotationConfig {
+    // Defines if rotation should be based on size or date
+    pub(crate) criterion: Criterion,
+    // Defines if rotated files should be numbered or get a date-based name
+    pub(crate) naming: Naming,
+    // Defines the cleanup strategy
+    pub(crate) cleanup: Cleanup,
+}

src/writers/file_log_writer/state.rs

@@ -2,12 +2,9 @@ mod list_and_cleanup;
 mod numbers;
 mod timestamps;
 
+use super::{config::FileLogWriterConfig, rotation_config::RotationConfig, InfixFilter};
 pub(crate) use timestamps::timestamp_from_ts_infix;
 
-use super::{
-    config::{FileLogWriterConfig, RotationConfig},
-    InfixFilter,
-};
 #[cfg(feature = "async")]
 use crate::util::eprint_msg;
 use crate::{