From 9cce69509017ae6261865b9827e1fe890d5f6b6c Mon Sep 17 00:00:00 2001 From: Mihaela Mihaljevic Date: Sun, 28 Jun 2026 19:53:10 +0200 Subject: [PATCH] fix(schema): reject a repeating sequence's trailing/leading particle overlap as a UPA violation (#368) A repeating model whose optional trailing particle and leading particle share a name, (c, b?, c?)*, is ambiguous, as libxml2/Xerces reject it: after b the next c could be the trailing c? or the looped leading c, two distinct particles. The compositional determinism check formed the internal sequence gap before the enclosing repetition's loop edge was added, so the combined {trailing, leading} followpos was never checked, an invalid-schemas- accepted false negative the finite XSTS corpus did not contain. The sequence now carries each trailing end-item's internal suffix in its lastFollow, so a parent gap or the loop edge (withRepeatEdge) forms the end-item's full followpos. The carried suffix is always a subset of the true followpos, so the check only catches more genuine conflicts and never over-rejects: the non-repeated (c, b?, c?) and disjoint shapes like (a, b?)* stay valid. Found by the new generated-schema determinism differential against libxml2. Verified over the full W3C XSTS corpus in release (valid-schemas-rejected/valid-instances-rejected stay 0, invalid-*-accepted unchanged at 8/14), full suite on macOS/Linux/WASI, and an adversarial over-rejection critic finding no valid schema newly rejected over 47 shapes. The related ambiguity through two references to one group in a repeating sequence stays an under-rejection (the cached group identity cannot distinguish a looped from a trailing occurrence without per-occurrence tracking), named in DeterminismDifferentialTests. --- CHANGELOG.md | 2 + ...ContentModelDeterminismCompositional.swift | 28 +++++++--- Tests/DeterminismDifferentialTests.swift | 16 ++++-- Tests/DeterminismSequenceFollowposTests.swift | 55 +++++++++++++++++++ 4 files changed, 88 insertions(+), 13 deletions(-) create mode 100644 Tests/DeterminismSequenceFollowposTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 55764fa..df333f0 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 ### Fixed +- A repeating model whose optional trailing particle and leading particle share a name, `(c, b?, c?)*`, is now correctly rejected as a Unique Particle Attribution violation, as libxml2/Xerces reject it: after `b` the next `c` could be the trailing `c?` or the looped leading `c`, two distinct particles. The compositional determinism check formed the internal sequence gap before the enclosing repetition's loop edge was added, so the combined `{trailing, leading}` followpos was never checked (an invalid-schemas-accepted false negative the finite XSTS corpus did not contain). The sequence now carries each trailing end-item's internal suffix in its `lastFollow`, so a parent gap or the loop edge forms the end-item's full followpos. The carried suffix is always a subset of the true followpos, so the check only catches more genuine conflicts and never over-rejects: the non-repeated `(c, b?, c?)` and disjoint shapes like `(a, b?)*` stay valid. Found by the new generated-schema determinism differential against libxml2; verified over the full W3C XSTS corpus in release (`valid-schemas-rejected`/`valid-instances-rejected` stay 0, `invalid-*-accepted` unchanged) with an adversarial over-rejection critic finding no valid schema newly rejected over 47 shapes. The related ambiguity through two references to one group in a repeating sequence stays an under-rejection (the cached group identity cannot distinguish a looped from a trailing occurrence without per-occurrence tracking), named in `DeterminismDifferentialTests`. + - Two references to the same `` that overlap (`(g?, g?)`, `(g | g)`, `(g*, g)`) are now correctly rejected as a Unique Particle Attribution violation, as the inline forms `(x?, x?)` / `(x | x)` already were and as libxml2/Xerces reject them. The compositional determinism check summarises each group once and reused the group's node identity across references, so the distinct-identity overlap test could not see that two references are two expanded occurrences, and PureXML accepted these models (an invalid-schemas-accepted false negative). It also let the per-instance content matcher run on such a non-UPA model, where a multiply-referenced nullable group is exponential: a depth-18 optional doubling chain was accepted and then hung instance validation, a keystroke-time interactive-safety hang on a schema PureXML itself accepted. The fix dedupes each decision set (first set, sequence gap, `all` cross-follow) by particle identity and treats a same-identity collision as the conflict, while the repetition self-loop (`a*`, `(a*)*`) keeps its non-flagging dedupe so a single particle looping is not misread as two occurrences. This also bounds the check: the deduped first/follow sets stay `O(labels)` instead of `2^K`, so the same depth-40 chain is now rejected at compile in 11 ms and no instance is ever matched against it. A redefine self-extension (`` inside the redefinition of `T`) lists a type's own content twice through `effectiveModelNodes`; that assembly artifact is deduped before the walk so it is not read as a conflict (addB094, schZ007). Verified over the full W3C XSTS corpus in release: `valid-schemas-rejected` and `valid-instances-rejected` stay 0 and `invalid-*-accepted` are unchanged (the corpus does not exercise this shape in its pinned buckets), with an adversarial over-rejection critic finding no valid schema newly rejected across the group-overlap shape space. - XSTS invalid-schemas-accepted (#145): a particle restriction whose restricting element has a union-typed base element now applies Type Derivation OK (Simple) (`cos-st-derived-ok`) rather than structural member subsetting. The NameAndTypeOK type clause (ยง3.9.6) requires the restricting element's type to be validly derived from the base element's type; PureXML accepted a *separately declared* union whenever its member types were a subset of the base union's members, which is not the spec's derivation relation. The restricting type is now valid over a union base only if it is the base type, derives from it through a declared restriction chain, or (clause 2.2.4) validly derives from one of the union's member types. Catches particlesIk011 (a fresh `union(integer)` is not a restriction of `union(integer, string)`): invalid-schemas-accepted 9 to 8, with valid-schemas-rejected held at 0 (verified over the full XSTS; the change is scoped to the element-restriction path, so no attribute or simple-type derivation check moved). Disclosed divergence: particlesIk025/Ik027, where the restricting type derives from a union *member*, are accepted per `cos-st-derived-ok` clause 2.2.4, a contested reading their XSTS `invalid` expectation does not share. diff --git a/Sources/Schema/ContentModelDeterminismCompositional.swift b/Sources/Schema/ContentModelDeterminismCompositional.swift index 39fd2aa..b8f8cff 100644 --- a/Sources/Schema/ContentModelDeterminismCompositional.swift +++ b/Sources/Schema/ContentModelDeterminismCompositional.swift @@ -215,15 +215,20 @@ private struct CompWalk { /// sequence). The sequence's own `lastFollow` is its trailing nullable suffix's /// end-items, carried up for the parent to combine with whatever follows. mutating func sequence(_ parts: [CompSummary]) -> CompSummary { + // `suffixFirst[index]` is the first of the nullable run starting after `index`, + // i.e. what can follow an end-item of `parts[index]` WITHIN this sequence. + var suffixFirst: [[CompItem]] = Array(repeating: [], count: parts.count) for index in parts.indices { - var suffixFirst: [CompItem] = [] + var suffix: [CompItem] = [] for next in parts[(index + 1)...] { - suffixFirst.append(contentsOf: next.first) + suffix.append(contentsOf: next.first) if !next.nullable { break } } - guard !suffixFirst.isEmpty else { continue } + suffixFirst[index] = suffix + } + for index in parts.indices where !suffixFirst[index].isEmpty { for entry in parts[index].lastFollow { - emit(merge(entry.follow, suffixFirst)) + emit(merge(entry.follow, suffixFirst[index])) } } var prefixFirsts: [[CompItem]] = [] @@ -231,10 +236,19 @@ private struct CompWalk { prefixFirsts.append(part.first) if !part.nullable { break } } + // Each trailing end-item carries its internal follow PLUS the suffix that follows + // it inside the sequence. Otherwise an end-item that is also followed internally + // (an optional element after it) loses that continuation in `lastFollow`, so a + // parent gap, or an enclosing repetition's loop edge, forms only a partial + // followpos: a repeating `(c, b?, c?)` would not see that after `b` both the + // trailing `c` and the looped leading `c` are reachable. The gap loop already + // flagged the internal followpos, so dedupe without re-flagging here. var trailing: [(item: CompItem, follow: [CompItem])] = [] - for part in parts.reversed() { - trailing.append(contentsOf: part.lastFollow) - if !part.nullable { break } + for index in parts.indices.reversed() { + for entry in parts[index].lastFollow { + trailing.append((entry.item, dedupOnly([entry.follow, suffixFirst[index]]))) + } + if !parts[index].nullable { break } } return CompSummary(nullable: parts.allSatisfy(\.nullable), first: mergeList(prefixFirsts), lastFollow: mergeLastFollow(trailing)) } diff --git a/Tests/DeterminismDifferentialTests.swift b/Tests/DeterminismDifferentialTests.swift index b64909d..2d6c1f9 100644 --- a/Tests/DeterminismDifferentialTests.swift +++ b/Tests/DeterminismDifferentialTests.swift @@ -19,12 +19,16 @@ import Testing /// this generator, the same shape space that earlier hid the multiply-referenced-group /// overlap (now fixed). The false negatives it surfaces are NAMED here rather than left /// silent, and triaged genuine vs contested: -/// - GENUINE, tracked under-rejection debt (its own FP-critical fix): a repeating model -/// whose optional trailing particle and leading particle share a name, `(c, b?, c?)*` -/// (and the same through two references to one group). They are distinct particles that -/// both match at the loop point, but the compositional check computes the internal -/// sequence gap before the enclosing repetition's loop edge is added, so the combined -/// `{trailing, leading}` decision set is never formed. +/// - FIXED: a repeating model whose optional trailing particle and leading particle share +/// a name, `(c, b?, c?)*`. Distinct particles that both match at the loop point; the +/// sequence now carries each end-item's internal suffix in `lastFollow` so the loop edge +/// forms the combined `{trailing, leading}` followpos (`DeterminismSequenceFollowposTests`). +/// - GENUINE but unfixed (a fundamental compositional limitation): the SAME shape through +/// two references to ONE group in a repeating sequence (`(g+, c, g?)+`). The trailing +/// and looped occurrences share the cached group identity, so the loop fold's +/// non-flagging dedupe cannot tell them from a single particle looping (`(a*)*`) without +/// over-rejecting; distinguishing them needs per-occurrence tracking the summarize-once +/// design omits. /// - CONTESTED, libxml2 the stricter party: an UNUSED group definition with an ambiguous /// model (`all(b, b)`) (XSD's UPA constrains the effective content type of complex /// types, and an unreferenced group is never one), and `(a?)+`, a nullable body diff --git a/Tests/DeterminismSequenceFollowposTests.swift b/Tests/DeterminismSequenceFollowposTests.swift new file mode 100644 index 0000000..f46e084 --- /dev/null +++ b/Tests/DeterminismSequenceFollowposTests.swift @@ -0,0 +1,55 @@ +import Testing +@testable import PureXML + +/// A repeating model whose optional trailing particle and leading particle share a name, +/// `(c, b?, c?)*`, is ambiguous: after `b` the next `c` could be the trailing `c?` or the +/// looped leading `c`, two distinct particles. The compositional determinism check +/// emitted the internal sequence gap before the enclosing repetition's loop edge, so the +/// combined `{trailing, leading}` followpos was never formed. The fix carries each +/// trailing end-item's internal suffix in the sequence's `lastFollow`, so the loop (and a +/// parent gap) form the full followpos. Found by the generated-schema determinism +/// differential against libxml2 (`DeterminismDifferentialTests`). +@Suite("UPA on a repeating sequence's trailing/leading overlap") +struct DeterminismSequenceFollowposTests { + private let opt = " minOccurs=\"0\"" + private let star = " maxOccurs=\"unbounded\"" + private let optStar = " minOccurs=\"0\" maxOccurs=\"unbounded\"" + + private func element(_ name: String, _ occurrence: String = "") -> String { + "" + } + + private func sequence(_ occurrence: String, _ members: String...) -> String { + "\(members.joined())" + } + + private func accepts(_ content: String) -> Bool { + let schema = "" + + "\(content)" + return (try? PureXML.Schema.Document(schema)) != nil + } + + @Test("a repeating sequence whose trailing optional and leading particle overlap is rejected") + func test_repeatingTrailingLeadingOverlapRejected() { + // (c, b?, c?)*: after b, the trailing c? and the looped leading c both match c. + #expect(!accepts(sequence(optStar, element("c"), element("b", opt), element("c", opt)))) + // (a, b?, a?)*: the same shape, leading and trailing a. + #expect(!accepts(sequence(star, element("a"), element("b", opt), element("a", opt)))) + // (a?, b, a?)*: leading optional a and trailing optional a, around a required b. + #expect(!accepts(sequence(star, element("a", opt), element("b"), element("a", opt)))) + } + + @Test("the same sequences NOT repeated, or without the overlap, stay valid") + func test_noOverRejection() { + // (c, b?, c?) NOT repeated: the trailing c is the only c after b, unambiguous. + #expect(accepts(sequence("", element("c"), element("b", opt), element("c", opt)))) + // (a, b?)*: distinct names, the loop's a does not overlap the trailing b. + #expect(accepts(sequence(optStar, element("a"), element("b", opt)))) + // (a, b, c)*: all distinct, the loop's a follows the required c unambiguously. + #expect(accepts(sequence(star, element("a"), element("b"), element("c")))) + // a*: a single particle looping is not two occurrences (the loop-fold must not flag). + #expect(accepts(sequence("", element("a", optStar)))) + // (c, b?, d?)*: the trailing d does not share the leading c's name. + #expect(accepts(sequence(star, element("c"), element("b", opt), element("d", opt)))) + } +}