From 3c3ae3ff9838cd7cd56b19b4c358696db5069352 Mon Sep 17 00:00:00 2001
From: Lawrence Chou <choznerol@protonmail.com>
Date: Wed, 12 Oct 2022 11:58:28 +0800
Subject: [PATCH] Impeove invalid config_file_path handling

1. Besides opt.config_file_path, also consider MEILI_CONFIG_FILE_PATH in the Err path because they are both user input.
2. Print out the incorrect file path in error message.
3. Add tests
https://github.com/meilisearch/meilisearch/pull/2804#discussion_r991999888
---
 Cargo.lock                     | 10 ++++++++
 meilisearch-http/Cargo.toml    |  1 +
 meilisearch-http/src/option.rs | 46 ++++++++++++++++++++++++++++------
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b26636a01..5e32e9afa 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2095,6 +2095,7 @@ dependencies = [
  "static-files",
  "sysinfo",
  "tar",
+ "temp-env",
  "tempfile",
  "thiserror",
  "time",
@@ -3451,6 +3452,15 @@ dependencies = [
  "xattr",
 ]
 
+[[package]]
+name = "temp-env"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a30d48359f77fbb6af3d7b928cc2d092e1dc90b44f397e979ef08ae15733ed65"
+dependencies = [
+ "once_cell",
+]
+
 [[package]]
 name = "tempfile"
 version = "3.3.0"
diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml
index cabb6ed48..11d142eb6 100644
--- a/meilisearch-http/Cargo.toml
+++ b/meilisearch-http/Cargo.toml
@@ -89,6 +89,7 @@ manifest-dir-macros = "0.1.16"
 maplit = "1.0.2"
 urlencoding = "2.1.2"
 yaup = "0.2.1"
+temp-env = "0.3.1"
 
 [features]
 default = ["analytics", "meilisearch-lib/default", "mini-dashboard"]
diff --git a/meilisearch-http/src/option.rs b/meilisearch-http/src/option.rs
index fcd0f4529..ddae59c5a 100644
--- a/meilisearch-http/src/option.rs
+++ b/meilisearch-http/src/option.rs
@@ -263,10 +263,12 @@ impl Opt {
         // Parse the args to get the config_file_path.
         let mut opts = Opt::parse();
         let mut config_read_from = None;
-        let config_file_path = opts
+        let user_specified_config_file_path = opts
             .config_file_path
             .clone()
-            .or_else(|| env::var("MEILI_CONFIG_FILE_PATH").map(PathBuf::from).ok())
+            .or_else(|| env::var("MEILI_CONFIG_FILE_PATH").map(PathBuf::from).ok());
+        let config_file_path = user_specified_config_file_path
+            .clone()
             .unwrap_or_else(|| PathBuf::from(DEFAULT_CONFIG_FILE_PATH));
 
         match std::fs::read(&config_file_path) {
@@ -284,12 +286,17 @@ impl Opt {
                 opts = Opt::parse();
                 config_read_from = Some(config_file_path);
             }
-            // If we have an error while reading the file defined by the user.
-            Err(_) if opts.config_file_path.is_some() => anyhow::bail!(
-                "unable to open or read the {:?} configuration file.",
-                opts.config_file_path.unwrap().display().to_string()
-            ),
-            _ => (),
+            Err(e) => {
+                match user_specified_config_file_path {
+                    // If we have an error while reading the file defined by the user.
+                    Some(path) => anyhow::bail!(
+                        "unable to open or read the {:?} configuration file: {}.",
+                        path,
+                        e,
+                    ),
+                    None => (),
+                }
+            }
         }
 
         Ok((opts, config_read_from))
@@ -529,4 +536,27 @@ mod test {
     fn test_valid_opt() {
         assert!(Opt::try_parse_from(Some("")).is_ok());
     }
+
+    #[test]
+    fn test_meilli_config_file_path_valid() {
+        temp_env::with_vars(
+            vec![("MEILI_CONFIG_FILE_PATH", Some("../config.toml"))], // Relative path in meilisearch_http package
+            || {
+                assert!(Opt::try_build().is_ok());
+            },
+        );
+    }
+
+    #[test]
+    fn test_meilli_config_file_path_invalid() {
+        temp_env::with_vars(
+            vec![("MEILI_CONFIG_FILE_PATH", Some("../configgg.toml"))],
+            || {
+                assert_eq!(
+                    Opt::try_build().unwrap_err().to_string(),
+                    "unable to open or read the \"../configgg.toml\" configuration file: No such file or directory (os error 2)."
+                );
+            },
+        );
+    }
 }