Merge pull request #1357 from iannitram/upstream-pr/cli-args-os

CLI: an argument that is not Unicode is a usage error, not a panic

apps/effectcraft-cli/src/main.rs

@@ -263,8 +263,18 @@ fn value_arg(s: &str) -> Value {
     serde_json::from_str(s).unwrap_or_else(|_| json!(s))
 }
 
+/// The command line as text. `std::env::args` panics on an argument that is not Unicode (a
+/// Latin-1 file name on Linux). Every argument is used as a string, and a lossy copy would read or
+/// write another file than the one named, so such an argument is a usage error naming it.
+fn utf8_args(raw: impl IntoIterator<Item = std::ffi::OsString>) -> Result<Vec<String>, String> {
+    raw.into_iter().map(|a| a.into_string().map_err(|a| format!("argument `{}` is not valid Unicode", a.to_string_lossy()))).collect()
+}
+
 fn main() {
-    let raw: Vec<String> = std::env::args().skip(1).collect();
+    let raw = match utf8_args(std::env::args_os().skip(1)) {
+        Ok(raw) => raw,
+        Err(e) => fail_usage(&e),
+    };
     if matches!(raw.first().map(String::as_str), Some("--version" | "-V" | "version")) {
         say!("effectcraft-cli {}", env!("CARGO_PKG_VERSION"));
         return;
@@ -1270,6 +1280,18 @@ mod tests {
         assert!(a.flag("--work-area") && a.flag("--json"));
     }
 
+    /// `std::env::args` panicked on an argument that is not Unicode; it is a usage error naming
+    /// it, and Unicode arguments pass through unchanged.
+    #[cfg(unix)]
+    #[test]
+    fn non_unicode_arguments_are_refused() {
+        use std::ffi::OsString;
+        use std::os::unix::ffi::OsStringExt;
+        let args = [OsString::from("render-frame"), OsString::from("--out"), OsString::from_vec(b"caf\xe9.png".to_vec())];
+        assert_eq!(utf8_args(args).unwrap_err(), "argument `caf\u{fffd}.png` is not valid Unicode");
+        assert_eq!(utf8_args([OsString::from("info"), OsString::from("caf\u{e9}.png")]).unwrap(), ["info", "caf\u{e9}.png"]);
+    }
+
     /// Every `--option` the CLI reads is declared in VALUED or FLAGS, so none is refused.
     #[test]
     fn every_option_read_is_declared() {

apps/effectcraft-cli/tests/cli.rs

@@ -167,6 +167,21 @@ fn unknown_options_are_usage_errors() {
     }
 }
 
+/// An argument that is not Unicode (a Latin-1 file name on Linux) is a usage error (exit 2)
+/// naming it, before anything runs: `std::env::args` panicked on one (exit 101).
+#[cfg(unix)]
+#[test]
+fn non_unicode_arguments_are_usage_errors() {
+    use std::os::unix::ffi::OsStrExt;
+    let name = std::ffi::OsStr::from_bytes(b"caf\xe9.png");
+    let out = bin().args(["render-frame", "--out"]).arg(name).output().unwrap();
+    let err = String::from_utf8_lossy(&out.stderr);
+    assert_eq!(out.status.code(), Some(2), "{err}");
+    assert!(err.contains("argument `caf\u{fffd}.png` is not valid Unicode"), "{err}");
+    assert!(!err.contains("panicked"), "{err}");
+    assert!(out.stdout.is_empty(), "nothing ran");
+}
+
 /// A reader that closes stdout before the CLI writes (`| head`) is not a crash: the command
 /// still does its work and exits 0, without a panic (#167).
 #[test]