From 7c059e13550226e76eb50ee58766c7f32a64f80d Mon Sep 17 00:00:00 2001 From: Mihaela Mihaljevic Date: Mon, 15 Jun 2026 01:50:35 +0200 Subject: [PATCH] fix(schema): reject circular model-group, attribute-group, and substitution-group references A model group may not contain itself (mg-props-correct.2), an attribute group may not reference itself transitively (ag-props-correct.3), and a substitution chain may not loop (e-props-correct.6). The model-group check follows references only within a definition's own compositor tree, stopping at element and type boundaries: a group referenced inside an element's content is a recursive data structure (permitted), not the group containing itself (particlesZ010, addB077). A reference is followed only when it resolves to the schema's own target namespace; a group or attribute group redefined in xs:redefine ends the chain (its single self-reference is the legal redefinition). XSTS: invalid schemas accepted 516 to 505 (attgC010/020/031, attgD015, groupB012-015, addB180, xsd009.e, xsd010.e now rejected), no other bucket raised. A genuine cycle introduced across two redefined definitions is a disclosed under-rejection (an adversarial critic's only finding). --- CHANGELOG.md | 1 + Sources/Schema/SchemaIDAttribute.swift | 1 + Sources/Schema/XSDDerivation.swift | 95 ++++++++++++++++++++++- Tests/SchemaCircularDerivationTests.swift | 35 +++++++++ Tests/XSTSSuiteTests.swift | 2 +- docs/schema-validity-burndown.md | 2 +- 6 files changed, 130 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bde9c89..1f43aac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Circular references among model groups, attribute groups, and substitution-group affiliations are now rejected (XSD 1.0 `mg-props-correct.2`, `ag-props-correct.3`, `e-props-correct.6`, #168). A model group may not contain itself, an attribute group may not reference itself transitively, and a substitution chain may not loop. The model-group check follows references only within a definition's own compositor tree, stopping at element and type boundaries: a group referenced inside an element's content is a recursive data structure (permitted), not the group containing itself. A reference is followed only when it resolves to the schema's own target namespace, and a group or attribute group redefined in `xs:redefine` (whose single self-reference is the legal redefinition) ends the chain. XSTS: invalid schemas accepted 516 to 505, no other bucket raised. A genuine cycle introduced across two redefined definitions is a disclosed under-rejection. - Circular type derivation is now rejected at compile time (XSD 1.0 `ct-props-correct.3` / `st-props-correct.2`, #168). A complex or simple type may not derive, transitively through its base, from itself; previously such a schema was accepted. Only the base chain is followed, so a type whose element content recurses (an element of its own type, the normal recursive-data-structure case) is correctly permitted, and a type redefined in `xs:redefine` (whose base legitimately names its former self) and the ur-types (`anyType`/`anySimpleType`/`anyAtomicType`, the roots the schema-for-schemas bootstraps) end the chain rather than appear as cycles. The base is followed only when it resolves to the schema's own target namespace, so a type whose base is a same-local-name type in another namespace is not mistaken for a self-cycle. XSTS: invalid schemas accepted 519 to 516, no other bucket raised. Union/list (`itemType`/`memberTypes`) and genuine cross-namespace derivation cycles are a disclosed under-rejection (the engine resolves by local name). - Particle Valid (Restriction) now implements the W3C MapAndSum rule for a sequence restricting a choice (#165). When a derived sequence restricts a base choice, the spec (XSD 1.0 ยง3.9.6) requires that each derived particle restrict some base branch AND that the derived sequence's occurrence count times its number of particles lie within the base choice's occurrence range. PureXML previously applied the same-compositor occurrence subsumption (comparing the sequence's own range directly to the choice's), which both rejected a valid restriction (`sequence{2,4}(e1,e2)` of `choice{3,9}(e1,e2)`, whose 4-to-8 elements fit the choice's 3-to-9) and accepted an invalid one (a two-member sequence restricting a single-occurrence choice, which the count-product `2 > 1` now rejects). The count-product is the spec's measure, matched to the reference implementation. XSTS: valid schemas rejected 2 to 1, invalid schemas accepted 520 to 519, no other bucket raised. The one remaining valid-rejected case (`particlesZ001`) is a documented spec ambiguity (its own test notes the RecurseAsIfGroup rule "is ambiguous"; the strict reading the reference follows rejects it), recorded in `docs/xsts-deviations.md`. diff --git a/Sources/Schema/SchemaIDAttribute.swift b/Sources/Schema/SchemaIDAttribute.swift index 3a2055b..c99904d 100644 --- a/Sources/Schema/SchemaIDAttribute.swift +++ b/Sources/Schema/SchemaIDAttribute.swift @@ -48,6 +48,7 @@ extension PureXML.Schema.XSDParser { let structural = idAttributeErrors(schema) + structureErrors(schema) + componentNameErrors(schema) let determinism = PureXML.Schema.ContentModelDeterminism.violations(in: schema, context: context) let cycles = derivationCycleErrors(containers, context.namespaceBindings, context.targetNamespace) + + circularReferenceErrors(containers, context.namespaceBindings, context.targetNamespace) return structural + determinism + cycles } } diff --git a/Sources/Schema/XSDDerivation.swift b/Sources/Schema/XSDDerivation.swift index 3c23459..d09d9e6 100644 --- a/Sources/Schema/XSDDerivation.swift +++ b/Sources/Schema/XSDDerivation.swift @@ -190,17 +190,104 @@ extension PureXML.Schema.XSDParser { /// The names of types redefined in an `xs:redefine`, whose `base` legitimately /// names themselves. private static func redefinedTypeNames(_ containers: [XSDTree]) -> Set { + redefinedNames(containers, "complexType").union(redefinedNames(containers, "simpleType")) + } + + /// The names of components of `kind` redefined in an `xs:redefine`. A redefined + /// model group or attribute group legitimately references its own former self + /// once, so it is a chain end rather than a cycle. + private static func redefinedNames(_ containers: [XSDTree], _ kind: String) -> Set { var names: Set = [] for container in containers where XSDDerivNode.localName(container) == "redefine" { - for kind in ["complexType", "simpleType"] { - for type in XSDDerivNode.children(container, named: kind) { - if let name = XSDDerivNode.attribute(type, "name") { names.insert(name) } - } + for definition in XSDDerivNode.children(container, named: kind) { + if let name = XSDDerivNode.attribute(definition, "name") { names.insert(name) } } } return names } + /// Rejects circular references among named model groups, attribute groups, and + /// substitution-group affiliations (XSD 1.0 `mg-props-correct.2`, + /// `ag-props-correct.3`, `e-props-correct.6`): a group may not contain itself, an + /// attribute group may not reference itself transitively, and a substitution + /// chain may not loop. A reference is followed only when it resolves to this + /// schema's own target namespace, and a redefined group/attribute group is a + /// chain end (its single self-reference is the legal redefinition). + static func circularReferenceErrors(_ containers: [XSDTree], _ bindings: [String: String], _ target: String?) -> [String] { + substitutionCycleErrors(containers, bindings, target) + + referenceCycleErrors(containers, "group", "model group", bindings, target) + + referenceCycleErrors(containers, "attributeGroup", "attribute group", bindings, target) + } + + private static func substitutionCycleErrors(_ containers: [XSDTree], _ bindings: [String: String], _ target: String?) -> [String] { + var graph: [String: String] = [:] + for container in containers { + for element in descendants(container, named: "element") { + guard let name = XSDDerivNode.attribute(element, "name"), + let head = XSDDerivNode.attribute(element, "substitutionGroup"), + XSDDerivNode.referenceNamespace(head, bindings) == target else { continue } + graph[name] = XSDDerivNode.stripPrefix(head) + } + } + return graph.keys.sorted().compactMap { name in + derivesFromItself(name, graph, []) ? "element '\(name)' must not be a member of its own substitution group" : nil + } + } + + /// A named-definition reference cycle: a `kind` definition (`group` / + /// `attributeGroup`) whose `kind`-references reach itself. Multi-edge, since a + /// definition may reference several others. + private static func referenceCycleErrors(_ containers: [XSDTree], _ kind: String, _ label: String, _ bindings: [String: String], _ target: String?) -> [String] { + let redefined = redefinedNames(containers, kind) + var adjacency: [String: [String]] = [:] + for container in containers { + for definition in descendants(container, named: kind) { + guard let name = XSDDerivNode.attribute(definition, "name") else { continue } + for ref in boundedReferences(definition, kind) where XSDDerivNode.referenceNamespace(ref, bindings) == target { + adjacency[name, default: []].append(XSDDerivNode.stripPrefix(ref)) + } + } + } + return adjacency.keys.sorted().compactMap { name in + guard !redefined.contains(name), reachesItself(name, adjacency, redefined) else { return nil } + return "\(label) '\(name)' must not reference itself" + } + } + + /// The `kind`-references a definition contains *directly* in its own content, + /// not crossing an element or type boundary. A `` reached through an + /// element's content model is the element's content (a recursive data + /// structure), not the group containing itself, so the descent stops at + /// `element`/`complexType`/`simpleType`/`attribute` scopes. + private static func boundedReferences(_ node: XSDTree, _ kind: String) -> [String] { + var references: [String] = [] + for child in XSDDerivNode.elementChildren(node) { + switch XSDDerivNode.localName(child) { + case "element", "complexType", "simpleType", "attribute": + continue + case kind: + if let ref = XSDDerivNode.attribute(child, "ref") { references.append(ref) } + default: + references += boundedReferences(child, kind) + } + } + return references + } + + /// Whether following `start`'s references (multi-edge) returns to `start`. A + /// sink (a redefined definition) is not expanded. + private static func reachesItself(_ start: String, _ adjacency: [String: [String]], _ sinks: Set) -> Bool { + var visited: Set = [] + var stack = adjacency[start] ?? [] + while let node = stack.popLast() { + if node == start { return true } + if sinks.contains(node) { continue } + guard visited.insert(node).inserted else { continue } + stack += adjacency[node] ?? [] + } + return false + } + /// Throws when a type inside an `xs:redefine` does not derive from itself: a /// redefinition's `base` must name the type being redefined. static func checkRedefine(_ containers: [XSDTree]) throws { diff --git a/Tests/SchemaCircularDerivationTests.swift b/Tests/SchemaCircularDerivationTests.swift index edaf04b..d61694b 100644 --- a/Tests/SchemaCircularDerivationTests.swift +++ b/Tests/SchemaCircularDerivationTests.swift @@ -65,6 +65,41 @@ struct SchemaCircularDerivationTests { #expect((try? PureXML.Schema.Document(schema)) != nil) } + @Test("Circular model-group, attribute-group, and substitution-group references are rejected") + func test_referenceCyclesRejected() { + // A model group that directly contains itself (mg-props-correct.2). + #expect(!compiles( + "" + + "", + )) + // An attribute-group reference cycle a -> b -> a (ag-props-correct.3). + #expect(!compiles( + "" + + "", + )) + // A substitution-group affiliation loop x -> y -> x (e-props-correct.6). + #expect(!compiles( + "", + )) + } + + @Test("A group recursing through an element, and a substitution chain, are allowed") + func test_groupRecursionAndChainAccepted() { + // particlesZ010: a group referenced inside an element's content is a recursive + // data structure, not a group containing itself. (Must compile.) + #expect(compiles( + "" + + "" + + "" + + "", + )) + // A non-cyclic substitution chain leaf -> mid -> head. + #expect(compiles( + "" + + "", + )) + } + @Test("A type redefining itself is allowed (the base names its former self)") func test_redefineSelfDerivationAccepted() { let base = "" diff --git a/Tests/XSTSSuiteTests.swift b/Tests/XSTSSuiteTests.swift index 3bc3d03..67e836c 100644 --- a/Tests/XSTSSuiteTests.swift +++ b/Tests/XSTSSuiteTests.swift @@ -29,7 +29,7 @@ struct XSTSSuiteTests { /// XSTS` (debug is far slower and the corpus is large). Per-case deviations /// are written to /tmp/xsts-failures.txt for the burn-down. private let knownSchemaValidRejected = 1 - private let knownSchemaInvalidAccepted = 516 + private let knownSchemaInvalidAccepted = 505 private let knownInstanceValidRejected = 180 private let knownInstanceInvalidAccepted = 160 diff --git a/docs/schema-validity-burndown.md b/docs/schema-validity-burndown.md index d4fcbd6..6876176 100644 --- a/docs/schema-validity-burndown.md +++ b/docs/schema-validity-burndown.md @@ -10,7 +10,7 @@ irreducible tail. Do not deviate to new features while this is open. | Bucket | Count | Reading | |---|---|---| | valid-schemas-rejected | **72 -> 4** | production target is 0: rejecting a correct schema is the most user-hostile defect (#148). Particle-restriction over-rejection fixed: pointless `maxOccurs=0` particles removed, content-free restriction of an EMPTY base, empty-derived emptiable-base, and (#163) the pairwise check over the normalized content model with occurrence subsumption applied only to same-kind pairings (cross-kind element/group, group/element, group/wildcard match occurrence inside, by effective total range / member). #164 resolved element-ref namespaces by prefix binding (the `particlesJj/Jk/Jm/Jn/Js`, `particlesQ/R` cluster, -28). Content-free (`maxOccurs=0`) derived groups and substitution-group declaration order closed `particlesW006`/`elemZ027_a`; the W3C MapAndSum rule (Sequence:Choice, #165) closed `particlesV003`. Remaining 1 (`particlesZ001`): a documented spec ambiguity (RecurseAsIfGroup; the reference rejects it), not a defect. | -| **invalid-schemas-accepted** | **2461 -> 516** | we rarely catch a bad one (facet, default/fixed value, particle-restriction name+type, id, structural content-model, UPA determinism, content order, identity-constraint XPath-subset, wildcard-namespace, final/block, simpleType content, group/wildcard and group/element cardinality, element-ref namespace, sequence/choice MapAndSum, circular type derivation landed). | +| **invalid-schemas-accepted** | **2461 -> 505** | we rarely catch a bad one (facet, default/fixed value, particle-restriction name+type, id, structural content-model, UPA determinism, content order, identity-constraint XPath-subset, wildcard-namespace, final/block, simpleType content, group/wildcard and group/element cardinality, element-ref namespace, sequence/choice MapAndSum, circular type derivation and circular group/attributeGroup/substitution references landed). | | valid-instances-rejected | **233 -> 180** | (instance side; #164's element-ref namespace fix is shared with instance validation, -34) | | invalid-instances-accepted | 160 | |