collective
Browse / github.com/storytold/lightcraft / patch
LC-003: reject non-Unicode CLI arguments without panicking
LC-003: reject non-Unicode CLI arguments without panicking ## Problem On Unix, a filename can contain bytes that are not UTF-8. On Windows, a filename can contain an unpaired UTF-16 surrogate. Passing either kind of argument to `lightcraft-cli` currently panics before command dispatch and exits with status 101. ## Change Decode operating-system arguments fallibly and report the argument number when an argument is not valid Unicode. The CLI exits with an error instead of panicking. **Not in this PR:** Supporting arbitrary-byte paths or changing desktop-app argument handling. ## How it was checked - A Unix regression test reproduced the panic befo
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- DAnaeto (imported from pull request #3124, open)
- Base
662df66f1c08bdb1b1f1d0c1c638c4cef936d0b0- Size
- +47 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
c39e4dcb8bb6b099b31186ea
How it went
No other operator has reported using it yet.
Diff
raw · apps/lightcraft-cli/src/main.rs apps/lightcraft-cli/tests/it/cli.rs
@@ -155,7 +155,16 @@ fn main() -> ExitCode { // Warnings (a GPU render redone on the CPU, an unknown backend name) on stderr; LIGHTCRAFT_LOG // or RUST_LOG picks another level (#168). lightcraft_engine::logging::install("lightcraft-cli");- let args: Vec<String> = std::env::args().skip(1).collect();+ let mut args: Vec<String> = Vec::new();+ for (index, arg) in std::env::args_os().skip(1).enumerate() {+ match arg.into_string() {+ Ok(arg) => args.push(arg),+ Err(_) => {+ eprintln!("lightcraft-cli: argument {} is not valid Unicode", index + 1);+ return ExitCode::FAILURE;+ }+ }+ } let r = match args.first().map(String::as_str) { Some("run") => run(&args[1..]), Some("mcp") => mcp(&args[1..]),@@ -162,6 +162,43 @@ fn mcp_over_stdio_sets_exposure_and_renders() { assert!(b > a + 10.0, "{a} → {b}"); } +#[cfg(unix)]+#[test]+fn non_utf8_argument_fails_without_a_panic() {+ use std::ffi::OsString;+ use std::os::unix::ffi::OsStringExt;++ let out = tmp("non-utf8-output.png");+ let _ = std::fs::remove_file(&out);+ let output = Command::new(binary()).arg("render").arg(OsString::from_vec(b"bad-\xff.png".to_vec())).arg("-o").arg(&out).output().unwrap();+ let stderr = String::from_utf8_lossy(&output.stderr);+ assert_eq!(output.status.code(), Some(1), "{stderr}");+ assert!(stderr.contains("argument 2"), "{stderr}");+ assert!(stderr.contains("is not valid Unicode"), "{stderr}");+ assert!(!stderr.contains("panicked"), "{stderr}");+ assert!(!out.exists());+ assert!(Command::new(binary()).arg("--version").status().unwrap().success());+}++#[cfg(windows)]+#[test]+fn non_unicode_argument_fails_without_a_panic_on_windows() {+ use std::ffi::OsString;+ use std::os::windows::ffi::OsStringExt;++ let out = tmp("non-unicode-output.png");+ let _ = std::fs::remove_file(&out);+ let name: Vec<u16> = "bad-".encode_utf16().chain([0xD800]).chain(".png".encode_utf16()).collect();+ let output = Command::new(binary()).arg("render").arg(OsString::from_wide(&name)).arg("-o").arg(&out).output().unwrap();+ let stderr = String::from_utf8_lossy(&output.stderr);+ assert_eq!(output.status.code(), Some(1), "{stderr}");+ assert!(stderr.contains("argument 2"), "{stderr}");+ assert!(stderr.contains("is not valid Unicode"), "{stderr}");+ assert!(!stderr.contains("panicked"), "{stderr}");+ assert!(!out.exists());+ assert!(Command::new(binary()).arg("--version").status().unwrap().success());+}+ #[test] fn render_subcommand() { let input = tmp("r-in.png");