From dd5806d146eaffec7e273e2b631b67a90f556203 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 2 Mar 2021 10:41:57 +0300 Subject: [PATCH] Don't panic when printing JSON with non-utf8 paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: λ cd \Xff/foo/ && cargo verify-project && cargo metadata {"success":"true"} warning: please specify `--format-version` flag explicitly to avoid compatibility problems thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("path contains invalid UTF-8 characters", line: 0, column: 0)', /rustc/a5a775e3f9e8043dad405e00aee0ae60882a7b71/src/tools/cargo/src/cargo/core/shell.rs:346:51 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace After: λ cd \Xff/foo/ && $cargo verify-project && $cargo metadata {"success":"true"} warning: please specify `--format-version` flag explicitly to avoid compatibility problems error: path contains invalid UTF-8 characters I am pretty sure that this has zero real-world impact, but the diff is small, so why not handle it? --- src/bin/cargo/commands/locate_project.rs | 2 +- src/bin/cargo/commands/metadata.rs | 2 +- src/bin/cargo/commands/read_manifest.rs | 4 +++- src/bin/cargo/commands/verify_project.rs | 4 ++-- src/cargo/core/shell.rs | 7 +++++-- tests/testsuite/metadata.rs | 26 ++++++++++++++++++++++++ 6 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/bin/cargo/commands/locate_project.rs b/src/bin/cargo/commands/locate_project.rs index e0cfaaf054f..16a76f6cfea 100644 --- a/src/bin/cargo/commands/locate_project.rs +++ b/src/bin/cargo/commands/locate_project.rs @@ -51,7 +51,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let location = ProjectLocation { root }; match MessageFormat::parse(args)? { - MessageFormat::Json => config.shell().print_json(&location), + MessageFormat::Json => config.shell().print_json(&location)?, MessageFormat::Plain => drop_println!(config, "{}", location.root), } diff --git a/src/bin/cargo/commands/metadata.rs b/src/bin/cargo/commands/metadata.rs index fb8e220b16d..3d14868dbb7 100644 --- a/src/bin/cargo/commands/metadata.rs +++ b/src/bin/cargo/commands/metadata.rs @@ -53,6 +53,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { }; let result = ops::output_metadata(&ws, &options)?; - config.shell().print_json(&result); + config.shell().print_json(&result)?; Ok(()) } diff --git a/src/bin/cargo/commands/read_manifest.rs b/src/bin/cargo/commands/read_manifest.rs index 4f66cedfb44..a3e162cf231 100644 --- a/src/bin/cargo/commands/read_manifest.rs +++ b/src/bin/cargo/commands/read_manifest.rs @@ -15,6 +15,8 @@ Deprecated, use `cargo metadata --no-deps` instead.\ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let ws = args.workspace(config)?; - config.shell().print_json(&ws.current()?.serialized(config)); + config + .shell() + .print_json(&ws.current()?.serialized(config))?; Ok(()) } diff --git a/src/bin/cargo/commands/verify_project.rs b/src/bin/cargo/commands/verify_project.rs index 5c2f2304ccc..aefae36b708 100644 --- a/src/bin/cargo/commands/verify_project.rs +++ b/src/bin/cargo/commands/verify_project.rs @@ -15,12 +15,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { if let Err(e) = args.workspace(config) { let mut h = HashMap::new(); h.insert("invalid".to_string(), e.to_string()); - config.shell().print_json(&h); + config.shell().print_json(&h)?; process::exit(1) } let mut h = HashMap::new(); h.insert("success".to_string(), "true".to_string()); - config.shell().print_json(&h); + config.shell().print_json(&h)?; Ok(()) } diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index e155709e144..ac1505d42bc 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -342,9 +342,12 @@ impl Shell { Ok(()) } - pub fn print_json(&mut self, obj: &T) { - let encoded = serde_json::to_string(&obj).unwrap(); + pub fn print_json(&mut self, obj: &T) -> CargoResult<()> { + // Path may fail to serialize to JSON ... + let encoded = serde_json::to_string(&obj)?; + // ... but don't fail due to a closed pipe. drop(writeln!(self.out(), "{}", encoded)); + Ok(()) } } diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 7896c2549c5..1909847b7eb 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -2953,3 +2953,29 @@ fn dep_kinds_workspace() { ) .run(); } + +// Creating non-utf8 path is an OS-specific pain, so let's run this only on +// linux, where arbitrary bytes work. +#[cfg(target_os = "linux")] +#[cargo_test] +fn cargo_metadata_non_utf8() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + use std::path::PathBuf; + + let base = PathBuf::from(OsString::from_vec(vec![255])); + + let p = project() + .no_manifest() + .file(base.join("./src/lib.rs"), "") + .file(base.join("./Cargo.toml"), &basic_lib_manifest("foo")) + .build(); + + p.cargo("metadata") + .cwd(p.root().join(base)) + .arg("--format-version") + .arg("1") + .with_stderr("error: path contains invalid UTF-8 characters") + .with_status(101) + .run(); +}