An argument that is not UTF-8 is ignored, not the end of the start-up (#730)

`std::env::args` panics on an argument that is not valid UTF-8, and it panics
while collecting them, before the launcher looks at what it was given. Any
start-up with such an argument ended at

    internal error: panicked at library/std/src/env.rs:878:51:
    called `Result::unwrap()` on an `Err` value: "/tmp/caf\xE9.png"

with exit 101 and the launcher's own panic report, even for `--version`, which
the packaging smoke test runs on the staged binary.

The launcher looks for two flags and ignores everything else a desktop start-up
brings along (`-psn_0_…` from Finder, a file dropped on the icon, an old login
entry's leftovers), so an argument it cannot read is one it would have ignored
anyway: `args_os`, and a warning in the log naming it.

tests/arguments.rs covers it, and `--version` with the arguments a desktop
adds. Without the change the first test fails with exit 101 and that report.

apps/artcraft-launcher/src/main.rs

@@ -46,7 +46,21 @@ fn main() -> ExitCode {
     artcraft_launcher_ui_egui::i18n::detect_system_lang_in_background();
     // Detect Discord protocol handler in the background without blocking startup.
     artcraft_launcher_native::process::ensure_discord_detection_started();
-    let args: Vec<String> = std::env::args().skip(1).collect();
+    // `std::env::args` panics on an argument that is not UTF-8, and the launcher is started with
+    // whatever the desktop hands it: a login entry, a file manager, a terminal in a Latin-1
+    // locale. It only ever looks for the two flags below, so an argument it cannot read is one it
+    // would have ignored anyway — and a start-up that dies with "internal error: panicked at
+    // library/std/src/env.rs" instead is the worst of both.
+    let args: Vec<String> = std::env::args_os()
+        .skip(1)
+        .filter_map(|a| match a.into_string() {
+            Ok(a) => Some(a),
+            Err(bad) => {
+                log::warn!("ignoring an argument that is not valid UTF-8: {}", bad.to_string_lossy());
+                None
+            }
+        })
+        .collect();
     if args.iter().any(|a| a == "--version" || a == "-V") {
         println!("artcraft-launcher {}", env!("CARGO_PKG_VERSION"));
         return ExitCode::SUCCESS;

apps/artcraft-launcher/tests/arguments.rs

@@ -0,0 +1,49 @@
+//! The arguments the desktop hands the launcher.
+//!
+//! It reads two flags — `--version`/`-V` and `--hidden` — and ignores everything else, because a
+//! desktop start-up brings along what it brings along: `-psn_0_…` from Finder when a bundle is
+//! opened, a file dropped on the icon, an old login entry's leftovers. An argument the launcher
+//! cannot read is ignored in the same way, rather than ending the start-up before the window.
+//!
+//! `--version` is the only thing the launcher does without opening a window, and the packaging
+//! scripts use it as their smoke test (`packaging/linux/package.sh` runs it on the staged binary,
+//! under an emulator for a foreign architecture), so it is what a test can ask for from here.
+
+use std::process::Command;
+
+fn launcher() -> Command {
+    Command::new(env!("CARGO_BIN_EXE_artcraft-launcher"))
+}
+
+const VERSION_LINE: &str = concat!("artcraft-launcher ", env!("CARGO_PKG_VERSION"), "\n");
+
+#[test]
+fn the_version_is_printed_with_arguments_the_desktop_adds() {
+    for extra in [vec![], vec!["-psn_0_1234567"], vec!["/tmp/a file.png"]] {
+        for flag in ["--version", "-V"] {
+            let mut args = extra.clone();
+            args.push(flag);
+            let out = launcher().args(&args).output().unwrap_or_else(|e| panic!("{args:?}: {e}"));
+            let err = String::from_utf8_lossy(&out.stderr);
+            assert_eq!(out.status.code(), Some(0), "{args:?}: {err}");
+            assert_eq!(String::from_utf8_lossy(&out.stdout), VERSION_LINE, "{args:?}");
+        }
+    }
+}
+
+/// `std::env::args` panics on an argument that is not UTF-8, which a Latin-1 file name on Linux
+/// or macOS is, and the panic came before the flags were looked at: every start-up with such an
+/// argument ended in `internal error: panicked at library/std/src/env.rs` and exit 101, the
+/// launcher's own panic report (photocraft#1108 is the same bug in that CLI).
+#[cfg(unix)]
+#[test]
+fn an_argument_that_is_not_utf8_is_ignored_like_any_other() {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    let out = launcher().arg(OsStr::from_bytes(b"/tmp/caf\xe9.png")).arg("--version").output().unwrap();
+    let err = String::from_utf8_lossy(&out.stderr);
+    assert_eq!(out.status.code(), Some(0), "an ignored argument, not a panic (101): {err}");
+    assert_eq!(String::from_utf8_lossy(&out.stdout), VERSION_LINE);
+    assert!(!err.contains("panicked"), "{err}");
+}