Fix #134: make xpathExpression values self-contained for namespace/ve… - #135
Open
humantypo wants to merge 1 commit into
Open
Fix #134: make xpathExpression values self-contained for namespace/ve…#135humantypo wants to merge 1 commit into
humantypo wants to merge 1 commit into
Conversation
…rsion XPath expressions previously resolved namespace prefixes and XPath version from a single Request- or Policy-wide XPathRequestDefaults/XPathPolicyDefaults table, in every representation including XML. XML permits the same prefix to be bound to different URIs in different parts of one document, so two individually-legal XML request entities using the same prefix for different URIs could not both be represented in JACAL/YACAL - and response Notice values had no defaults-carrying mechanism at all. xpathExpression values now carry their own required XPathVersion and Namespace (a prefix-to-URI mapping in JSON/YAML, xmlns:*/XPathVersion attributes on the Value element in XML) unconditionally, in every representation. Selectors keep centralized, policy-wide XPathVersion and ancestor-based XML namespace lookup unchanged, since a selector's Path never leaves its policy. XPathRequestDefaultsType is removed entirely (XML, JSON, YAML) as the direct consequence: nothing consumes it once request-side values are self-contained. Namespace's old array-of-Prefix/Name-object-pairs shape is replaced by a prefix-to-URI mapping everywhere it appears, ending the verbosity a repeated array shape produced once every value/selector needs its own copy. Namespace map keys are now validated as XML NCNames rather than reusing Core's looser IdentifierType, which was never intended for this lexical space. Also fixes: the identical namespace-scoping defect in this profile's own canonical XML example, HRP/MDP's just-merged JACAL/YACAL examples (which used the now-removed mechanism), and several unrelated pre-existing bugs found while verifying touched examples end-to-end (Rule3.xml's wrong-version attribute identifier, Request.json's Content.Body missing an xmlns:xsi declaration needed for standalone well-formedness). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Notes
There are number of decision points associated with this issue so I wanted to try to capture the context of them in a way that will hopefully make it easier to track why it was done in this manner (and why not one of the listed alternatives).
1. Fix direction: self-containment, not translator-side rewriting
Chosen: adopt Steven's self-containment direction.
Rejected alternative: keep the single global
Namespacetable, and instead specify how a translator converting XML → JACAL/YACAL must detect prefix collisions and rewrite one side's prefixes to avoid them.Why: Steven's own text calls conflict-detection-and-rewriting non-trivial, especially once the default namespace (empty prefix) is in play. Every downstream translator would have to reimplement that logic correctly. Self-containment pushes the same complexity onto the producer, once, at the point where the XML's actual in-scope namespaces are already known. It's cheaper and more reliable than asking every consumer to reconstruct scope information that XML's structure already had and JACAL/YACAL's flat structure throws away.
2. Scope: all three surfaces in one pass
Chosen:
xpathExpressionvalues,XPathAttributeSelectorType/XPathEntityAttributeSelectorType, and responseNoticevalues all get the fix together.Rejected alternative: fix request-side
xpathExpressionvalues now (what Steven's worked example shows concretely), defer selectors and response notices to follow-up issues.Why: all three are the same underlying defect: a namespace-context resolution mechanism that doesn't travel with the expression it governs. Leaving any one out reintroduces the exact ambiguity being fixed for that surface, and the response case doesn't even have a broken fallback today —
there is no mechanism there at all.
3.
Namespaceshape: prefix→URI mapping, applied consistently everywhere (not just new sites)Chosen: redefine
Namespaceas a JSON object / YAML mapping directly from prefix string to URI string ({"ns": "urn:example:resource"}), and apply that shape everywhere the concept appears, including the pre-existingXPathRequestDefaults/XPathPolicyDefaultsusage, not only the new per-value sites.Rejected alternatives:
{Prefix,Name}-objects shape verbatim at the new sites. Rejected:fine for a table that appears once per document; repeated at every
xpathExpressionvalue, selector, and response notice it is materially more verbose than necessary in both JSON and YAML, and this is exactly what triggered the original "awkward for YAML" observation that started this review.XPathRequestDefaults/XPathPolicyDefaults. Rejected: would leave two different encodings of the same concept live in the same profile at the same time, for no reason once the cutover (decision 6) is clean rather than phased.Effect: prefix uniqueness becomes structural (an object/mapping cannot have two identical keys in the data model) rather than a bolted-on
isUnique(Prefix)constraint the JSON schema could never actually enforce. Still needs the duplicate-serialized-key caveat (see decision 8).4. Local context: full replacement, not merge with Defaults
Chosen: when a local
Namespaceis present, it is the entire effective namespace context. Nothing is inherited fromXPathRequestDefaults/XPathPolicyDefaults.Rejected alternative: merge semantics: local entries override same-prefix Defaults entries, everything else still inherited from Defaults. This does not actually reintroduce Steven's original cross-value ambiguity (each va lue's effective context would still be fully determined), and it
reduces duplication for documents where most namespaces are shared.
Why full replace won this: merge keeps a value's meaning dependent on its container. That is exactly the property self-containment exists to remove. It breaks the moment a value is copied into a response, cached, compared, or moved between policies, since a merged value's true meaning
requires re-resolving against whatever Defaults happened to be in scope at the copy site. Implementations would need to snapshot the merged context at every scope boundary a value might cross, which is real complexity for a problem (verbosity) that's smaller than the problem it's avoiding (context-dependent meaning). Namespace blocks are typically small; if authoring convenience matters, that belongs in tooling/templates, not wire semantics.
Effect:
Namespace: {}is a meaningful, distinct state — "this expression deliberately uses no prefixed names" — not the same as "inherit from Defaults." (This distinction becomes moot once Defaults-levelNamespaceis deleted outright per decision 6, but the requiredness question survives it (see decision 9).Explicit exclusion from "complete context": bindings XPath/XML defines implicitly (via the reserved
xmlprefix) are not required in the local map. Everything else needed to preserve the original XML expression's static context must be local.5.
XPathVersionscope: intrinsic to the value type, centralized for selectors — in every representation, including XMLChosen:
XPathVersionbecomes a required, local property ofXPathExpressionValueType, in XML as well as JSON/YAML, axpathExpressionvalue's version was previously always resolved fromXPathRequestDefaults/XPathPolicyDefaultsin every representation, sinceXPathExpressionValueType's abstract UML never had a version property of its own at all before this change. It travels with the value everywhere the value can go, symmetrically withNamespace. Selector types (XPathAttributeSelectorType/XPathEntityAttributeSelectorType) keep a centralized, policy-wide version fromXPathPolicyDefaults, in every representation. There is no local override.Rejected alternatives:
XPathVersioncentralized for everything, add it locally only where nothing else exists to carry it (i.e. only on responseNoticevalues). Rejected: two properties on the same value type (Namespace,XPathVersion) would follow different scoping rules with no principled reason for the asymmetry once one of them (a value crossing into a response) needs the local treatment anyway.examples/acal-xpath/Request.xml) declaredxmlns:mdonRequestEntity— an ancestor of theValueelement, not theValueelement itself — which is exactly Steven's scoping defect. XML's nativexmlns:*syntax made the gap easy to miss, not exempt from it: a value can be copied into a response, cached, or moved into another policy in any representation, none of which carry the original ancestor/Defaults context.Why the value/selector split holds, in every representation:
XPathExpressionValueTypeis a first-class value. It can move from a request or policy into a response, notice, cache, or another processing stage, so both its version and its namespace context must travel with it or it stops being self-describing the moment it moves — in XML this meansxmlns:*and a newXPathVersionattribute declared directly on theValueelement itself, not inherited fromRequestEntityor any other ancestor (xs:anyAttributeon Core'sValueTypealready permits this, so no XSD complex-type addition was needed, only prose). A selector'sPathis policy syntax — it never escapes the policy it's written in, in any representation, so a single policy-wide XPath version continues to serve its actual use; XML selectors keep the pre-existing ancestor-based in-scope-namespace lookup unchanged, and only JSON/YAML selectors gained the newNamespaceproperty, because only JSON/YAML lacked a native way to express it at all.Effect: producers copying an
xpathExpressionvalue into a response must materialize its effective version and namespace context at copy time, in every representation — this is now a real, statable producer obligation rather than an unaddressed gap.XPathRequestDefaultsType's removal (decision 7) follows the same representation-independent logic: the XSD'sXPathRequestDefaultssubstitution-group element was removed too, since nothing was left consuming it once request-side values became self-contained.6. Compatibility strategy: clean cutover, not phased legacy fallback
Chosen: remove
NamespacefromXPathRequestDefaultsType/XPathPolicyDefaultsTypeoutright, in this same change. LocalNamespace/XPathVersionbecome required immediately. No SHOULD-now/MUST-later staging, no legacy-fallback-to-Defaults path.Rejected alternative: a phased rollout — keep Defaults-level
Namespacealive as a working legacy fallback, introduce localNamespace/XPathVersionas SHOULD in this stage, strengthen to MUST in a later one. This was the initial framing and was reconsidered once the document's actualpublication stage was checked.
Why:
acal-xpath-v1.0.mdis CSD02 — a public Committee Specification Draft, explicitly a revisable stage, not a stable release. A defect this fresh is cheap to correct now and gets more expensive to correct at every later stage. Carrying two coexisting namespace-resolution mechanisms through an indefinite transition period buys protection for implementers the TC has no evidence exist yet, at the cost of permanent implementation and specification complexity.7.
XPathRequestDefaultsType: deleted outright, not left vestigialChosen: delete
XPathRequestDefaultsType, its Tree wrapper in the JSON/YAML schemas, and its extension point onRequestType, entirely.Rejected alternative: keep an empty/reserved
XPathRequestDefaultsTypearound for symmetry withXPathPolicyDefaultsType, which survives (asXPathVersion-only — selectors still need it).Why: this is a direct structural consequence of decisions 5 and 6, not a separately-argued choice.
XPathRequestDefaultsTypecurrently has exactly two properties,XPathVersionandNamespace. Selectors (the only thing that still needs a centralized version) are policy-only, served byXPathPolicyDefaults. Once request-sidexpathExpressionvalues carry both properties locally, nothing in a request consumesXPathRequestDefaultsTypefor anything. An empty extension point kept only for symmetry has no function and no precedent in this repo's conventions.Effect: this is a bigger removal than "deprecate" — an entire type and its
RequestTypeextension point disappear from the XPath Profile's XML/JSON/YAML artifacts together. Worth stating explicitly in the PR description, since it's the most visible structural change in the diff and easy to read as accidental scope creep if not flagged as a deliberate consequence.8. Duplicate-key safety: directive implementer prose, not a schema constraint
Chosen: state a normative SHALL in prose — producers SHALL NOT emit a
Namespaceobject/mapping containing the same key twice; consumers SHALL reject one that does, as a requirement on the parsing/deserialization step, with an explicit note that JSON Schema cannot enforce it.Rejected alternative: don't police it at all. Treat a document with a duplicate
Namespacekey as malformed input, out of scope for the spec to define behavior for, the same way XACML doesn't define behavior for malformed XML.Why: JSON Schema validates the parsed instance; by the time a validator sees it, a document with duplicate keys has already been silently resolved (typically last-key-wins, but this is parser-dependent, not spec-mandated by RFC 8259) or already rejected, entirely outside the schema's visibility. Given the map shape's correctness now depends on key uniqueness (decision 3), silently allowing implementation-defined duplicate-key handling would leave the actual failure mode—a namespace silently resolving to the wrong URI—undocumented for implementers who would have no reason to suspect it. This follows the same pattern already established for Issue 120's
DataType-uniqueness$comment: where JSON has no structural backstop, the profile states a procedural recipe instead (e.g.implementer-guidance-is-serialization-native-not-mirroredinarchitectural_decisions.md).9.
Namespacerequiredness: required,{}for "none"Chosen:
Namespacestays required alongsideXPathVersiononXPathExpressionValueTypeand both selector types, using{}to mean "no namespaces needed."Rejected alternative: make it optional, with absence equivalent to
{}.Why: with Defaults-level
Namespacedeleted outright (decision 6/7), "absent" and "present-empty" no longer point at two different fallback targets — there's nothing to fall back to either way, so the original reason for insisting on a distinguishable empty state is gone. But required-with-{}was kept anyway, for a different reason: it keepsNamespaceandXPathVersionparallel (bothrequired, both fully local), and makes every value visibly self-describing — a reader or validator never has to wonder whether
Namespacewas simply forgotten versus deliberately empty.10. HRP/MDP example ripple: fixed in this same branch
Chosen: update HRP §7.1 and MDP §8.2's JACAL/YACAL examples (merged hours earlier as part of PR 133) to the new local-
Namespaceshape, in this branch.Rejected alternative: land the XPath Profile fix alone; file a follow-up issue for HRP/MDP's examples once the new shape is settled and separately reviewed.
Why: those examples currently resolve prefixes via exactly the
XPathRequestDefaults/Namespacemechanism this change deletes. Under the clean-cutover decision they become non-conformant the moment this change lands — not a latent gap discoverable later, but an immediate, self-inflicted contradiction between two merged/in-flight pieces of the same repo. Deferring the fixwould mean shipping a known-broken normative example on
main.11.
Namespacekey validation: XMLNCName, not reusedIdentifierTypeChosen:
NamespaceMapType'spropertyNames(JSON and YAML) validate a non-empty key against an NCName-lexical pattern (^[A-Za-z_][A-Za-z0-9_.-]*$) and explicitly excludexmlns, instead of reusing Core'sIdentifierType.acal-xpath-v1.0.md§5.2'sPrefixproperty gained a matching normative sentence.Rejected alternative: leave
propertyNamesasIdentifierType, inherited unchanged from the pre-#134NamespaceDeclarationType.Prefixproperty, which used the same type. Core'sIdentifierType$commentstates its looseness is deliberate ("the pattern may be improved for stricter validation... if your ACAL implementation is already parsing and validating such values strictly according to the ACAL core specification, then your implementation-specific validation is probably more optimal") — so at first glance, extending that same type here looked like it was just carrying forward an already-decided, already-justified design stance rather than a new gap.Why the rejected alternative was checked, not assumed: read closely,
IdentifierType's$commentis about ACAL's own identifier grammar specifically — URN-shaped strings with{ShortId}macro-substitution, a concept the JACAL specification (§5.1.2.1) defines a complete grammar for, which implementations are expected to already validate against independently. A namespace prefix is not an ACAL identifier at all: it is an XMLNCName[NAMESPACES], an unrelated external lexical space with no macro-substitution concept, that JACAL §5.1.2.1's grammar wouldn't even help validate. ReusingIdentifierTypeforNamespace's keys was inherited convenience from before this issue, not a considered application of that$comment's rationale — no verbiage anywhere actually addressed prefix validation for this case. This is a real, previously-unaddressed gap, not a documented "leave it to the implementer" stance for this specific case.Effect:
xmlns— never a valid namespace prefix under [NAMESPACES] — is now rejected structurally in both JSON and YAML.xmlremains schema-permitted: [NAMESPACES] allows redundantly redeclaring it to its own correct URI, and rejecting only a wrong-URI redeclaration is a cross-field constraintpropertyNamescan't express; §5.2 already states thexmlbinding is implicit and MUST be treated as available without appearing inNamespace, but doesn't forbid correctly restating it. The ASCII-practical NCName pattern is deliberately not maximally strict, in the same "improve later if needed" spirit asIdentifierType's own$comment— just applied to the correct grammar this time, with its own$commentsaying so explicitly rather than silently reusing the neighboring one.Two items identified during design, not decision forks — carry into drafting
"Visibly utilized" (exc-c14n) does not transfer as-is. It's defined over XML infoset structures (element/attribute names,
xsi:typevalues, etc.), not over arbitrary substrings of an XPath expression string — reusing the term directly for "prefixes used within the XPath expression" is imprecise bordering on incorrect. The safe normative rule: require the complete effective in-scope namespace context to be copied into the local map; treat minimization (leaving out prefixes that aren't actually referenced) as implementer-optional, not as a MUST governed by an exclusive-C14N term that was never defined for this context.Steven's issue example has two transcription slips, not design points, but worth not reproducing verbatim in the PR/spec text: both
xmlns:nsattribute values in his XML snippet are missing their closing quote, and the second JSONValueobject's"Name"field reads"ns:user/ns:username"(the XPath expression text) where it should read"urn:example:user"(the namespace URI).Target-state shape (consolidated)
Applies in all three representations, not just JSON/YAML (see decision 5): XML expresses
Namespace/XPathVersionnatively —xmlns:*and a newXPathVersionattribute declared directly on theValueelement carrying the expression, not inherited fromRequestEntityor any ancestor — while JSON/YAML use the mapping shape above, identical as a concrete representation in both JACAL and YACAL. Selectors are unaffected in XML (ancestor-based lookup unchanged);XPathRequestDefaultsType's removal likewise reaches the XSD, not just JSON/YAML.