Check --config for dotted keys only

src/cargo/util/config/mod.rs

@@ -1176,18 +1176,51 @@ 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 `{}` as a dotted key expression", arg)
+                })?;
+                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 (_, n) = table.iter().next().expect("len() == 1 above");
+                        if let Some(nt) = n.as_table() {
+                            table = nt;
+                        } else if n.is_inline_table() {
+                            bail!(
+                                "--config argument `{}` sets a value to an inline table, which is not accepted",
+                                arg,
+                            );
+                        } else {
+                            got_to_value = true;
+                            break;
+                        }
+                    }
+                    got_to_value
+                };
+                if !ok {
                     bail!(
-                        "--config argument `{}` expected exactly one key=value pair, got {} keys",
-                        arg,
-                        toml_table.len()
+                        "--config argument `{}` was not a TOML dotted key expression (a.b.c = _)",
+                        arg
                     );
                 }
+
+                // Rather than trying to bridge between toml_edit::Document and toml::Value, we
+                // just parse the whole argument again now that we know it has the right format.
+                let toml_v = toml::from_str(arg).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))?
             };

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::<bool>("a").unwrap(), true);
+    assert_eq!(
+        config.get::<HashMap<String, bool>>("b").unwrap(),
+        HashMap::from([("a".to_string(), true)])
+    );
+    assert_eq!(
+        config
+            .get::<HashMap<String, HashMap<String, bool>>>("c")
+            .unwrap(),
+        HashMap::from([("b".to_string(), HashMap::from([("a".to_string(), true)]))])
+    );
+    assert_eq!(
+        config
+            .get::<HashMap<String, HashMap<String, bool>>>("d")
+            .unwrap(),
+        HashMap::from([("=".to_string(), HashMap::from([("=".to_string(), true)]))])
+    );
+    assert_eq!(
+        config
+            .get::<HashMap<String, HashMap<String, bool>>>("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 (a.b.c = _)",
+    );
 }
 
 #[cargo_test]
@@ -305,14 +364,33 @@ 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 (a.b.c = _)",
     );
+}
 
-    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 `` expected exactly one key=value pair, got 0 keys",
+--config argument `[[a.b]]
+c = \"d\"` was not a TOML dotted key expression (a.b.c = _)",
     );
 }
 

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