diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 22cbc198d76..0f183cdf563 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -79,7 +79,7 @@ use cargo_util::paths; use curl::easy::Easy; use lazycell::LazyCell; use serde::Deserialize; -use toml_edit::easy as toml; +use toml_edit::{easy as toml, Item}; use url::Url; mod de; @@ -1176,20 +1176,82 @@ impl Config { map.insert("include".to_string(), value); CV::Table(map, Definition::Cli) } else { - // TODO: This should probably use a more narrow parser, reject - // comments, blank lines, [headers], etc. - let toml_v: toml::Value = toml::de::from_str(arg) - .with_context(|| format!("failed to parse --config argument `{}`", arg))?; - let toml_table = toml_v.as_table().unwrap(); - if toml_table.len() != 1 { + // We only want to allow "dotted key" (see https://toml.io/en/v1.0.0#keys) + // expressions followed by a value that's not an "inline table" + // (https://toml.io/en/v1.0.0#inline-table). Easiest way to check for that is to + // parse the value as a toml_edit::Document, and check that the (single) + // inner-most table is set via dotted keys. + let doc: toml_edit::Document = arg.parse().with_context(|| { + format!("failed to parse value from --config argument `{arg}` as a dotted key expression") + })?; + fn non_empty_decor(d: &toml_edit::Decor) -> bool { + d.prefix().map_or(false, |p| !p.trim().is_empty()) + || d.suffix().map_or(false, |s| !s.trim().is_empty()) + } + let ok = { + let mut got_to_value = false; + let mut table = doc.as_table(); + let mut is_root = true; + while table.is_dotted() || is_root { + is_root = false; + if table.len() != 1 { + break; + } + let (k, n) = table.iter().next().expect("len() == 1 above"); + match n { + Item::Table(nt) => { + if table.key_decor(k).map_or(false, non_empty_decor) + || non_empty_decor(nt.decor()) + { + bail!( + "--config argument `{arg}` \ + includes non-whitespace decoration" + ) + } + table = nt; + } + Item::Value(v) if v.is_inline_table() => { + bail!( + "--config argument `{arg}` \ + sets a value to an inline table, which is not accepted" + ); + } + Item::Value(v) => { + if non_empty_decor(v.decor()) { + bail!( + "--config argument `{arg}` \ + includes non-whitespace decoration" + ) + } + got_to_value = true; + break; + } + Item::ArrayOfTables(_) => { + bail!( + "--config argument `{arg}` \ + sets a value to an array of tables, which is not accepted" + ); + } + + Item::None => { + bail!("--config argument `{arg}` doesn't provide a value") + } + } + } + got_to_value + }; + if !ok { bail!( - "--config argument `{}` expected exactly one key=value pair, got {} keys", - arg, - toml_table.len() + "--config argument `{arg}` was not a TOML dotted key expression (such as `build.jobs = 2`)" ); } + + let toml_v = toml::from_document(doc).with_context(|| { + format!("failed to parse value from --config argument `{arg}`") + })?; + CV::from_toml(Definition::Cli, toml_v) - .with_context(|| format!("failed to convert --config argument `{}`", arg))? + .with_context(|| format!("failed to convert --config argument `{arg}`"))? }; let mut seen = HashSet::new(); let tmp_table = self @@ -1197,7 +1259,7 @@ impl Config { .with_context(|| "failed to load --config include".to_string())?; loaded_args .merge(tmp_table, true) - .with_context(|| format!("failed to merge --config argument `{}`", arg))?; + .with_context(|| format!("failed to merge --config argument `{arg}`"))?; } Ok(loaded_args) } diff --git a/tests/testsuite/config_cli.rs b/tests/testsuite/config_cli.rs index 8cc5d1dd4df..db4c8600fc6 100644 --- a/tests/testsuite/config_cli.rs +++ b/tests/testsuite/config_cli.rs @@ -3,7 +3,7 @@ use super::config::{assert_error, assert_match, read_output, write_config, ConfigBuilder}; use cargo::util::config::Definition; use cargo_test_support::{paths, project}; -use std::fs; +use std::{collections::HashMap, fs}; #[cargo_test] fn config_gated() { @@ -234,11 +234,64 @@ fn merge_array_mixed_def_paths() { } #[cargo_test] -fn unused_key() { - // Unused key passed on command line. +fn enforces_format() { + // These dotted key expressions should all be fine. let config = ConfigBuilder::new() - .config_arg("build={jobs=1, unused=2}") + .config_arg("a=true") + .config_arg(" b.a = true ") + .config_arg("c.\"b\".'a'=true") + .config_arg("d.\"=\".'='=true") + .config_arg("e.\"'\".'\"'=true") .build(); + assert_eq!(config.get::("a").unwrap(), true); + assert_eq!( + config.get::>("b").unwrap(), + HashMap::from([("a".to_string(), true)]) + ); + assert_eq!( + config + .get::>>("c") + .unwrap(), + HashMap::from([("b".to_string(), HashMap::from([("a".to_string(), true)]))]) + ); + assert_eq!( + config + .get::>>("d") + .unwrap(), + HashMap::from([("=".to_string(), HashMap::from([("=".to_string(), true)]))]) + ); + assert_eq!( + config + .get::>>("e") + .unwrap(), + HashMap::from([("'".to_string(), HashMap::from([("\"".to_string(), true)]))]) + ); + + // But anything that's not a dotted key expression should be disallowed. + let _ = ConfigBuilder::new() + .config_arg("[a] foo=true") + .build_err() + .unwrap_err(); + let _ = ConfigBuilder::new() + .config_arg("a = true\nb = true") + .build_err() + .unwrap_err(); + + // We also disallow overwriting with tables since it makes merging unclear. + let _ = ConfigBuilder::new() + .config_arg("a = { first = true, second = false }") + .build_err() + .unwrap_err(); + let _ = ConfigBuilder::new() + .config_arg("a = { first = true }") + .build_err() + .unwrap_err(); +} + +#[cargo_test] +fn unused_key() { + // Unused key passed on command line. + let config = ConfigBuilder::new().config_arg("build.unused = 2").build(); config.build_config().unwrap(); let output = read_output(config); @@ -284,7 +337,7 @@ fn bad_parse() { assert_error( config.unwrap_err(), "\ -failed to parse --config argument `abc` +failed to parse value from --config argument `abc` as a dotted key expression Caused by: TOML parse error at line 1, column 4 @@ -295,6 +348,12 @@ Unexpected end of input Expected `.` or `=` ", ); + + let config = ConfigBuilder::new().config_arg("").build_err(); + assert_error( + config.unwrap_err(), + "--config argument `` was not a TOML dotted key expression (such as `build.jobs = 2`)", + ); } #[cargo_test] @@ -305,14 +364,55 @@ fn too_many_values() { config.unwrap_err(), "\ --config argument `a=1 -b=2` expected exactly one key=value pair, got 2 keys", +b=2` was not a TOML dotted key expression (such as `build.jobs = 2`)", ); +} - let config = ConfigBuilder::new().config_arg("").build_err(); +#[cargo_test] +fn no_inline_table_value() { + // Disallow inline tables + let config = ConfigBuilder::new() + .config_arg("a.b={c = \"d\"}") + .build_err(); + assert_error( + config.unwrap_err(), + "--config argument `a.b={c = \"d\"}` sets a value to an inline table, which is not accepted" + ); +} + +#[cargo_test] +fn no_array_of_tables_values() { + // Disallow array-of-tables when not in dotted form + let config = ConfigBuilder::new() + .config_arg("[[a.b]]\nc = \"d\"") + .build_err(); + assert_error( + config.unwrap_err(), + "\ +--config argument `[[a.b]] +c = \"d\"` was not a TOML dotted key expression (such as `build.jobs = 2`)", + ); +} + +#[cargo_test] +fn no_comments() { + // Disallow comments in dotted form. + let config = ConfigBuilder::new() + .config_arg("a.b = \"c\" # exactly") + .build_err(); + assert_error( + config.unwrap_err(), + "\ +--config argument `a.b = \"c\" # exactly` includes non-whitespace decoration", + ); + + let config = ConfigBuilder::new() + .config_arg("# exactly\na.b = \"c\"") + .build_err(); assert_error( config.unwrap_err(), "\ - --config argument `` expected exactly one key=value pair, got 0 keys", +--config argument `# exactly\na.b = \"c\"` includes non-whitespace decoration", ); } diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index 349e0dce467..ac210b777cb 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -274,7 +274,7 @@ fn cli_path() { assert_error( config.unwrap_err(), "\ -failed to parse --config argument `missing.toml` +failed to parse value from --config argument `missing.toml` as a dotted key expression Caused by: TOML parse error at line 1, column 13