diff --git a/crates/effects/src/ocio_config.rs b/crates/effects/src/ocio_config.rs index af860c2fc..e31281ede 100644 --- a/crates/effects/src/ocio_config.rs +++ b/crates/effects/src/ocio_config.rs @@ -168,19 +168,29 @@ fn split_key(s: &str) -> Option<(String, &str)> { None } -/// Bracket balance of a flow fragment (outside quotes). -fn balance(s: &str) -> i32 { - let (mut sq, mut dq, mut d) = (false, false, 0i32); - for c in s.chars() { - match c { - '\'' if !dq => sq = !sq, - '"' if !sq => dq = !dq, - '{' | '[' if !sq && !dq => d = d.saturating_add(1), - '}' | ']' if !sq && !dq => d = d.saturating_sub(1), - _ => {} +/// Bracket balance of a flow fragment (outside quotes), kept as lines are appended so each +/// character is looked at once however many lines a collection spans. +#[derive(Default)] +struct Balance { + sq: bool, + dq: bool, + d: i32, +} + +impl Balance { + /// Takes in the next piece of the fragment and returns the balance so far. + fn feed(&mut self, s: &str) -> i32 { + for c in s.chars() { + match c { + '\'' if !self.dq => self.sq = !self.sq, + '"' if !self.sq => self.dq = !self.dq, + '{' | '[' if !self.sq && !self.dq => self.d = self.d.saturating_add(1), + '}' | ']' if !self.sq && !self.dq => self.d = self.d.saturating_sub(1), + _ => {} + } } + self.d } - d } struct Flow<'a> { @@ -223,12 +233,19 @@ impl Flow<'_> { self.i += 1; break; } + // A `]` cannot close a mapping: the text is malformed, so the rest is + // skipped (as past the depth limit) instead of read again and again. + Some(b']') => { + self.i = self.s.len(); + break; + } Some(b',') => { self.i += 1; continue; } _ => {} } + let start = self.i; let k = self.scalar(true); self.ws(); if self.peek() == Some(b':') { @@ -236,6 +253,11 @@ impl Flow<'_> { } let v = self.nested(); m.push((unquote(&k), v)); + // Every entry consumes text; one that did not would repeat forever. + if self.i == start { + self.i = self.s.len(); + break; + } } Yaml::Map(m) } @@ -250,11 +272,23 @@ impl Flow<'_> { self.i += 1; break; } + // A `}` cannot close a sequence: skip the malformed rest (see above). + Some(b'}') => { + self.i = self.s.len(); + break; + } Some(b',') => { self.i += 1; continue; } - _ => v.push(self.nested()), + _ => { + let start = self.i; + v.push(self.nested()); + if self.i == start { + self.i = self.s.len(); + break; + } + } } } Yaml::Seq(v) @@ -364,11 +398,14 @@ impl Lines { if rest.starts_with('{') || rest.starts_with('[') || (rest.starts_with('!') && (rest.contains('{') || rest.contains('['))) { // Flow collections may continue on the next lines. let mut text = rest.to_string(); - while balance(&text) > 0 + let mut bal = Balance::default(); + let mut open = bal.feed(&text); + while open > 0 && let Some((_, next)) = self.v.get(*i) { text.push(' '); text.push_str(next); + open = bal.feed(next); *i += 1; } return flow(&text, depth); diff --git a/crates/effects/src/ocio_config/tests.rs b/crates/effects/src/ocio_config/tests.rs index f534fe0e2..b616e6263 100644 --- a/crates/effects/src/ocio_config/tests.rs +++ b/crates/effects/src/ocio_config/tests.rs @@ -94,6 +94,24 @@ fn yaml_nesting_is_bounded() { assert!(Config::parse(&flow, Path::new(".")).is_err()); } +/// A closing bracket of the wrong kind ends the collection instead of being read forever, and a +/// flow collection spread over many lines is assembled in linear time. +#[test] +fn yaml_malformed_flow_is_bounded() { + for text in ["a: {x: ]\nb: 1\n", "a: [x, }\nb: 1\n", "a: [{k: ], 1]\n", "a: {[}: ]}\n"] { + let _ = parse_yaml(text); + } + assert!(matches!(parse_yaml("a: {x: ]\n"), Yaml::Map(_))); + let mut many = String::from("a: [\n"); + for _ in 0..200_000 { + many.push_str(" 1,\n"); + } + many.push_str("]\nb: 2\n"); + let y = parse_yaml(&many); + assert_eq!(y.get("a").map(|a| a.seq().len()), Some(200_000)); + assert_eq!(y.get("b").and_then(Yaml::str), Some("2")); +} + #[test] fn parses_spaces_roles_displays_and_converts() { let dir = temp_dir("basic");