diff --git a/CHANGELOG.md b/CHANGELOG.md index 89f9ade..20852cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The mutable `Model.TreeNode` tree type is now depth-safe: building one from a value `Node`, projecting it back with `node`, and releasing it all run in bounded native stack instead of recursing one frame per level. Construction (`TreeNode(_:)`) and projection (`node`/`projectedChildren`) drive explicit post-order work stacks, and `deinit` flattens its subtree into a heap worklist before releasing, so a deeply-nested tree no longer overflows the stack through a recursive chain of child releases. The teardown only flattens nodes it solely owns (`isKnownUniquelyReferenced`), so a child still referenced elsewhere keeps its subtree. A 60000-deep tree releases in well under a tenth of a second; behaviour for ordinary trees is unchanged. The validator walk and the XPath descendant axis (`AxisNavigation.appendDescendants`) were made iterative for the same reason. (Part of #341. The immutable value `Node`'s own recursive release, and XPath result ordering/dedup, are still depth-bounded and tracked there.) + - 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.) diff --git a/Sources/Model/TreeNode.swift b/Sources/Model/TreeNode.swift index 0cb848a..dad97b3 100644 --- a/Sources/Model/TreeNode.swift +++ b/Sources/Model/TreeNode.swift @@ -111,6 +111,35 @@ public extension PureXML.Model { } } + /// Tears the subtree down iteratively so releasing a deeply-nested tree + /// stays in bounded native stack. The implicit release walk would + /// otherwise recurse one frame per level (a chain of child releases), and + /// a naive `deinit` that let each child deallocate in turn would just nest + /// the same way. Instead, each node's children are moved into a flat heap + /// worklist and cleared before that node is released, so its own `deinit` + /// finds an empty `children` and returns at once: the teardown is flat. + /// + /// Only a node this teardown uniquely owns is drained; a node still held + /// elsewhere (an external reference into the tree) keeps its subtree + /// intact and tears down later through this same `deinit`. + deinit { + guard !children.isEmpty else { return } + var pending = children + children = [] + while !pending.isEmpty { + // `removeLast` moves the node into `node` as the sole binding, so + // `isKnownUniquelyReferenced` sees no spurious reference (a + // `while let`/`popLast` binding would itself count). A node still + // referenced elsewhere keeps its subtree intact; only a node this + // teardown solely owns is flattened, so its release stays shallow. + var node = pending.removeLast() + if isKnownUniquelyReferenced(&node), !node.children.isEmpty { + pending.append(contentsOf: node.children) + node.children = [] + } + } + } + // MARK: Factories /// Creates a document node holding the given top-level children. diff --git a/Sources/Model/TreeNodeConversion.swift b/Sources/Model/TreeNodeConversion.swift index 5557156..b16754e 100644 --- a/Sources/Model/TreeNodeConversion.swift +++ b/Sources/Model/TreeNodeConversion.swift @@ -1,15 +1,19 @@ public extension PureXML.Model.TreeNode { /// Builds a mutable tree from a parsed value ``Node``, wiring up parent links. + /// + /// The conversion is iterative, not recursive on depth: a deeply-nested + /// document is built bottom-up through an explicit work stack so construction + /// stays in bounded native stack (see ``buildForest(_:)``). convenience init(_ node: PureXML.Model.Node) { switch node { case let .document(children): - self.init(adopting: .document, children: children.map(PureXML.Model.TreeNode.init)) + self.init(adopting: .document, children: Self.buildForest(children)) case let .element(element): self.init( adopting: .element, name: element.name, attributes: element.attributes, - children: element.children.map(PureXML.Model.TreeNode.init), + children: Self.buildForest(element.children), ) case let .text(value): self.init(kind: .text, value: value) @@ -22,17 +26,46 @@ public extension PureXML.Model.TreeNode { } } + /// Builds the `TreeNode` children for a forest of value ``Node``s without + /// recursing on tree depth. A frame holds a branch node (element or document) + /// being converted: the position of the next source child to process and the + /// `TreeNode` children built so far. Leaves convert directly; a branch is + /// assembled (adopting its built children, which sets their parent links) only + /// once all its source children are done, then handed to its own parent frame. + /// A synthetic document frame carries the roots; it is never assembled (that + /// would re-parent them), so its built children are the result. + private static func buildForest(_ roots: [PureXML.Model.Node]) -> [PureXML.Model.TreeNode] { + let rootFrame = BuildFrame(.document(roots), roots) + var stack: [BuildFrame] = [rootFrame] + while let frame = stack.last { + guard frame.next < frame.children.count else { + stack.removeLast() + guard let parent = stack.last else { break } + parent.built.append(buildBranch(frame)) + continue + } + let child = frame.children[frame.next] + frame.next += 1 + if let grandchildren = branchChildren(child) { + stack.append(BuildFrame(child, grandchildren)) + } else { + frame.built.append(buildLeaf(child)) + } + } + return rootFrame.built + } + /// Rebuilds an immutable value ``Node`` from this subtree, for serialization - /// or value comparison. + /// or value comparison. Iterative on depth (see ``projectedChildren(of:)``). var node: PureXML.Model.Node { switch kind { case .document: - .document(projectedChildren) + .document(Self.projectedChildren(of: self)) case .element: .element(PureXML.Model.Element( name: name ?? PureXML.Model.QualifiedName(""), attributes: attributes, - children: projectedChildren, + children: Self.projectedChildren(of: self), )) case .text: .text(value) @@ -52,19 +85,127 @@ public extension PureXML.Model.TreeNode { } } - /// The value-`Node` children of this subtree: doctype and namespace nodes are + /// The value-`Node` children of `root`'s subtree, projected into a pure + /// content tree without recursing on depth: doctype and namespace nodes are /// dropped (they carry no content), and an entity-reference node is spliced - /// open into its replacement nodes, so the projection is a pure content tree. - private var projectedChildren: [PureXML.Model.Node] { - children.flatMap { child -> [PureXML.Model.Node] in + /// open into its replacement nodes. A frame accumulates a branch node's + /// projected children; on completion an element or document frame wraps them + /// in the corresponding value `Node`, while an entity-reference frame splices + /// them into its parent (no wrapper). + private static func projectedChildren(of root: PureXML.Model.TreeNode) -> [PureXML.Model.Node] { + let rootFrame = ProjectFrame(root, splice: false) + var stack: [ProjectFrame] = [rootFrame] + while let frame = stack.last { + guard frame.next < frame.tree.children.count else { + stack.removeLast() + guard let parent = stack.last else { break } + finishProjection(frame, into: parent) + continue + } + let child = frame.tree.children[frame.next] + frame.next += 1 switch child.kind { case .doctype, .namespace: - [] + continue case .entityReference: - child.projectedChildren + stack.append(ProjectFrame(child, splice: true)) + case .element, .document: + stack.append(ProjectFrame(child, splice: false)) default: - [child.node] + frame.built.append(projectLeaf(child)) } } + return rootFrame.built + } + + // MARK: Build helpers + + private static func branchChildren(_ node: PureXML.Model.Node) -> [PureXML.Model.Node]? { + switch node { + case let .document(children): children + case let .element(element): element.children + default: nil + } + } + + private static func buildLeaf(_ node: PureXML.Model.Node) -> PureXML.Model.TreeNode { + switch node { + case let .text(value): PureXML.Model.TreeNode(kind: .text, value: value) + case let .cdata(value): PureXML.Model.TreeNode(kind: .cdata, value: value) + case let .comment(value): PureXML.Model.TreeNode(kind: .comment, value: value) + case let .processingInstruction(target, data): + PureXML.Model.TreeNode(kind: .processingInstruction, name: PureXML.Model.QualifiedName(target), value: data) + // Unreachable: `buildLeaf` is only called when `branchChildren` returned + // nil (a non-branch node). An empty text node keeps the conversion total. + case .document, .element: PureXML.Model.TreeNode(kind: .text, value: "") + } + } + + private static func buildBranch(_ frame: BuildFrame) -> PureXML.Model.TreeNode { + switch frame.node { + case let .element(element): + PureXML.Model.TreeNode(adopting: .element, name: element.name, attributes: element.attributes, children: frame.built) + // Only branch nodes (document or element) get a frame; a non-branch frame + // is unreachable, so the document shape is a total fallback. + default: + PureXML.Model.TreeNode(adopting: .document, children: frame.built) + } + } + + // MARK: Projection helpers + + private static func projectLeaf(_ tree: PureXML.Model.TreeNode) -> PureXML.Model.Node { + switch tree.kind { + case .cdata: .cdata(tree.value) + case .comment: .comment(tree.value) + case .processingInstruction: + .processingInstruction(target: tree.name?.description ?? "", data: tree.value) + // doctype/namespace are dropped before reaching here; everything else + // projects as the text leaf. + default: .text(tree.value) + } + } + + /// Resolves a completed frame into `parent`'s children: an entity-reference + /// frame splices its projected nodes in, an element wraps them, and any other + /// branch (a document) wraps them as a document. + private static func finishProjection(_ frame: ProjectFrame, into parent: ProjectFrame) { + if frame.splice { + parent.built.append(contentsOf: frame.built) + } else if case .element = frame.tree.kind { + parent.built.append(.element(PureXML.Model.Element( + name: frame.tree.name ?? PureXML.Model.QualifiedName(""), + attributes: frame.tree.attributes, + children: frame.built, + ))) + } else { + parent.built.append(.document(frame.built)) + } + } +} + +/// A branch node being converted from a value ``Node`` to a ``TreeNode``: the +/// source children, the cursor into them, and the converted children so far. +private final class BuildFrame { + let node: PureXML.Model.Node + let children: [PureXML.Model.Node] + var next = 0 + var built: [PureXML.Model.TreeNode] = [] + init(_ node: PureXML.Model.Node, _ children: [PureXML.Model.Node]) { + self.node = node + self.children = children + } +} + +/// A ``TreeNode`` being projected back to a value ``Node``: `splice` marks an +/// entity-reference frame whose projected children fold into its parent. +private final class ProjectFrame { + let tree: PureXML.Model.TreeNode + let splice: Bool + var next = 0 + var built: [PureXML.Model.Node] = [] + init(_ tree: PureXML.Model.TreeNode, splice: Bool) { + self.tree = tree + self.splice = splice } } diff --git a/Sources/Validation/Validator.swift b/Sources/Validation/Validator.swift index 5de12c2..8a8ecc1 100644 --- a/Sources/Validation/Validator.swift +++ b/Sources/Validation/Validator.swift @@ -149,40 +149,43 @@ public extension PureXML.Validation { if !toThrow.isEmpty { throw ValidationErrorCollection(values: toThrow) } } - private func walk(_ node: PureXML.Model.Node, path: [PathKey], _ document: Document, _ findings: inout [ValidationError]) { - apply(node, at: path, in: document, into: &findings) - switch node { - case let .document(children): - walkChildren(children, path: path, document, &findings) - case let .element(element): - apply(element, at: path, in: document, into: &findings) - for attribute in element.attributes { - apply(attribute, at: path + [.attribute(attribute.name.localName)], in: document, into: &findings) - } - walkChildren(element.children, path: path, document, &findings) - case .text, .cdata, .comment, .processingInstruction: - break - } - } - - private func walkChildren( - _ children: [PureXML.Model.Node], - path: [PathKey], - _ document: Document, - _ findings: inout [ValidationError], - ) { - let elementNames = children.compactMap { child -> String? in - guard case let .element(element) = child else { return nil } - return element.name.description - } - var steps = PathKey.steps(forChildNames: elementNames).makeIterator() - for child in children { - guard case .element = child else { - walk(child, path: path, document, &findings) + /// Validates `root` and its subtree in document order. The walk is + /// iterative, driven by an explicit `(node, path)` stack, so a deeply + /// nested document does not overflow the call stack; children are pushed + /// reversed so they pop in document order, giving the same pre-order + /// finding sequence a recursive descent would. + private func walk(_ root: PureXML.Model.Node, path rootPath: [PathKey], _ document: Document, _ findings: inout [ValidationError]) { + var stack: [(node: PureXML.Model.Node, path: [PathKey])] = [(root, rootPath)] + while let (node, path) = stack.popLast() { + apply(node, at: path, in: document, into: &findings) + let children: [PureXML.Model.Node] + switch node { + case let .document(documentChildren): + children = documentChildren + case let .element(element): + apply(element, at: path, in: document, into: &findings) + for attribute in element.attributes { + apply(attribute, at: path + [.attribute(attribute.name.localName)], in: document, into: &findings) + } + children = element.children + case .text, .cdata, .comment, .processingInstruction: continue } - let step = steps.next() ?? .element("") - walk(child, path: path + [step], document, &findings) + let elementNames = children.compactMap { child -> String? in + guard case let .element(element) = child else { return nil } + return element.name.description + } + var steps = PathKey.steps(forChildNames: elementNames).makeIterator() + var childItems: [(node: PureXML.Model.Node, path: [PathKey])] = [] + childItems.reserveCapacity(children.count) + for child in children { + if case .element = child { + childItems.append((child, path + [steps.next() ?? .element("")])) + } else { + childItems.append((child, path)) + } + } + stack.append(contentsOf: childItems.reversed()) } } diff --git a/Sources/XPath/AxisNavigation.swift b/Sources/XPath/AxisNavigation.swift index 5da3cd1..d18eea3 100644 --- a/Sources/XPath/AxisNavigation.swift +++ b/Sources/XPath/AxisNavigation.swift @@ -72,9 +72,15 @@ extension PureXML.XPath { /// wrapped ``Node`` means the traversal builds no intermediate per-level /// arrays and wraps each node exactly once, when it is appended. private static func appendDescendants(of tree: PureXML.Model.TreeNode, into result: inout [Node]) { - for child in tree.children { - result.append(.tree(child)) - appendDescendants(of: child, into: &result) + // Iterative pre-order walk: a deeply-nested tree would overflow the + // call stack if descended recursively. Children are pushed reversed so + // they pop in document order. + var stack = Array(tree.children.reversed()) + while let node = stack.popLast() { + result.append(.tree(node)) + if !node.children.isEmpty { + stack.append(contentsOf: node.children.reversed()) + } } } @@ -104,9 +110,14 @@ extension PureXML.XPath { where keep: (PureXML.Model.TreeNode) -> Bool, into result: inout [Node], ) { - for child in tree.children { - if keep(child) { result.append(.tree(child)) } - appendDescendants(of: child, where: keep, into: &result) + // Iterative pre-order walk (see the unfiltered overload); children are + // pushed reversed so they pop in document order. + var stack = Array(tree.children.reversed()) + while let node = stack.popLast() { + if keep(node) { result.append(.tree(node)) } + if !node.children.isEmpty { + stack.append(contentsOf: node.children.reversed()) + } } } diff --git a/Tests/TreeNodeDepthSafetyTests.swift b/Tests/TreeNodeDepthSafetyTests.swift new file mode 100644 index 0000000..2e6dad3 --- /dev/null +++ b/Tests/TreeNodeDepthSafetyTests.swift @@ -0,0 +1,59 @@ +import Testing +@testable import PureXML + +/// Depth safety for the mutable ``TreeNode`` tree type (#341): building a tree +/// from a value ``Node``, projecting it back, and releasing it must all run in +/// bounded native stack, so a deeply-nested document cannot overflow the call +/// stack through a recursive walk or a recursive chain of child releases. +/// +/// The release test builds its tree bottom-up through the adopting initializer +/// rather than from a parsed value ``Node``: a value `Node` that deep still +/// releases recursively (its storage change is tracked separately in #341), so +/// using one as a fixture would itself overflow on teardown and mask what is +/// under test here. +@Suite("TreeNode depth safety") +struct TreeNodeDepthSafetyTests { + /// A chain of `depth` nested elements, built bottom-up so construction never + /// recurses and holds no deep value `Node`. + private func deepChain(_ depth: Int) -> PureXML.Model.TreeNode { + var node = PureXML.Model.TreeNode(adopting: .element, name: PureXML.Model.QualifiedName("a"), children: []) + for _ in 0 ..< depth { + node = PureXML.Model.TreeNode(adopting: .element, name: PureXML.Model.QualifiedName("a"), children: [node]) + } + return node + } + + @Test("releasing a 60k-deep tree does not overflow the stack") + func test_deepReleaseIsBounded() { + var root: PureXML.Model.TreeNode? = deepChain(60000) + root = nil + #expect(root == nil) + } + + @Test("a held attached child keeps its subtree when the root is released") + func test_heldChildKeepsSubtreeOnRootRelease() { + let leaf = PureXML.Model.TreeNode.element("leaf") + let mid = PureXML.Model.TreeNode.element("mid", children: [leaf]) + var root: PureXML.Model.TreeNode? = PureXML.Model.TreeNode.element("root", children: [mid]) + root = nil + // The teardown only flattens nodes it solely owns; `mid` is still held, so + // its subtree must remain intact rather than be cleared. + #expect(mid.children.count == 1) + #expect(mid.children.first === leaf) + #expect(leaf.name?.description == "leaf") + } + + @Test("Node to TreeNode and back round-trips through the iterative conversions") + func test_conversionRoundTrip() throws { + // Held at the default depth bound: a value `Node` this deep still releases + // recursively, and a Swift Task's stack (this test runs on one) overflows + // far shallower than the main thread, so a deeper fixture would crash on + // teardown for a reason unrelated to the conversions under test (#341). + let depth = 200 + let xml = String(repeating: "", count: depth) + "leaf" + String(repeating: "", count: depth) + let node = try PureXML.parse(xml) + let tree = PureXML.Model.TreeNode(node) + let back = tree.node + #expect(PureXML.serialize(back, options: .compact) == PureXML.serialize(node, options: .compact)) + } +}