diff --git a/.changes/fix-nsis-updater-args.md b/.changes/fix-nsis-updater-args.md new file mode 100644 index 00000000..b2483090 --- /dev/null +++ b/.changes/fix-nsis-updater-args.md @@ -0,0 +1,6 @@ +--- +updater: patch +updater-js: patch +--- + +Fixed an issue preventing updates via the NSIS installer from succeeding when the app was launched with command line arguments containing spaces. diff --git a/plugins/updater/src/updater.rs b/plugins/updater/src/updater.rs index 78fc0a9b..707c1489 100644 --- a/plugins/updater/src/updater.rs +++ b/plugins/updater/src/updater.rs @@ -683,17 +683,25 @@ impl Update { let install_mode = self.config.install_mode(); let current_args = &self.current_exe_args()[1..]; let msi_args; + let nsis_args; let installer_args: Vec<&OsStr> = match &updater_type { - WindowsUpdaterType::Nsis { .. } => install_mode - .nsis_args() - .iter() - .map(OsStr::new) - .chain(once(OsStr::new("/UPDATE"))) - .chain(once(OsStr::new("/ARGS"))) - .chain(current_args.to_vec()) - .chain(self.installer_args()) - .collect(), + WindowsUpdaterType::Nsis { .. } => { + nsis_args = current_args + .iter() + .map(escape_nsis_current_exe_arg) + .collect::>(); + + install_mode + .nsis_args() + .iter() + .map(OsStr::new) + .chain(once(OsStr::new("/UPDATE"))) + .chain(once(OsStr::new("/ARGS"))) + .chain(nsis_args.iter().map(OsStr::new)) + .chain(self.installer_args()) + .collect() + } WindowsUpdaterType::Msi { path, .. } => { let escaped_args = current_args .iter() @@ -1363,6 +1371,41 @@ impl PathExt for PathBuf { } } +// adapted from https://github.com/rust-lang/rust/blob/1c047506f94cd2d05228eb992b0a6bbed1942349/library/std/src/sys/args/windows.rs#L174 +#[cfg(windows)] +fn escape_nsis_current_exe_arg(arg: &&OsStr) -> String { + let arg = arg.to_string_lossy(); + let mut cmd: Vec = Vec::new(); + + // compared to std we additionally escape `/` so that nsis won't interpret them as a beginning of an nsis argument. + let quote = arg.chars().any(|c| c == ' ' || c == '\t' || c == '/') || arg.is_empty(); + let escape = true; + if quote { + cmd.push('"'); + } + let mut backslashes: usize = 0; + for x in arg.chars() { + if escape { + if x == '\\' { + backslashes += 1; + } else { + if x == '"' { + // Add n+1 backslashes to total 2n+1 before internal '"'. + cmd.extend((0..=backslashes).map(|_| '\\')); + } + backslashes = 0; + } + } + cmd.push(x); + } + if quote { + // Add n backslashes to total 2n before ending '"'. + cmd.extend((0..backslashes).map(|_| '\\')); + cmd.push('"'); + } + cmd.into_iter().collect() +} + #[cfg(windows)] fn escape_msi_property_arg(arg: impl AsRef) -> String { let mut arg = arg.as_ref().to_string_lossy().to_string(); @@ -1375,7 +1418,7 @@ fn escape_msi_property_arg(arg: impl AsRef) -> String { } if arg.contains('"') { - arg = arg.replace('"', r#""""""#) + arg = arg.replace('"', r#""""""#); } if arg.starts_with('-') { @@ -1406,7 +1449,7 @@ mod tests { #[test] #[cfg(windows)] - fn it_escapes_correctly() { + fn it_escapes_correctly_for_msi() { use crate::updater::escape_msi_property_arg; // Explanation for quotes: @@ -1451,4 +1494,47 @@ mod tests { assert_eq!(escape_msi_property_arg(orig), escaped); } } + + #[test] + #[cfg(windows)] + fn it_escapes_correctly_for_nsis() { + use crate::updater::escape_nsis_current_exe_arg; + use std::ffi::OsStr; + + let cases = [ + "something", + "--flag", + "--empty=", + "--arg=value", + "some space", // This simulates `./my-app "some string"`. + "--arg value", // -> This simulates `./my-app "--arg value"`. Same as above but it triggers the startsWith(`-`) logic. + "--arg=unwrapped space", // `./my-app --arg="unwrapped space"` + "--arg=\"wrapped\"", // `./my-app --args=""wrapped""` + "--arg=\"wrapped space\"", // `./my-app --args=""wrapped space""` + "--arg=midword\"wrapped space\"", // `./my-app --args=midword""wrapped""` + "", // `./my-app '""'` + ]; + // Note: These may not be the results we actually want (monitor this!). + // We only make sure the implementation doesn't unintentionally change. + let cases_escaped = [ + "something", + "--flag", + "--empty=", + "--arg=value", + "\"some space\"", + "\"--arg value\"", + "\"--arg=unwrapped space\"", + "--arg=\\\"wrapped\\\"", + "\"--arg=\\\"wrapped space\\\"\"", + "\"--arg=midword\\\"wrapped space\\\"\"", + "\"\"", + ]; + + // Just to be sure we didn't mess that up + assert_eq!(cases.len(), cases_escaped.len()); + + for (orig, escaped) in cases.iter().zip(cases_escaped) { + assert_eq!(escape_nsis_current_exe_arg(&OsStr::new(orig)), escaped); + } + } }