From b05b428e3038e983c49b98ea54d45dac20368a52 Mon Sep 17 00:00:00 2001 From: Mihaela Mihaljevic Date: Fri, 26 Jun 2026 15:30:51 +0200 Subject: [PATCH] perf(parsing): linearize entity-graph checks for billion-laughs (#342) * perf(parsing): memoize the markup-producing entity check (billion-laughs) splitAtMarkupEntity / producesMarkup recursed into every reference of every replacement with no memoization, so a nested entity chain was traversed O(references^depth) times: a classic eight-level billion-laughs took ~18s of CPU to reject. The check is now memoized per entity name (the property depends only on the fixed entity graph), so it is O(entities). The same document now rejects in ~0.6s, bounded by the expansion budget. No change to which documents are accepted; cyclic entities still rejected. Found by adversarial probing; closes the no-markup case of #339. * perf(parsing): three-colour DFS for the No-Recursion check checkRecursion validated the entity reference graph with only an in-progress (grey) set and no record of fully-verified entities, so a billion-laughs graph was re-walked O(references^depth) times: a markup-producing eight-level document took ~31s to reject. It is now a three-colour DFS: an entity is added to a black set only after its whole reachable subgraph returns acyclic, so it can never reach a chain node that reaches it, and skipping it on revisit is sound. The walk becomes one pass over the distinct edges (~31s -> ~11s for that document). Cyclic entities are still rejected; no accepted document changes. Part of #339. --- CHANGELOG.md | 4 +++ Sources/Parsing/EntityDecoder.swift | 46 ++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1bb4f7..89f9ade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Rejecting a billion-laughs document is no longer exponential in nesting depth. The "does this entity produce markup" check (`splitAtMarkupEntity` / `producesMarkup`) recursed into every reference of every replacement without memoizing, so a chain `lol8 -> 10x lol7 -> 100x lol6 -> ...` was traversed O(references^depth) times. The check is now memoized per entity name (the property depends only on the fixed entity graph), making it O(entities). A classic eight-level billion-laughs with DTD processing enabled drops from about 18 s to about 0.6 s (now bounded by the expansion budget, not the traversal), with no change to which documents are accepted; a cyclic entity is still rejected. + +- The No-Recursion check (`checkRecursion`) no longer re-traverses shared sub-chains. It validated the reference graph with only an in-progress (grey) set, so a billion-laughs graph was walked O(references^depth) times; a markup-producing eight-level document took about 31 s to reject. It is now a three-colour DFS that records each fully-verified entity (black set) and skips it on revisit, one pass over the distinct edges. The same document drops to about 11 s; cyclic entities are still rejected and no accepted document changes. (The residual cost is the re-injection re-read of the growing remainder on the markup-inclusion path, where `Reader.inject` re-maps the unconsumed tail each cycle; tracked in #339.) + - Entity and content expansion is no longer quadratic in the size of a single expansion. The parser's lookahead buffer consumed from the front with `Array.removeFirst()` and re-injected entity replacement text with `insert(at: 0)`, both O(buffer), so a large internal entity (or a long run pushed back by reference inclusion) was consumed in O(n^2). The buffer now advances a head index, so front-consume is O(1), and re-injection reuses the space the consumed prefix freed, so a single large expansion is linear. An 800000-character internal text entity that previously took quadratic time now parses in about 0.6 s. Behavior, characters, and limits are unchanged; guarded by the standing suite. (A separate quadratic in deeply-nested entity references, where the content scanner re-reads the un-expanded remainder, is tracked as its own issue.) - A complex type with simple content that restricts a base may no longer relax a base `required` attribute to `optional` (XSD 1.0 `cos-ct-derived-ok` / `derivation-ok-restriction.2`). The attribute-use restriction check ran only over `complexContent` restrictions, so a `simpleContent` restriction that relaxed a required attribute (or added a non-base attribute, or widened the attribute wildcard) was accepted; it now runs over both derivation roots. Closes XSTS `particlesZ030_d`, which libxml2 also rejects, so the fix aligns with the reference rather than exceeding it. XSTS invalid-schemas-accepted 10 -> 9 with valid-schemas-rejected held at 0 (full archive). Guarded by `SimpleContentAttrRestrictionTests`. diff --git a/Sources/Parsing/EntityDecoder.swift b/Sources/Parsing/EntityDecoder.swift index cbb9a65..2b7cbbe 100644 --- a/Sources/Parsing/EntityDecoder.swift +++ b/Sources/Parsing/EntityDecoder.swift @@ -69,11 +69,12 @@ extension PureXML.Parsing.EntityDecoder { /// plain decoding suffices. static func splitAtMarkupEntity(_ raw: String, entities: [String: String]) -> MarkupSplit? { guard !entities.isEmpty else { return nil } + var markupCache: [String: Bool] = [:] var index = raw.startIndex while index < raw.endIndex, let amp = raw[index...].firstIndex(of: "&") { guard let semicolon = raw[amp...].firstIndex(of: ";") else { return nil } let name = String(raw[raw.index(after: amp) ..< semicolon]) - if !name.hasPrefix("#"), !predefined.contains(name), producesMarkup(name, entities: entities, visiting: []) { + if !name.hasPrefix("#"), !predefined.contains(name), producesMarkup(name, entities: entities, cache: &markupCache) { return MarkupSplit( prefix: String(raw[..) -> Bool { - guard !visiting.contains(name), let replacement = entities[name] else { return false } - if replacement.contains("<") { return true } - return declaredReferences(in: replacement).contains { - producesMarkup($0, entities: entities, visiting: visiting.union([name])) + /// Memoized per entity name: the property depends only on the (fixed) entity + /// graph, so a deeply-nested reference chain resolves in O(entities) rather + /// than O(references^depth), which a billion-laughs document otherwise forces. + /// A tentative `false` recorded before the recursion breaks any cycle (a + /// cyclic entity is an error caught when the reference is expanded). + private static func producesMarkup(_ name: String, entities: [String: String], cache: inout [String: Bool]) -> Bool { + if let cached = cache[name] { return cached } + guard let replacement = entities[name] else { + cache[name] = false + return false } + cache[name] = false + let result = replacement.contains("<") + || declaredReferences(in: replacement).contains { producesMarkup($0, entities: entities, cache: &cache) } + cache[name] = result + return result } private static func checkRecursion( @@ -123,13 +134,32 @@ extension PureXML.Parsing.EntityDecoder { entities: [String: String], visiting: Set, at mark: PureXML.Parsing.Mark, + ) throws { + var verified: Set = [] + try checkRecursion(name, entities: entities, visiting: visiting, verified: &verified, at: mark) + } + + /// Three-colour DFS for the No-Recursion WFC. `visiting` is the grey set + /// (entities on the current reference chain; revisiting one is the cycle). + /// `verified` is the black set: an entity is added only after its whole + /// reachable subgraph returns acyclic, so it can never reach a chain node + /// that reaches it. Skipping a black entity is therefore sound and collapses + /// the otherwise O(references^depth) revisits of a billion-laughs graph to + /// one pass over the distinct edges. + private static func checkRecursion( + _ name: String, + entities: [String: String], + visiting: Set, + verified: inout Set, + at mark: PureXML.Parsing.Mark, ) throws { guard let replacement = entities[name] else { return } - for reference in declaredReferences(in: replacement) { + for reference in declaredReferences(in: replacement) where !verified.contains(reference) { guard !visiting.contains(reference) else { throw PureXML.Parsing.ParseError.recursiveEntity(name: reference, mark) } - try checkRecursion(reference, entities: entities, visiting: visiting.union([reference]), at: mark) + try checkRecursion(reference, entities: entities, visiting: visiting.union([reference]), verified: &verified, at: mark) + verified.insert(reference) } }