From f4982219a6d392319fdf83bf8bc02452ecdc64da Mon Sep 17 00:00:00 2001 From: Mihaela Mihaljevic Date: Sat, 27 Jun 2026 16:55:03 +0200 Subject: [PATCH] perf(xinclude): make XInclude processing depth-safe (#354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PureXML.XInclude.process` rebuilt the tree recursively (`process → processElement → process`), so a deeply-nested document overflowed the stack one native frame per element level, even with no includes present. It now drives an explicit work stack: each branch gathers its processed children into a reference accumulator, then a deferred build step assembles the node. The `xml:base`, inclusion-chain depth (bounded by `maxDepth`), and cycle-detection visited set thread down per context exactly as before; an include splices its (or its fallback's) content in place by pushing the loaded nodes back onto the stack. Behavior is unchanged: the XInclude/XPointer conformance corpus and the fallback, xpointer, cycle, and xml:base cases all pass. A 50000-deep document, with or without an include at its base, now processes without crashing, guarded by `XIncludeDepthSafetyTests`. Part of #350. --- CHANGELOG.md | 2 + Sources/XInclude/XIncludeProcessor.swift | 129 ++++++++++++++++++----- Tests/XIncludeDepthSafetyTests.swift | 69 ++++++++++++ 3 files changed, 171 insertions(+), 29 deletions(-) create mode 100644 Tests/XIncludeDepthSafetyTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 106f75f..a16b177 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Serializing a tree as HTML (`PureXML.HTML.serialize`, the html output method's serializer) no longer recurses on the tree's depth, so it no longer overflows the stack on a deeply-nested tree (as shallow as a couple of thousand deep on a Swift Task stack). The serializer now drives a deferred-close work stack (the same shape as the XML serializer and `Canonicalizer`): an element pushes a close step then its children reversed, and a raw-text element (`script`, `style`) flags its direct text children to emit verbatim. The output is byte-identical: the void-element, raw-text, comment, and escaping semantics are unchanged, with the HTML conformance corpus passing. A 50000-deep tree now serializes without crashing. (#350) +- Processing `xi:include` elements (`PureXML.XInclude.process`) no longer recurses on the tree's depth, so it no longer overflows the stack on a deeply-nested document, even one with no includes (as shallow as a couple of thousand deep on a Swift Task stack). The pass now drives an explicit work stack: each branch gathers its processed children into a reference accumulator, then a deferred build step assembles the node, while the `xml:base`, inclusion-chain depth, and cycle-detection visited set thread down exactly as before. An include splices its (or its fallback's) content in place by pushing the loaded nodes back onto the stack. Behavior is unchanged: the XInclude/XPointer conformance corpus and the fallback, xpointer, cycle, and `xml:base` cases pass. A 50000-deep document, with or without an include at its base, now processes without crashing. (#350) + - Canonicalizing a document (`Canonicalizer`, the C14N form used for XML signatures) no longer recurses on the document's depth, so it no longer overflows the stack on a deeply-nested document (it crashed around 5000 deep). The serializer now drives a deferred-close work stack that threads each element's namespace context to its children. The canonical bytes are unchanged: the C14N 1.0/1.1 and exclusive conformance corpora pass. (#350) - Applying DTD attribute defaults to a document (`parseApplyingInternalDTDDefaults`, and the defaults pass behind DTD validation) no longer recurses on the document's depth, so it no longer overflows the stack on a deeply-nested document. The tree is rebuilt bottom-up through a new shared iterative spine (`Node.rebuildingBottomUp`), which the other tree-to-tree transforms will adopt. (#350) diff --git a/Sources/XInclude/XIncludeProcessor.swift b/Sources/XInclude/XIncludeProcessor.swift index 65d6777..61d772c 100644 --- a/Sources/XInclude/XIncludeProcessor.swift +++ b/Sources/XInclude/XIncludeProcessor.swift @@ -1,6 +1,31 @@ /// One XInclude pass over a value tree. File-scope and private: an internal /// detail of ``PureXML/XInclude``. Operates on the immutable ``PureXML/Model/Node`` /// tree, returning a new tree with `xi:include` elements replaced. +/// The in-scope context threaded down an XInclude pass: the `xml:base` for +/// resolving hrefs, the inclusion-chain `depth` (bounded by `maxDepth`), and the +/// `visited` resources that detect a cycle. +private struct XIncludeContext { + let base: String + let depth: Int + let visited: Set +} + +/// A mutable bag of processed nodes a branch's children gather into, so the +/// deferred build step assembles the parent once they are done. +private final class XIncludeAccumulator { + var nodes: [PureXML.Model.Node] = [] +} + +/// One unit of XInclude work: visit a source node (appending its processed result +/// to `target`), or, after a branch's children are gathered, build the element or +/// document and append it to `target`. File-scoped to keep the work stack off the +/// type-nesting depth. +private enum XIncludeWork { + case visit(PureXML.Model.Node, XIncludeContext, XIncludeAccumulator) + case buildElement(PureXML.Model.Element, XIncludeAccumulator, XIncludeAccumulator) + case buildDocument(XIncludeAccumulator, XIncludeAccumulator) +} + private struct XIncludeRun { typealias Node = PureXML.Model.Node typealias Element = PureXML.Model.Element @@ -11,49 +36,93 @@ private struct XIncludeRun { let maxDepth = 30 let namespace = "http://www.w3.org/2001/XInclude" - func process(_ node: Node, base: String, depth: Int, visited: Set) throws -> [Node] { + /// Rebuilds the tree with `xi:include` elements replaced, without recursing on + /// the tree's depth: an explicit work stack drives a post-order rebuild so a + /// deeply-nested document does not overflow the stack. Each branch gathers its + /// processed children into a reference accumulator, then a deferred build step + /// assembles the node once those children are done. `base` (the in-scope + /// `xml:base`) threads down per element; `depth` (the inclusion-chain depth, + /// bounded by `maxDepth`) and `visited` (the cycle set) advance only across an + /// include boundary, exactly as the recursive form did. + func process(_ root: Node, base: String) throws -> [Node] { + let rootResult = XIncludeAccumulator() + var stack: [XIncludeWork] = [.visit(root, XIncludeContext(base: base, depth: 0, visited: []), rootResult)] + while let work = stack.popLast() { + switch work { + case let .visit(node, context, target): + try visit(node, context, into: target, stack: &stack) + case let .buildElement(element, children, target): + target.nodes.append(.element(Element(name: element.name, attributes: element.attributes, children: children.nodes))) + case let .buildDocument(children, target): + target.nodes.append(.document(children.nodes)) + } + } + return rootResult.nodes + } + + /// Processes one node into `target`. A branch pushes its build step first, then + /// its children reversed, so the children pop in document order and the build + /// runs after them. + private func visit(_ node: Node, _ context: XIncludeContext, into target: XIncludeAccumulator, stack: inout [XIncludeWork]) throws { switch node { case let .document(children): - try [.document(children.flatMap { try process($0, base: base, depth: depth, visited: visited) })] + let gathered = XIncludeAccumulator() + stack.append(.buildDocument(gathered, target)) + for child in children.reversed() { + stack.append(.visit(child, context, gathered)) + } case let .element(element): - try processElement(element, base: base, depth: depth, visited: visited) + if isInclude(element) { + try resolveInclude(element, context, into: target, stack: &stack) + } else { + let elementBase = baseURI(of: element, base: context.base) + let childContext = XIncludeContext(base: elementBase, depth: context.depth, visited: context.visited) + let gathered = XIncludeAccumulator() + stack.append(.buildElement(element, gathered, target)) + for child in element.children.reversed() { + stack.append(.visit(child, childContext, gathered)) + } + } default: - [node] + target.nodes.append(node) } } - private func processElement(_ element: Element, base: String, depth: Int, visited: Set) throws -> [Node] { - if isInclude(element) { - return try resolveInclude(element, base: base, depth: depth, visited: visited) - } - let elementBase = baseURI(of: element, base: base) - let children = try element.children.flatMap { try process($0, base: elementBase, depth: depth, visited: visited) } - return [.element(Element(name: element.name, attributes: element.attributes, children: children))] - } - - private func resolveInclude(_ element: Element, base: String, depth: Int, visited: Set) throws -> [Node] { - guard depth < maxDepth else { throw PureXML.XInclude.XIncludeError.toodeep } - let elementBase = baseURI(of: element, base: base) + /// Replaces an `xi:include` with its target, pushing the included (or fallback) + /// content onto the stack to be processed into `target` in place. The included + /// content advances the inclusion chain (`depth + 1`, base reset to the + /// resource, the resource added to `visited`); fallback content stays in the + /// include's own context, matching the recursive form. + private func resolveInclude(_ element: Element, _ context: XIncludeContext, into target: XIncludeAccumulator, stack: inout [XIncludeWork]) throws { + guard context.depth < maxDepth else { throw PureXML.XInclude.XIncludeError.toodeep } + let elementBase = baseURI(of: element, base: context.base) guard let href = attribute(element, "href") else { - return try fallback(element, base: base, depth: depth, visited: visited, error: .missingHref) + try pushFallback(element, context, into: target, error: .missingHref, stack: &stack) + return } let resolved = PureXML.XInclude.URIReference.resolve(href, against: elementBase) let isText = attribute(element, "parse") == "text" let xpointer = attribute(element, "xpointer") if isText, xpointer != nil { throw PureXML.XInclude.XIncludeError.textWithFragment } // A resource already on its own inclusion chain is a cycle. - guard !visited.contains(resolved) else { throw PureXML.XInclude.XIncludeError.cycle(resolved) } + guard !context.visited.contains(resolved) else { throw PureXML.XInclude.XIncludeError.cycle(resolved) } guard let content = load(request(element, uri: resolved, isText: isText)) else { - return try fallback(element, base: base, depth: depth, visited: visited, error: .unresolved(href)) + try pushFallback(element, context, into: target, error: .unresolved(href), stack: &stack) + return } if isText { - return [.text(content)] + target.nodes.append(.text(content)) + return } guard let parsed = try? PureXML.parse(content) else { - return try fallback(element, base: base, depth: depth, visited: visited, error: .unresolved(href)) + try pushFallback(element, context, into: target, error: .unresolved(href), stack: &stack) + return } let selected = selectedNodes(from: parsed, xpointer: xpointer) - return try selected.flatMap { try process($0, base: resolved, depth: depth + 1, visited: visited.union([resolved])) } + let includedContext = XIncludeContext(base: resolved, depth: context.depth + 1, visited: context.visited.union([resolved])) + for node in selected.reversed() { + stack.append(.visit(node, includedContext, target)) + } } private func request(_ element: Element, uri: String, isText: Bool) -> Request { @@ -88,19 +157,21 @@ private struct XIncludeRun { } } - private func fallback( + private func pushFallback( _ element: Element, - base: String, - depth: Int, - visited: Set, + _ context: XIncludeContext, + into target: XIncludeAccumulator, error: PureXML.XInclude.XIncludeError, - ) throws -> [Node] { + stack: inout [XIncludeWork], + ) throws { guard let fallback = element.children.first(where: isFallback), case let .element(fallbackElement) = fallback else { throw error } - return try fallbackElement.children.flatMap { try process($0, base: base, depth: depth, visited: visited) } + for child in fallbackElement.children.reversed() { + stack.append(.visit(child, context, target)) + } } private func baseURI(of element: Element, base: String) -> String { @@ -163,7 +234,7 @@ public extension PureXML.XInclude { loading: @escaping (_ request: XIncludeRequest) -> String?, ) throws -> PureXML.Model.Node { let run = XIncludeRun(load: loading) - return try run.process(node, base: base, depth: 0, visited: []).first ?? node + return try run.process(node, base: base).first ?? node } /// Parses `xml` and processes its `xi:include` elements with a diff --git a/Tests/XIncludeDepthSafetyTests.swift b/Tests/XIncludeDepthSafetyTests.swift new file mode 100644 index 0000000..a119629 --- /dev/null +++ b/Tests/XIncludeDepthSafetyTests.swift @@ -0,0 +1,69 @@ +import Testing +@testable import PureXML + +/// XInclude processing must not recurse on a tree's depth (#350). The pass +/// rebuilt the tree recursively, so a deeply-nested document overflowed the +/// stack (as shallow as a couple of thousand deep on a Swift Task stack), even +/// with no includes present. It now drives an explicit work stack; these tests +/// reach depths the recursive form could not survive, for both a plain tree and +/// a deep included resource. +@Suite("XInclude depth safety") +struct XIncludeDepthSafetyTests { + private func deepDocument(_ depth: Int, root: String, leaf: String) -> PureXML.Model.Node { + var node: PureXML.Model.Node = .text(leaf) + for _ in 0 ..< depth { + node = .element(.init("a", children: [node])) + } + return .document([.element(.init(root, children: [node]))]) + } + + /// The nesting depth of the first-child chain and the deepest text leaf. + private func descend(_ node: PureXML.Model.Node) -> (depth: Int, leaf: String?) { + var current = node + var depth = 0 + while true { + switch current { + case let .document(children): + guard let first = children.first else { return (depth, nil) } + current = first + case let .element(element): + guard let first = element.children.first else { return (depth, nil) } + depth += 1 + current = first + case let .text(value), let .cdata(value): + return (depth, value) + default: + return (depth, nil) + } + } + } + + @Test("processing a 50k-deep document with no includes does not overflow") + func test_deepPlainTreeIsBounded() throws { + let document = deepDocument(50000, root: "doc", leaf: "leaf") + let result = try PureXML.XInclude.process(document, loadingURI: { _ in nil }) + // root plus 50000 , with the leaf preserved. + let descended = descend(result) + #expect(descended.depth == 50001) + #expect(descended.leaf == "leaf") + } + + @Test("an include at the bottom of a deep tree resolves without overflowing") + func test_includeDeepInTreeIsBounded() throws { + // A 50k-deep chain whose innermost child is an xi:include, so the include + // is resolved at the bottom of the iterative walk. + let depth = 50000 + var node: PureXML.Model.Node = .element(.init("xi:include", attributes: [.init("href", "x.xml")])) + for _ in 0 ..< depth { + node = .element(.init("a", children: [node])) + } + let document = PureXML.Model.Node.document([.element(.init("doc", children: [node]))]) + let result = try PureXML.XInclude.process(document, loadingURI: { uri in + uri == "x.xml" ? "hi" : nil + }) + let descended = descend(result) + // -> 50000 -> spliced -> "hi". + #expect(descended.depth == depth + 2) + #expect(descended.leaf == "hi") + } +}