From 8934b34489290886b4bdddc4b5949d0c955f86e7 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sun, 15 Oct 2023 09:29:48 +0100 Subject: fix(nix-compat/derivation): handle dups This now properly checks for duplicate output names in input derivation output names, and duplicate input sources. Change-Id: I0053854bfbf504f4f511fb3fe1a77a82b3aa61dd Reviewed-on: https://cl.tvl.fyi/c/depot/+/9738 Autosubmit: flokli Tested-by: BuildkiteCI Reviewed-by: raitobezarius --- tvix/nix-compat/src/derivation/parser.rs | 180 +++++++++++++++++++++++++++---- 1 file changed, 161 insertions(+), 19 deletions(-) (limited to 'tvix/nix-compat/src/derivation/parser.rs') diff --git a/tvix/nix-compat/src/derivation/parser.rs b/tvix/nix-compat/src/derivation/parser.rs index dfcd327e4f..2b5a804159 100644 --- a/tvix/nix-compat/src/derivation/parser.rs +++ b/tvix/nix-compat/src/derivation/parser.rs @@ -125,8 +125,45 @@ fn parse_outputs(i: &[u8]) -> NomResult<&[u8], BTreeMap> { } } -fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap>> { - parse_kv::, _>(aterm::parse_str_list)(i) +fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap>> { + let (i, input_derivations_list) = parse_kv::, _>(aterm::parse_str_list)(i)?; + + // This is a HashMap of drv paths to a list of output names. + let mut input_derivations: BTreeMap> = BTreeMap::new(); + + for (input_derivation, output_names) in input_derivations_list { + let mut new_output_names = BTreeSet::new(); + for output_name in output_names.into_iter() { + if !new_output_names.insert(output_name.clone()) { + return Err(nom::Err::Failure(NomError { + input: i, + code: ErrorKind::DuplicateInputDerivationOutputName( + input_derivation.to_string(), + output_name.to_string(), + ), + })); + } + } + input_derivations.insert(input_derivation, new_output_names); + } + + Ok((i, input_derivations)) +} + +fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet> { + let (i, input_sources_lst) = aterm::parse_str_list(i).map_err(into_nomerror)?; + + let mut input_sources: BTreeSet<_> = BTreeSet::new(); + for input_source in input_sources_lst.into_iter() { + if !input_sources.insert(input_source.clone()) { + return Err(nom::Err::Failure(NomError { + input: i, + code: ErrorKind::DuplicateInputSource(input_source), + })); + } + } + + Ok((i, input_sources)) } pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> { @@ -144,7 +181,7 @@ pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> { // // parse input derivations terminated(parse_input_derivations, nomchar(',')), // // parse input sources - |i| terminated(aterm::parse_str_list, nomchar(','))(i).map_err(into_nomerror), + terminated(parse_input_sources, nomchar(',')), // // parse system |i| terminated(aterm::parse_string_field, nomchar(','))(i).map_err(into_nomerror), // // parse builder @@ -166,24 +203,12 @@ pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> { arguments, environment, )| { - // All values in input_derivations need to be converted from - // Vec to BTreeSet - let mut input_derivations_new: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - for (k, v) in input_derivations.into_iter() { - let values_new: BTreeSet<_> = BTreeSet::from_iter(v.into_iter()); - input_derivations_new.insert(k, values_new); - // TODO: actually check they're not duplicate in the parser side! - } - - // Input sources need to be converted from Vec<_> to BTreeSet<_> - let input_sources_new: BTreeSet<_> = BTreeSet::from_iter(input_sources); - Derivation { arguments, builder, environment, - input_derivations: input_derivations_new, - input_sources: input_sources_new, + input_derivations, + input_sources, outputs, system, } @@ -246,9 +271,9 @@ where #[cfg(test)] mod tests { - use std::collections::BTreeMap; + use std::collections::{BTreeMap, BTreeSet}; - use crate::derivation::Output; + use crate::derivation::{parse_error::ErrorKind, Output}; use bstr::{BString, ByteSlice}; use lazy_static::lazy_static; use test_case::test_case; @@ -279,8 +304,44 @@ mod tests { b.insert("b".to_string(), b"2".as_bstr().to_owned()); b }; + static ref EXP_INPUT_DERIVATIONS_SIMPLE: BTreeMap> = { + let mut b = BTreeMap::new(); + b.insert( + "/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv".to_string(), + { + let mut output_names = BTreeSet::new(); + output_names.insert("out".to_string()); + output_names + }, + ); + b.insert( + "/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv".to_string(), + { + let mut output_names = BTreeSet::new(); + output_names.insert("out".to_string()); + output_names.insert("lib".to_string()); + output_names + }, + ); + b + }; + static ref EXP_INPUT_DERIVATIONS_SIMPLE_ATERM: String = { + format!( + "[(\"{0}\",[\"out\"]),(\"{1}\",[\"out\",\"lib\"])]", + "/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv", + "/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv" + ) + }; + static ref EXP_INPUT_SOURCES_SIMPLE: BTreeSet = { + let mut b = BTreeSet::new(); + b.insert("/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out".to_string()); + b.insert("/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib".to_string()); + b + }; } + /// Ensure parsing KVs works + #[test_case(b"[]", &BTreeMap::new(), b""; "empty")] #[test_case(b"[(\"a\",\"1\"),(\"b\",\"2\")]", &EXP_AB_MAP, b""; "simple")] fn parse_kv(input: &'static [u8], expected: &BTreeMap, exp_rest: &[u8]) { let (rest, parsed) = super::parse_kv::(crate::aterm::parse_bstr_field)(input) @@ -289,6 +350,87 @@ mod tests { assert_eq!(*expected, parsed); } + /// Ensures the kv parser complains about duplicate map keys + #[test] + fn parse_kv_fail_dup_keys() { + let input: &'static [u8] = b"[(\"a\",\"1\"),(\"a\",\"2\")]"; + let e = super::parse_kv::(crate::aterm::parse_bstr_field)(input) + .expect_err("must fail"); + + match e { + nom::Err::Failure(e) => { + assert_eq!(ErrorKind::DuplicateMapKey("a".to_string()), e.code); + } + _ => panic!("unexpected error"), + } + } + + /// Ensure parsing input derivations works. + #[test_case(b"[]", &BTreeMap::new(); "empty")] + #[test_case(EXP_INPUT_DERIVATIONS_SIMPLE_ATERM.as_bytes(), &EXP_INPUT_DERIVATIONS_SIMPLE; "simple")] + fn parse_input_derivations( + input: &'static [u8], + expected: &BTreeMap>, + ) { + let (rest, parsed) = super::parse_input_derivations(input).expect("must parse"); + + assert_eq!(expected, &parsed, "parsed mismatch"); + assert!(rest.is_empty(), "rest must be empty"); + } + + /// Ensures the input derivation parser complains about duplicate output names + #[test] + fn parse_input_derivations_fail_dup_output_names() { + let input_str = format!( + "[(\"{0}\",[\"out\"]),(\"{1}\",[\"out\",\"out\"])]", + "/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv", + "/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv" + ); + let e = super::parse_input_derivations(input_str.as_bytes()).expect_err("must fail"); + + match e { + nom::Err::Failure(e) => { + assert_eq!( + ErrorKind::DuplicateInputDerivationOutputName( + "/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv".to_string(), + "out".to_string() + ), + e.code + ); + } + _ => panic!("unexpected error"), + } + } + + /// Ensure parsing input sources works + #[test_case(b"[]", &BTreeSet::new(); "empty")] + #[test_case(b"[\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out\",\"/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib\"]", &EXP_INPUT_SOURCES_SIMPLE; "simple")] + fn parse_input_sources(input: &'static [u8], expected: &BTreeSet) { + let (rest, parsed) = super::parse_input_sources(input).expect("must parse"); + + assert_eq!(expected, &parsed, "parsed mismatch"); + assert!(rest.is_empty(), "rest must be empty"); + } + + /// Ensures the input sources parser complains about duplicate input sources + #[test] + fn parse_input_sources_fail_dup_keys() { + let input: &'static [u8] = b"[\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo\",\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo\"]"; + let e = super::parse_input_sources(input).expect_err("must fail"); + + match e { + nom::Err::Failure(e) => { + assert_eq!( + ErrorKind::DuplicateInputSource( + "/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo".to_string() + ), + e.code + ); + } + _ => panic!("unexpected error"), + } + } + #[test_case( br#"("out","/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo","","")"#, ("out".to_string(), Output { -- cgit 1.4.1