From 6ac6f3d086628d76c9eec072518d01ba6d4e11c3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 13 Jul 2018 16:12:41 -0400 Subject: [PATCH] treefile.rs: Deny unknown fields by default Let's not make the same mistake we did with JSON where typoing a field means it's silently ignored. This actually caught a bug in a YAML usage we had: ``` error: Failed to load YAML treefile: unknown field `install_langs`, expected one of ... `install-langs` ... ``` Yes, this is a compatibility break with the feature we just announced but...I seriously doubt anyone (that isn't known to me) has converted yet, and if they are excited enough to start using a two-week-old feature they can adjust. Closes: #1459 Approved by: cgwalters --- rust/src/treefile.rs | 50 +++++++++++++++++++++++++++--- tests/composedata/fedora-base.json | 15 ++------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 8e596e7d..8ae54d76 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -26,10 +26,8 @@ use serde_yaml; use std::path::Path; use std::{fs, io}; -pub fn treefile_read_impl(filename: &Path, output: W) -> io::Result<()> { - let f = io::BufReader::new(fs::File::open(filename)?); - - let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(f) { +fn treefile_parse_yaml(input: R) -> io::Result { + let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(input) { Ok(t) => t, Err(e) => { return Err(io::Error::new( @@ -44,8 +42,13 @@ pub fn treefile_read_impl(filename: &Path, output: W) -> io::Resul treefile.packages = Some(whitespace_split_packages(&pkgs)); } - serde_json::to_writer_pretty(output, &treefile)?; + Ok(treefile) +} +pub fn treefile_read_impl(filename: &Path, output: W) -> io::Result<()> { + let f = io::BufReader::new(fs::File::open(filename)?); + let treefile = treefile_parse_yaml(f)?; + serde_json::to_writer_pretty(output, &treefile)?; Ok(()) } @@ -99,6 +102,7 @@ fn serde_true() -> bool { } #[derive(Serialize, Deserialize, Debug)] +#[serde(deny_unknown_fields)] pub struct TreeComposeConfig { // Compose controls #[serde(rename = "ref")] @@ -187,3 +191,39 @@ pub struct TreeComposeConfig { #[serde(rename = "remove-from-packages")] pub remove_from_packages: Option>>, } + +#[cfg(test)] +mod tests { + use super::*; + + static VALID_PRELUDE : &str = r###" +ref: "exampleos/x86_64/blah" +packages: + - foo bar + - baz +"###; + + #[test] + fn basic_valid() { + let input = io::BufReader::new(VALID_PRELUDE.as_bytes()); + let treefile = treefile_parse_yaml(input).unwrap(); + assert!(treefile.treeref == "exampleos/x86_64/blah"); + assert!(treefile.packages.unwrap().len() == 3); + } + + #[test] + fn basic_invalid() { + let mut buf = VALID_PRELUDE.to_string(); + buf.push_str(r###"install_langs: + - "klingon" + - "esperanto" +"###); + let buf = buf.as_bytes(); + let input = io::BufReader::new(buf); + match treefile_parse_yaml(input) { + Err(ref e) if e.kind() == io::ErrorKind::InvalidInput => {}, + Err(ref e) => panic!("Expected invalid treefile, not {}", e.to_string()), + _ => panic!("Expected invalid treefile"), + } + } +} diff --git a/tests/composedata/fedora-base.json b/tests/composedata/fedora-base.json index e5dc691c..2a16ca85 100644 --- a/tests/composedata/fedora-base.json +++ b/tests/composedata/fedora-base.json @@ -5,19 +5,8 @@ "repos": ["fedora", "updates"], "packages": ["kernel", "nss-altfiles", "systemd", "ostree", "selinux-policy-targeted", "chrony", - "tuned", "iputils", "fedora-release-atomichost", "docker", "container-selinux"], - - "packages-aarch64": ["grub2-efi", "ostree-grub2", - "efibootmgr", "shim"], - - "packages-armhfp": ["extlinux-bootloader"], - - "packages-ppc64": ["grub2", "ostree-grub2"], - - "packages-ppc64le": ["grub2", "ostree-grub2"], - - "packages-x86_64": ["grub2", "grub2-efi", "ostree-grub2", - "efibootmgr", "shim"], + "tuned", "iputils", "fedora-release-atomichost", "docker", "container-selinux", + "grub2", "grub2-efi", "ostree-grub2", "efibootmgr", "shim"], "ignore-removed-users": ["root"], "ignore-removed-groups": ["root"],