diff --git a/src/refs/mod.rs b/src/refs/mod.rs index 9b6d7b6..2dd4a40 100644 --- a/src/refs/mod.rs +++ b/src/refs/mod.rs @@ -3,6 +3,7 @@ mod parser; use crate::types::{Mapping, Value}; use anyhow::{anyhow, Result}; use nom::error::{convert_error, VerboseError}; +use std::collections::HashSet; #[derive(Debug, PartialEq, Eq)] /// Represents a parsed Reclass reference @@ -18,11 +19,26 @@ pub enum Token { #[derive(Clone, Debug, Default)] pub struct ResolveState { + /// Reference paths which we've seen during reference resolution + seen_paths: HashSet, /// Recursion depth of the resolution (in number of calls to Token::resolve() for Token::Ref /// objects). depth: usize, } +impl ResolveState { + /// Formats paths that have been seen as a comma-separated list. + fn seen_paths_list(&self) -> String { + let mut paths = self + .seen_paths + .iter() + .map(|p| format!("\"{p}\"")) + .collect::>(); + paths.sort(); + paths.join(", ") + } +} + /// Maximum allowed recursion depth for Token::resolve(). We're fairly conservative with the value, /// since it's rather unlikely that a well-formed inventory will have any references that are /// nested deeper than 64. @@ -98,14 +114,24 @@ impl Token { // implementation. We abort at a recursion depth of 64, since it's quite // unlikely that there's a legitimate case where we have a recursion depth of // 64 when resolving references for a well formed inventory. + let paths = state.seen_paths_list(); return Err(anyhow!( - "Token resolution exceeded recursion depth of {RESOLVE_MAX_DEPTH}." + "Token resolution exceeded recursion depth of {RESOLVE_MAX_DEPTH}. \ + We've seen the following reference paths: [{paths}].", )); } // Construct flattened ref path by resolving any potential nested references in the // Ref's Vec. let path = interpolate_token_slice(parts, params, state)?; + if state.seen_paths.contains(&path) { + // we've already seen this reference, so we know there's a loop, and can abort + // resolution. + let paths = state.seen_paths_list(); + return Err(anyhow!("Reference loop with reference paths [{paths}].")); + } + state.seen_paths.insert(path.clone()); + // generate iterator containing flattened reference path segments let mut refpath_iter = path.split(':'); // we handle the first element separately, so we can establish a local mutable diff --git a/src/refs/token_resolve_parse_tests.rs b/src/refs/token_resolve_parse_tests.rs index 68c73c7..c6d7c15 100644 --- a/src/refs/token_resolve_parse_tests.rs +++ b/src/refs/token_resolve_parse_tests.rs @@ -222,7 +222,7 @@ fn test_resolve_mapping_embedded() { } #[test] -#[should_panic(expected = "Token resolution exceeded recursion depth of 64.")] +#[should_panic(expected = "Reference loop with reference paths [\"foo\"].")] fn test_resolve_recursive_error() { let p = r#" foo: ${foo} @@ -235,7 +235,7 @@ fn test_resolve_recursive_error() { } #[test] -#[should_panic(expected = "Token resolution exceeded recursion depth of 64.")] +#[should_panic(expected = "Reference loop with reference paths [\"bar\", \"foo\"].")] fn test_resolve_recursive_error_2() { let p = r#" foo: ${bar} @@ -249,7 +249,7 @@ fn test_resolve_recursive_error_2() { } #[test] -#[should_panic(expected = "Token resolution exceeded recursion depth of 64.")] +#[should_panic(expected = "Reference loop with reference paths [\"baz\", \"foo\"].")] fn test_resolve_nested_recursive_error() { let p = r#" foo: ${baz} diff --git a/src/types/value/value_interpolate_tests.rs b/src/types/value/value_interpolate_tests.rs index b40cab8..19bbc4c 100644 --- a/src/types/value/value_interpolate_tests.rs +++ b/src/types/value/value_interpolate_tests.rs @@ -471,7 +471,7 @@ fn test_interpolate_nested_mapping_no_loop() { #[test] #[should_panic(expected = "While resolving references in \ {\"foo\": {\"bar\": \"${bar}\"}, \"bar\": [{\"baz\": \"baz\", \"qux\": \"qux\"}, \ - {\"baz\": \"${foo}\"}]}: Token resolution exceeded recursion depth of 64.")] + {\"baz\": \"${foo}\"}]}: Reference loop with reference paths [\"bar\", \"foo\"].")] fn test_merge_interpolate_loop() { let base = r#" foo: @@ -499,7 +499,7 @@ fn test_merge_interpolate_loop() { #[should_panic(expected = "While resolving references in \ {\"foo\": {\"bar\": [\"${bar}\", \"${baz}\"]}, \"bar\": \"${qux}\", \ \"baz\": {\"bar\": \"${foo}\"}, \"qux\": 3.14}: \ - Token resolution exceeded recursion depth of 64.")] + Reference loop with reference paths [\"baz\", \"foo\"].")] fn test_interpolate_sequence_loop() { let base = r#" foo: @@ -522,7 +522,7 @@ fn test_interpolate_sequence_loop() { {\"foo\": {\"bar\": {\"baz\": \"${foo:baz:bar}\", \"qux\": \"${foo:qux:foo}\"}, \ \"baz\": {\"bar\": \"qux\", \"qux\": \"${foo:bar:qux}\"}, \"qux\": \ {\"foo\": \"${foo:baz:qux}\"}}}: \ - Token resolution exceeded recursion depth of 64.")] + Reference loop with reference paths [\"foo:bar:qux\", \"foo:baz:qux\", \"foo:qux:foo\"].")] fn test_interpolate_nested_mapping_loop() { let m = r#" foo: @@ -551,7 +551,8 @@ fn test_interpolate_nested_mapping_loop() { ${foo:${foo:${foo:${foo:${foo:${foo:${foo:${foo:${foo:${foo:${foo:${foo:\ ${foo:${foo:${foo:${foo:${foo:${foo}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}\ }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}\": \ - Token resolution exceeded recursion depth of 64." + Token resolution exceeded recursion depth of 64. \ + We've seen the following reference paths: []." )] fn test_interpolate_depth_exceeded() { // construct a reference string which is a nested sequence of ${foo:....${foo}} with 70 nesting