Merge pull request #616 from SteveL-MSFT/path-windows

Ensure `systemRoot()` always has trailing separator character

Author: SteveL-MSFT

dsc/src/subcommand.rs

@@ -341,7 +341,12 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, mounte
             exit(EXIT_INVALID_ARGS);
         }
 
-        configurator.set_system_root(path);
+        // make sure path has a trailing separator if it's a drive letter
+        if path.len() == 2 && path.chars().nth(1).unwrap_or(' ') == ':' {
+            configurator.set_system_root(&format!("{path}\\"));
+        } else {
+            configurator.set_system_root(path);
+        }
     }
 
     if let Err(err) = configurator.set_context(parameters.as_ref()) {

dsc/tests/dsc_functions.tests.ps1

@@ -54,7 +54,7 @@ Describe 'tests for function expressions' {
 '@
 
         $expected = if ($IsWindows) {
-            $env:SYSTEMDRIVE
+            $env:SYSTEMDRIVE + '\'
         } else {
             '/'
         }

dsc_lib/src/configure/context.rs

@@ -45,9 +45,9 @@ impl Default for Context {
 
 #[cfg(target_os = "windows")]
 fn get_default_os_system_root() -> PathBuf {
-    // use SYSTEMDRIVE env var to get the default target path
+    // use SYSTEMDRIVE env var to get the default target path, append trailing separator
     let system_drive = std::env::var("SYSTEMDRIVE").unwrap_or_else(|_| "C:".to_string());
-    PathBuf::from(system_drive)
+    PathBuf::from(system_drive + "\\")
 }
 
 #[cfg(not(target_os = "windows"))]

dsc_lib/src/functions/path.rs

@@ -48,19 +48,80 @@ mod tests {
     use crate::configure::context::Context;
     use crate::parser::Statement;
 
+    const SEPARATOR: char = std::path::MAIN_SEPARATOR;
+
+    #[test]
+    fn start_with_drive_letter() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute("[path('C:\\','test')]", &Context::new()).unwrap();
+
+        #[cfg(target_os = "windows")]
+        assert_eq!(result, format!("C:{SEPARATOR}test"));
+
+        #[cfg(not(target_os = "windows"))]
+        assert_eq!(result, format!("C:\\{SEPARATOR}test"));
+    }
+
+    #[test]
+    fn drive_letter_in_middle() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute("[path('a','C:\\','test')]", &Context::new()).unwrap();
+
+        // if any part of the path is absolute, it replaces it instead of appending on Windows
+        #[cfg(target_os = "windows")]
+        assert_eq!(result, format!("C:{SEPARATOR}test"));
+
+        // non-Windows, the colon is a valid character in a path
+        #[cfg(not(target_os = "windows"))]
+        assert_eq!(result, format!("a{SEPARATOR}C:\\{SEPARATOR}test"));
+    }
+
+    #[test]
+    fn multiple_drive_letters() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute("[path('C:\\','D:\\','test')]", &Context::new()).unwrap();
+
+        // if any part of the path is absolute, it replaces it instead of appending on Windows
+        #[cfg(target_os = "windows")]
+        assert_eq!(result, format!("D:\\test"));
+
+        // non-Windows, the colon is a valid character in a path
+        #[cfg(not(target_os = "windows"))]
+        assert_eq!(result, format!("C:\\{SEPARATOR}D:\\{SEPARATOR}test"));
+    }
+
+    #[test]
+    fn relative_path() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute("[path('a','..','b')]", &Context::new()).unwrap();
+        assert_eq!(result, format!("a{SEPARATOR}..{SEPARATOR}b"));
+    }
+
+    #[test]
+    fn path_segement_with_separator() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute(format!("[path('a','b{SEPARATOR}c')]").as_str(), &Context::new()).unwrap();
+        assert_eq!(result, format!("a{SEPARATOR}b{SEPARATOR}c"));
+    }
+
+    #[test]
+    fn unix_absolute_path() {
+        let mut parser = Statement::new().unwrap();
+        let result = parser.parse_and_execute("[path('/','a','b')]", &Context::new()).unwrap();
+        assert_eq!(result, format!("/a{SEPARATOR}b"));
+    }
+
     #[test]
     fn two_args() {
         let mut parser = Statement::new().unwrap();
-        let separator = std::path::MAIN_SEPARATOR;
         let result = parser.parse_and_execute("[path('a','b')]", &Context::new()).unwrap();
-        assert_eq!(result, format!("a{separator}b"));
+        assert_eq!(result, format!("a{SEPARATOR}b"));
     }
 
     #[test]
     fn three_args() {
         let mut parser = Statement::new().unwrap();
-        let separator = std::path::MAIN_SEPARATOR;
         let result = parser.parse_and_execute("[path('a','b','c')]", &Context::new()).unwrap();
-        assert_eq!(result, format!("a{separator}b{separator}c"));
+        assert_eq!(result, format!("a{SEPARATOR}b{SEPARATOR}c"));
     }
 }

dsc_lib/src/functions/system_root.rs

@@ -44,7 +44,7 @@ mod tests {
         let result = parser.parse_and_execute("[systemRoot()]", &Context::new()).unwrap();
         // on windows, the default is SYSTEMDRIVE env var
         #[cfg(target_os = "windows")]
-        assert_eq!(result, std::env::var("SYSTEMDRIVE").unwrap());
+        assert_eq!(result, format!("{}\\", std::env::var("SYSTEMDRIVE").unwrap()));
         // on linux/macOS, the default is /
         #[cfg(not(target_os = "windows"))]
         assert_eq!(result, "/");