Skip to content

Commit dd5806d

Browse files
committed
Don't panic when printing JSON with non-utf8 paths
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?
1 parent cb0e8c3 commit dd5806d

File tree

6 files changed

+38
-7
lines changed

6 files changed

+38
-7
lines changed

src/bin/cargo/commands/locate_project.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
5151
let location = ProjectLocation { root };
5252

5353
match MessageFormat::parse(args)? {
54-
MessageFormat::Json => config.shell().print_json(&location),
54+
MessageFormat::Json => config.shell().print_json(&location)?,
5555
MessageFormat::Plain => drop_println!(config, "{}", location.root),
5656
}
5757

src/bin/cargo/commands/metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
5353
};
5454

5555
let result = ops::output_metadata(&ws, &options)?;
56-
config.shell().print_json(&result);
56+
config.shell().print_json(&result)?;
5757
Ok(())
5858
}

src/bin/cargo/commands/read_manifest.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ Deprecated, use `cargo metadata --no-deps` instead.\
1515

1616
pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
1717
let ws = args.workspace(config)?;
18-
config.shell().print_json(&ws.current()?.serialized(config));
18+
config
19+
.shell()
20+
.print_json(&ws.current()?.serialized(config))?;
1921
Ok(())
2022
}

src/bin/cargo/commands/verify_project.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
1515
if let Err(e) = args.workspace(config) {
1616
let mut h = HashMap::new();
1717
h.insert("invalid".to_string(), e.to_string());
18-
config.shell().print_json(&h);
18+
config.shell().print_json(&h)?;
1919
process::exit(1)
2020
}
2121

2222
let mut h = HashMap::new();
2323
h.insert("success".to_string(), "true".to_string());
24-
config.shell().print_json(&h);
24+
config.shell().print_json(&h)?;
2525
Ok(())
2626
}

src/cargo/core/shell.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,12 @@ impl Shell {
342342
Ok(())
343343
}
344344

345-
pub fn print_json<T: serde::ser::Serialize>(&mut self, obj: &T) {
346-
let encoded = serde_json::to_string(&obj).unwrap();
345+
pub fn print_json<T: serde::ser::Serialize>(&mut self, obj: &T) -> CargoResult<()> {
346+
// Path may fail to serialize to JSON ...
347+
let encoded = serde_json::to_string(&obj)?;
348+
// ... but don't fail due to a closed pipe.
347349
drop(writeln!(self.out(), "{}", encoded));
350+
Ok(())
348351
}
349352
}
350353

tests/testsuite/metadata.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2953,3 +2953,29 @@ fn dep_kinds_workspace() {
29532953
)
29542954
.run();
29552955
}
2956+
2957+
// Creating non-utf8 path is an OS-specific pain, so let's run this only on
2958+
// linux, where arbitrary bytes work.
2959+
#[cfg(target_os = "linux")]
2960+
#[cargo_test]
2961+
fn cargo_metadata_non_utf8() {
2962+
use std::ffi::OsString;
2963+
use std::os::unix::ffi::OsStringExt;
2964+
use std::path::PathBuf;
2965+
2966+
let base = PathBuf::from(OsString::from_vec(vec![255]));
2967+
2968+
let p = project()
2969+
.no_manifest()
2970+
.file(base.join("./src/lib.rs"), "")
2971+
.file(base.join("./Cargo.toml"), &basic_lib_manifest("foo"))
2972+
.build();
2973+
2974+
p.cargo("metadata")
2975+
.cwd(p.root().join(base))
2976+
.arg("--format-version")
2977+
.arg("1")
2978+
.with_stderr("error: path contains invalid UTF-8 characters")
2979+
.with_status(101)
2980+
.run();
2981+
}

0 commit comments

Comments
 (0)