diff --git a/src/parsers/yaml.rs b/src/parsers/yaml.rs index 2a1245dfbf9..55cf73ff39c 100644 --- a/src/parsers/yaml.rs +++ b/src/parsers/yaml.rs @@ -2354,7 +2354,6 @@ pub struct Parser<'i, Enc: Encoding> { pub stack_check: StackCheck, - pub merge_props_budget: usize, pub alias_expansion_budget: usize, } @@ -2390,7 +2389,6 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { tag_handles: StringHashMap::default(), whitespace_buf: Vec::new(), stack_check: StackCheck::init(), - merge_props_budget: MappingProps::MAX_MERGED_PROPERTIES, alias_expansion_budget: Self::MAX_ALIAS_EXPANSION, } } @@ -2781,7 +2779,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { Expr::init(E::Null {}, self.token.start.loc()) }; let mut props = MappingProps::init(); - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; Expr::init( E::Object { properties: props.move_list(), @@ -2914,7 +2912,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { current_mapping_indent: Some(self.token.indent), ..Default::default() })?; - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; } // [140] ns-s-flow-map-entries: after an entry, only `,` or `}`. @@ -3104,7 +3102,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { _ => Expr::init(E::Null {}, mapping_value_start.loc()), }; - props.append_maybe_merge(first_key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(first_key, value, &mut self.alias_expansion_budget)?; } if self.context.get() == Context::FlowIn { @@ -3236,7 +3234,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { } }; - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; } Ok(Expr::init( @@ -3270,8 +3268,6 @@ pub struct MappingProps { } impl MappingProps { - pub const MAX_MERGED_PROPERTIES: usize = 1024 * 1024; - pub fn init() -> Self { Self { list: bun_alloc::AstAlloc::vec(), @@ -3289,8 +3285,18 @@ impl MappingProps { &mut self, merge_props: &[G::Property], budget: &mut usize, - ) -> Result<(), AllocError> { - self.list.reserve(merge_props.len().min(*budget)); + ) -> Result<(), ParseError> { + // The `merge_props` slice may have been produced by an inner `<<:` + // that already expanded an alias, so a chain of inline wrappers + // (`{<<: {<<: ... {<<: *big}}}`) re-materializes the same properties + // at every level without any further alias resolution. Charge each + // copy against the alias-expansion budget so nested merges are + // bounded the same way direct `*alias` references are. + *budget = budget + .checked_sub(merge_props.len()) + .ok_or(ParseError::ExcessiveAliasing)?; + + self.list.reserve(merge_props.len()); while self.merge_indexed < self.list.len() { let idx = self.merge_indexed; @@ -3314,7 +3320,6 @@ impl MappingProps { } } } - *budget = budget.checked_sub(1).ok_or(AllocError)?; // `G::Property` is not `Clone`; reconstruct from its `Copy` fields. self.list.push(G::Property { key: merge_prop.key, @@ -3338,7 +3343,7 @@ impl MappingProps { key: Expr, value: Expr, budget: &mut usize, - ) -> Result<(), AllocError> { + ) -> Result<(), ParseError> { let is_merge_key = match &key.data { ast::ExprData::EString(key_str) => key_str.eql_comptime(b"<<"), _ => false, diff --git a/test/js/bun/yaml/yaml.test.ts b/test/js/bun/yaml/yaml.test.ts index a7242cdb2f7..c2fca3b9e7a 100644 --- a/test/js/bun/yaml/yaml.test.ts +++ b/test/js/bun/yaml/yaml.test.ts @@ -4473,7 +4473,7 @@ test("merging the same large anchor many times completes quickly", () => { expect(elapsed).toBeLessThan(isDebug || isASAN ? 15_000 : 4_000); }, 30_000); -test("limits how many properties merge keys can materialize from a small document", () => { +test("merge keys across many mappings are bounded only by the alias-expansion budget", () => { // A normal merge-key document still resolves. const small = YAML.parse("base: &base\n x: 1\n y: 2\nchild:\n <<: *base\n z: 3\n") as { base: Record; @@ -4481,25 +4481,87 @@ test("limits how many properties merge keys can materialize from a small documen }; expect(small.child).toEqual({ x: 1, y: 2, z: 3 }); - // One anchor with `keyCount` properties merged into `mergeCount` separate - // mappings would materialize keyCount * mergeCount (~1.2 million) property - // entries from a ~30 KB document. The parser caps the total number of - // properties materialized through merge keys and reports an error instead - // of allocating memory proportional to the product. - const keyCount = 2048; - const mergeCount = 600; + // One 64-key anchor merged into 16,500 separate mappings materializes just + // over a million properties from a ~380 KB document. Every `*base` reference + // is already charged against the alias-expansion budget (16M nodes), so the + // parser must accept this document rather than imposing a separate + // per-stream cap on merged properties. + const keyCount = 64; + const mergeCount = 16_500; - const lines: string[] = ["a: &a"]; + const lines: string[] = ["base: &base"]; for (let i = 0; i < keyCount; i++) { lines.push(` k${i}: ${i}`); } + lines.push("out:"); for (let i = 0; i < mergeCount; i++) { - lines.push(`m${i}:`); - lines.push(" <<: *a"); + lines.push(` m${i}:`); + lines.push(" <<: *base"); } const input = lines.join("\n"); - expect(() => YAML.parse(input)).toThrow(); + const parsed = YAML.parse(input) as { + base: Record; + out: Record>; + }; + + expect(Object.keys(parsed.base)).toHaveLength(keyCount); + expect(Object.keys(parsed.out)).toHaveLength(mergeCount); + expect(parsed.out.m0).toEqual(parsed.base); + expect(parsed.out[`m${mergeCount - 1}`]).toEqual(parsed.base); + expect(parsed.out.m0[`k${keyCount - 1}`]).toBe(keyCount - 1); +}, 120_000); + +test("bounds merge-key materialization through nested inline wrappers", () => { + // `{<<: {<<: ... {<<: *big}}}` resolves `*big` once but re-materializes + // every property at each nesting level, so depth * keyCount properties + // are allocated from an input that is linear in depth + keyCount. Each + // copy must be charged against the alias-expansion budget. + function wrap(keyCount: number, depth: number) { + const keys: string[] = []; + for (let i = 0; i < keyCount; i++) keys.push(`k${i}: ${i}`); + let out = "out: "; + for (let i = 0; i < depth; i++) out += "{<<: "; + out += "*big"; + for (let i = 0; i < depth; i++) out += "}"; + return [`big: &big {${keys.join(", ")}}`, out]; + } + + // A reasonable nesting depth still parses and produces the merged result. + const ok = YAML.parse(wrap(100, 50).join("\n") + "\n") as { + big: Record; + out: Record; + }; + expect(Object.keys(ok.out)).toHaveLength(100); + expect(ok.out).toEqual(ok.big); + + // Pre-consume the alias-expansion budget down to a small remainder, then + // show that nested-merge copies are charged against the same budget: 200 + // inline wrappers around a 100-key anchor push it over the limit and are + // rejected rather than silently materializing depth * keyCount properties. + // + // Each `*d` reference walks 837,931 nodes and each `*c` walks 27,931, so + // building a..d plus `pad: [*d x 18, *c x 29]` charges ~16,759,547 of the + // 16,777,216 budget (MAX_ALIAS_EXPANSION), leaving ~17,669 for the nested + // merges. + const width = 30; + const fan = (ref: string) => `[${new Array(width).fill(ref).join(", ")}]`; + const bomb = [ + `a: &a ${fan("0")}`, + `b: &b ${fan("*a")}`, + `c: &c ${fan("*b")}`, + `d: &d ${fan("*c")}`, + `pad: [${new Array(18).fill("*d").concat(new Array(29).fill("*c")).join(", ")}]`, + ]; + // Precondition: the bomb on its own must stay under the budget; if it did + // not, the final assertion could pass because `pad:` threw, not because + // nested merges are charged. Appending an unresolved alias throws + // "Unresolved alias" only if the parser got past `pad:` without exhausting + // the budget (and avoids materializing the bomb to JS on success). + expect(() => YAML.parse(bomb.join("\n") + "\nprobe: *nope\n")).toThrow(/Unresolved alias/); + + const payload = bomb.concat(wrap(100, 200)).join("\n") + "\n"; + expect(() => YAML.parse(payload)).toThrow(/[Ee]xcessive aliasing/); }, 30_000); test("bounds alias expansion for parsed and imported YAML documents", async () => {