From 33358379f4bbbda3f4fd20bd5a413f3003e959cf Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Tue, 23 Aug 2022 18:51:23 +0100 Subject: [PATCH] tor-config: Introduce ResolutionResults This will allow us to handle new kinds of warnigns etc. --- crates/arti/src/cfg.rs | 13 ++++---- crates/tor-config/semver.md | 4 +++ crates/tor-config/src/lib.rs | 2 +- crates/tor-config/src/load.rs | 60 ++++++++++++++++++++++++----------- crates/tor-config/src/misc.rs | 8 ++++- 5 files changed, 60 insertions(+), 27 deletions(-) diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs index 4e7701ffe..a5f6b1cc9 100644 --- a/crates/arti/src/cfg.rs +++ b/crates/arti/src/cfg.rs @@ -178,6 +178,7 @@ mod test { use arti_client::config::TorClientConfigBuilder; use regex::Regex; use std::time::Duration; + use tor_config::load::ResolutionResults; use super::*; @@ -208,14 +209,14 @@ mod test { // Also we should ideally test that every setting from the config appears here in // the file. Possibly that could be done with some kind of stunt Deserializer, // but it's not trivial. - let (parsed, unrecognized): (ArtiCombinedConfig, _) = - tor_config::resolve_return_unrecognized(cfg).unwrap(); + let results: ResolutionResults = + tor_config::resolve_return_results(cfg).unwrap(); - assert_eq!(&parsed, &default); - assert_eq!(&parsed, &empty_config); + assert_eq!(&results.value, &default); + assert_eq!(&results.value, &empty_config); - assert_eq!(unrecognized, &[]); - parsed + assert_eq!(results.unrecognized, &[]); + results.value }; let _ = parses_to_defaults(ARTI_EXAMPLE_CONFIG); diff --git a/crates/tor-config/semver.md b/crates/tor-config/semver.md index 6d0fb2d24..a4c55f10a 100644 --- a/crates/tor-config/semver.md +++ b/crates/tor-config/semver.md @@ -4,3 +4,7 @@ ADDED: resolve_option_general ADDED: sources::FoundConfigFiles BREAKING: ConfigurationSources takes ConfigurationSource for files, not Paths BREAKING: ConfigurationSources::from_cmdline wants an iterator of defaults +BREAKING: load::resolve_ignore_unrecognized renamed resolve_ignore_warnings +BREAKING: load::resolve_return_unrecognized replaced with resolve_return_results +BREAKING: load::UnrecognizedKey renamed to DisfavouredKey + diff --git a/crates/tor-config/src/lib.rs b/crates/tor-config/src/lib.rs index ceacc9900..4f5303f7b 100644 --- a/crates/tor-config/src/lib.rs +++ b/crates/tor-config/src/lib.rs @@ -106,7 +106,7 @@ pub use cmdline::CmdLine; pub use config as config_crate; pub use educe; pub use err::{ConfigBuildError, ReconfigureError}; -pub use load::{resolve, resolve_ignore_unrecognized, resolve_return_unrecognized}; +pub use load::{resolve, resolve_ignore_warnings, resolve_return_results}; pub use misc::*; pub use mut_cfg::MutCfg; pub use paste::paste; diff --git a/crates/tor-config/src/load.rs b/crates/tor-config/src/load.rs index 7aa780ac3..740124e4a 100644 --- a/crates/tor-config/src/load.rs +++ b/crates/tor-config/src/load.rs @@ -285,7 +285,7 @@ enum PathEntry { fn resolve_inner( input: config::Config, want_unrecognized: bool, -) -> Result<(T, Vec), ConfigResolveError> +) -> Result, ConfigResolveError> where T: Resolvable, { @@ -298,9 +298,9 @@ where }, }; - let val = Resolvable::resolve(&mut lc)?; + let value = Resolvable::resolve(&mut lc)?; - let ign = match lc.unrecognized { + let unrecognized = match lc.unrecognized { UK::AllKeys => panic!("all unrecognized, as if we had processed nothing"), UK::These(ign) => ign, } @@ -308,7 +308,10 @@ where .filter(|ip| !ip.path.is_empty()) .collect_vec(); - Ok((val, ign)) + Ok(ResolutionResults { + value, + unrecognized, + }) } /// Deserialize and build overall configuration from config sources @@ -327,29 +330,43 @@ pub fn resolve(input: config::Config) -> Result where T: Resolvable, { - let (val, ign) = resolve_inner(input, true)?; - for ign in ign { + let ResolutionResults { + value, + unrecognized, + } = resolve_inner(input, true)?; + for ign in unrecognized { warn!("unrecognized configuration key: {}", &ign); } - Ok(val) + Ok(value) } /// Deserialize and build overall configuration, reporting unrecognized keys in the return value -pub fn resolve_return_unrecognized( +pub fn resolve_return_results( input: config::Config, -) -> Result<(T, Vec), ConfigResolveError> +) -> Result, ConfigResolveError> where T: Resolvable, { resolve_inner(input, true) } +/// Results of a successful `resolve_return_disfavoured` +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct ResolutionResults { + /// The configuration, successfully parsed + pub value: T, + + /// Any config keys which were found in the input, but not recognized (and so, ignored) + pub unrecognized: Vec, +} + /// Deserialize and build overall configuration, silently ignoring unrecognized config keys -pub fn resolve_ignore_unrecognized(input: config::Config) -> Result +pub fn resolve_ignore_warnings(input: config::Config) -> Result where T: Resolvable, { - Ok(resolve_inner(input, false)?.0) + Ok(resolve_inner(input, false)?.value) } impl Resolvable for T @@ -696,12 +713,17 @@ mod test { .build() .unwrap(); - let _: (TestConfigA, TestConfigB) = resolve_ignore_unrecognized(cfg.clone()).unwrap(); + let _: (TestConfigA, TestConfigB) = resolve_ignore_warnings(cfg.clone()).unwrap(); - let ((a, b), ign): ((TestConfigA, TestConfigB), _) = - resolve_return_unrecognized(cfg).unwrap(); + let resolved: ResolutionResults<(TestConfigA, TestConfigB)> = + resolve_return_results(cfg).unwrap(); + let (a, b) = resolved.value; - let ign = ign.into_iter().map(|ik| ik.to_string()).collect_vec(); + let ign = resolved + .unrecognized + .into_iter() + .map(|ik| ik.to_string()) + .collect_vec(); assert_eq! { &a, &TestConfigA { a: "hi".into() } }; assert_eq! { &b, &TestConfigB { b: "".into() } }; @@ -742,15 +764,15 @@ mod test { .unwrap(); { // First try "A", then "C". - let res1: Result<((TestConfigA, TestConfigC), Vec), _> = - resolve_return_unrecognized(cfg.clone()); + let res1: Result, _> = + resolve_return_results(cfg.clone()); assert!(res1.is_err()); assert!(matches!(res1, Err(ConfigResolveError::Deserialize(_)))); } { // Now the other order: first try "C", then "A". - let res2: Result<((TestConfigC, TestConfigA), Vec), _> = - resolve_return_unrecognized(cfg.clone()); + let res2: Result, _> = + resolve_return_results(cfg.clone()); assert!(res2.is_err()); assert!(matches!(res2, Err(ConfigResolveError::Deserialize(_)))); } diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs index 703d32ee5..72ea44b22 100644 --- a/crates/tor-config/src/misc.rs +++ b/crates/tor-config/src/misc.rs @@ -104,7 +104,13 @@ impl Listen { /// /// Special case: if `port` is zero, specifies no listening. pub fn new_localhost(port: u16) -> Listen { - Listen(port.try_into().ok().map(ListenItem::Localhost).into_iter().collect_vec()) + Listen( + port.try_into() + .ok() + .map(ListenItem::Localhost) + .into_iter() + .collect_vec(), + ) } /// Create a new `Listen`, possibly specifying listening on a port on localhost