Skip to content

Commit

Permalink
Auto merge of #9226 - matklad:utf8everywhere, r=alexcrichton
Browse files Browse the repository at this point in the history
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?
  • Loading branch information
bors committed Mar 2, 2021
2 parents 5c74eb5 + dd5806d commit c68432f
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/locate_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
4 changes: 3 additions & 1 deletion src/bin/cargo/commands/read_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
4 changes: 2 additions & 2 deletions src/bin/cargo/commands/verify_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
7 changes: 5 additions & 2 deletions src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,12 @@ impl Shell {
Ok(())
}

pub fn print_json<T: serde::ser::Serialize>(&mut self, obj: &T) {
let encoded = serde_json::to_string(&obj).unwrap();
pub fn print_json<T: serde::ser::Serialize>(&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(())
}
}

Expand Down
26 changes: 26 additions & 0 deletions tests/testsuite/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit c68432f

Please sign in to comment.