Merge pull request #264 from J-ZhengLi/patch-no-proxy-env

[patch] fix no_proxy env var could be mistakenly replaced after installation

Author: J-ZhengLi

rim_dev/src/main.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::obfuscated_if_else)]
+
 mod common;
 mod dist;
 mod mocked;

rim_dev/src/mocked/server.rs

@@ -484,7 +484,7 @@ fn calculate_sha256(file_path: &Path) -> Result<String> {
 
     // Finalize and get the checksum as a hex string
     let result = hasher.finalize();
-    Ok(format!("{:x}", result))
+    Ok(format!("{result:x}"))
 }
 
 pub(crate) fn generate_rim_server_files() -> Result<()> {

rim_test/rim-test-macro/src/lib.rs

@@ -10,7 +10,7 @@ pub fn rim_test(attr: TokenStream, item: TokenStream) -> TokenStream {
             "ignore" => {
                 ignore = true;
             }
-            _ => panic!("unknown rule: {:?}", rule),
+            _ => panic!("unknown rule: {rule:?}"),
         }
     }
 

rim_test/rim-test-support/src/lib.rs

@@ -30,11 +30,11 @@ pub fn panic_error(what: &str, err: impl Into<anyhow::Error>) -> ! {
     pe(what, err);
     #[track_caller]
     fn pe(what: &str, err: anyhow::Error) -> ! {
-        let mut result = format!("{}\nerror: {}", what, err);
+        let mut result = format!("{what}\nerror: {err}");
         for cause in err.chain().skip(1) {
             let _ = writeln!(result, "\nCaused by:");
-            let _ = write!(result, "{}", cause);
+            let _ = write!(result, "{cause}");
         }
-        panic!("\n{}", result);
+        panic!("\n{result}");
     }
 }

rim_test/rim-test-support/src/paths.rs

@@ -35,7 +35,7 @@ pub fn test_root() -> TempDir {
     let id = TEST_ID.with(|n| n.borrow().expect("Failed to get test thread id"));
 
     let test_root_dir = global_root_dir();
-    let prefix = format!("t{}", id);
+    let prefix = format!("t{id}");
     TempDir::with_prefix_in(prefix, test_root_dir).expect("Failed to create temp test dir")
 }
 

src/cli/common.rs

@@ -123,7 +123,7 @@ where
     }
     // print extra info, such as a list of selectable options.
     if let Some(ex) = extra {
-        writeln!(&mut stdout, "\n{}", ex)?;
+        writeln!(&mut stdout, "\n{ex}")?;
     }
 
     loop {

src/core/install.rs

@@ -184,7 +184,18 @@ impl<'a> InstallConfiguration<'a> {
                 env_vars.insert("https_proxy", url.to_string());
             }
             if let Some(s) = &proxy.no_proxy {
-                env_vars.insert("no_proxy", s.to_string());
+                // keep use's original no_proxy var.
+                #[cfg(windows)]
+                let prev_np = std::env::var("no_proxy").unwrap_or_default();
+                #[cfg(unix)]
+                let prev_np = "$no_proxy";
+
+                let no_proxy = if prev_np.is_empty() {
+                    s.to_string()
+                } else {
+                    format!("{s},{prev_np}")
+                };
+                env_vars.insert("no_proxy", no_proxy);
             }
         }
 
@@ -594,4 +605,56 @@ c = { version = "0.1.0", conflicts = ["d", "a"] }
         let error = conflicts.expect_err("has conflicts");
         println!("{error}");
     }
+
+    #[test]
+    fn no_proxy_env_var() {
+        let raw = r#"
+[rust]
+version = "1.0.0"
+
+[proxy]
+no_proxy = "localhost,.example.com,.foo.com"
+"#;
+
+        let manifest = ToolkitManifest::from_str(raw).unwrap();
+        let install_dir = tempfile::tempdir().unwrap();
+        let install_cfg = InstallConfiguration::new(install_dir.path(), &manifest).unwrap();
+
+        // Temporarily modify no_proxy var to test inheritance.
+        // FIXME (master): Later commits introduces mocked env tests, make sure to tesk this
+        // with mocked env, that we don't need to set no_proxy var here and potentially
+        // mess up other concurrent test cases.
+        let no_proxy_backup = std::env::var("no_proxy");
+        std::env::remove_var("no_proxy");
+
+        let env_vars = install_cfg.env_vars().unwrap();
+        let new_no_proxy_var = env_vars.get("no_proxy").unwrap();
+
+        #[cfg(windows)]
+        assert_eq!(new_no_proxy_var, "localhost,.example.com,.foo.com");
+        #[cfg(unix)]
+        assert_eq!(
+            new_no_proxy_var,
+            "localhost,.example.com,.foo.com,$no_proxy"
+        );
+
+        std::env::set_var("no_proxy", ".bar.com,baz.com");
+        let env_vars = install_cfg.env_vars().unwrap();
+        let new_no_proxy_var = env_vars.get("no_proxy").unwrap();
+
+        #[cfg(windows)]
+        assert_eq!(
+            new_no_proxy_var,
+            "localhost,.example.com,.foo.com,.bar.com,baz.com"
+        );
+        #[cfg(unix)]
+        assert_eq!(
+            new_no_proxy_var,
+            "localhost,.example.com,.foo.com,$no_proxy"
+        );
+
+        if let Ok(bck) = no_proxy_backup {
+            std::env::set_var("no_proxy", bck);
+        }
+    }
 }