NodetteHQ/PureXML
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

perf(parsing): linearize entity-graph checks for billion-laughs (#342)

Browse Source 
* 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.
This commit is contained in:
Mihaela Mihaljevic 2026-06-26 15:30:51 +02:00 committed by GitHub
parent 46a5c27e8c
commit b05b428e30
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 42 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@ -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`.

46
Sources/Parsing/EntityDecoder.swift
View File

@ -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[..<amp]),
name: name,
@ -110,12 +111,22 @@ extension PureXML.Parsing.EntityDecoder {
}
/// Whether `name`'s replacement contains markup, directly or transitively.
private static func producesMarkup(_ name: String, entities: [String: String], visiting: Set<String>) -> 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<String>,
at mark: PureXML.Parsing.Mark,
) throws {
var verified: Set<String> = []
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<String>,
verified: inout Set<String>,
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)
}
}