Repository navigation
Preserve ids carried under attributes, closing a document-ID collision - #133
Merged
Merged
Conversation
Closes the collision half of #125. Two documents differing only in their CSL citation keys canonicalized to identical bytes: A: citation:[{id:"smith2020",...},{id:"jones2019",...}] B: citation:[{id:"OTHER-KEY",...},{id:"DIFFERENT",...}] both -> sha256:869299f2c3d550ca698e09ad7db9ac0152e5a23dae32370742ae48aa8546ce5c A signature over B verified A. §4.3.1 item 5's alpha-equivalence is for author-chosen LABELS; it was being applied to third-party vocabulary data where the keys are content — and item 5 already exempts a `semantic:bibliography` entry's id for exactly that reason. The same data one field over, under `attributes.semantic`, was relabeled. Whether it fired turned on `type` versus `@type`, so well-formed JSON-LD was accidentally immune and CSL-JSON was not. PRESERVED, not excluded. Excluding was implemented and reverted yesterday: it takes the id out of the namespace but leaves `rewriteIds` mapping it onto a generated name, which is a silent collision rather than a loud one. A preserved id stays in the map's input and out of the map, so `renameId` returns it verbatim — the mechanism `semantic:term` and `semantic:footnote` already use. It also inherits item 5's canonical-name prohibition, so an id under `attributes` spelling `b0` is now an error. Baseline differential, every witness schema-valid: before after CSL keys under attributes id -> b0 preserved duplicate anchor id there ID-COLLISION ID-COLLISION link href resolving into attributes href -> #b0 resolves by name id there consumes a b-index real block b1 real block b0 semantic:ref there, dangling DANGLING DANGLING id there duplicating a real block id ID-COLLISION ID-COLLISION CONTROL, same shapes in real content unchanged unchanged The duplicate cases still REJECT and that is correct: measured, a `link` href resolves INTO `attributes` and is rewritten, so that material is reachable and a duplicate there is a real collision. The comment claiming "nothing anchors into it" was a normative wish stated as fact. The b-index shift is deliberate: item 5 says a preserved occurrence "consumes no index", and states that reserving one instead would give identical content a different document ID. Zero documents in the corpus carry such an id, so nothing moves here.
A test encoding the whole differential, a published vector for the
canonicalizer behaviour, and a published fixture for the reader's.
The vector and the fixture kill DIFFERENT mutants, which is why both are
here:
relabeling returns (the fix reverted)
-> canonicalize/attributes-id-preserved-not-relabeled
expected "id":"smith2020", got "id":"b0"
ids EXCLUDED from the namespace instead of preserved
-> document/reject-content-attributes-id-collision
disposition IGNORE outside [INTEGRITY-ERROR, REJECT]
That second mutant is the approach tried and reverted on the #128
branch. It reports nothing here AND relabels the id anyway, which is the
silent collision. Nothing in the published suite caught it before.
The fixture expectation is a RANGE, not a pin: CDX-E-ID-COLLISION is
registered INTEGRITY-ERROR and the later states escalate it, as the other
id-collision cases already express. Pinning REJECT is what the first
draft did, and check:conformance rejected it.
The Python oracle needed no counterpart, measured rather than assumed:
it does not relabel, so preserved-versus-relabeled is invisible to it,
while uniqueness is unchanged. It confirms the new fixture independently
(115 -> 116 injected defects confirmed).
216 vectors + 164 fixtures = 380 checks, all 30 gates green.
215 -> 216 vectors, 163 -> 164 fixtures, 378 -> 380 checks. Added cases take the patch position pre-1.0. The entry records the one deliberate cost — a preserved occurrence consumes no index, so a real sibling takes the name the preserved one would have had, changing the document id of any document carrying an id under `attributes`. Zero in the corpus, measured.
The fix shipped behaviour the specification contradicts, which is the defect #126 was filed about, repeated one branch later. Item 5 said `semantic:term` and `semantic:footnote` "are the only namespace members whose ids are left as authored". An id under `attributes` is now a third kind, and the transform preamble still counted "the two id-relabeling exceptions item 5 names". `attributes` appeared once in the whole document, in item 4's barrier sentence, and never in item 5. Item 5 now states the position-keyed exception with its reason — an id under a closed set of authored values is a vocabulary key, not a label, and relabeling it applies alpha-equivalence to data the author did not choose as one — and marks the two extension block types as the further TYPE-keyed exception, which is the one that departs from this item's position principle. The "preserved id remains a full member" paragraph now covers both kinds, and the `b<digits>` prohibition was already generic to "a preserved id". The reason is the same one already given for a `semantic:bibliography` entry's `id`, so the erratum cites it rather than restating it.
The adversarial lens found the entry overclaiming in three places and the published artifacts wrong in two. The collision CLASS is not closed. The same harm is reachable through any extension data array not literally named `entries`: a `vendor:refs` block with a `citation` array gives one document ID for two entirely different key sets. Item 5 already states the broader rule and the reference implements it as a one-element denylist. Membership also still turns on `type` versus `@type`. Both are `definedId` keying membership on shape where item 5 says position, and both are #132 — this branch closes the `attributes` path through that, not the class. "a `link` mark's href resolves into it and is rewritten" was false. Measured, the href stays verbatim, and correctly so: a preserved id takes no canonical name, so there is nothing to rewrite it to. Only resolution reaches it. The vector did not discriminate. Its id sat in a `citation` ARRAY, which item 5's data-array sentence already leaves as authored — so an implementation that never applied the `attributes` rule reproduced the expected bytes, and the vector pinned the outcome rather than the rule. It now uses a SINGULAR sub-object, which nothing but the `attributes` rule reaches, and pins the index-consumption behaviour alongside. The fixture was named `reject-` while expecting INTEGRITY-ERROR, and its description opened "REJECT." flat, contradicting its own floor. Renamed to `integrity-error-attributes-id-collision`, matching the two existing collision fixtures, and the disposition stated as the state-invariant Error it is.
The correctness review found the branch rejecting documents main accepts, measured across 30,000 generated documents: 1,590 regressions, all one error class, and the only new rejection class on the branch. Extending item 5's `b<number>` prohibition to position-preserved ids made third-party vocabulary keys a hard canonicalization error. `b1`, `b2` is the CrossRef/JATS reference convention, so CSL exported from such a source carries exactly those keys — the data class this branch exists to protect. Worse, it was asymmetric: the identical CSL object in a `semantic:bibliography` `entries` array was accepted and kept its keys, while under `attributes` it was rejected. The erratum's own prose calls those two "the same reasoning applied to the same data one field over"; the code did the opposite. `collectDefinedIds` now reports `preservedByType` alongside `preserved`, and only the type-keyed set carries the prohibition. The two positions now behave identically, which is what the prose already claimed. Item 5 is corrected in two places. The prohibition sentence now says it does not extend to a position-preserved id, with the reason. And the membership sentence said "An id carried under `attributes`", whose plain reading makes every id there a namespace member — including an untyped array item, which the reference accepts as a duplicate. It now reads "An id that would otherwise be a member of this namespace", matching what `definedId` admits. The raw walk's `preserved` output was asserted by nothing: two barrier mutations survived all 30 gates. The differential now compares `preserved` as well as `duplicates` — its docstring already claimed to assert the raw walk reproduces canon, and it was checking one field of three. Two cases were added to reach it, since every existing `attributes` row REJECTs and the comparison skips those; the second sits in a mark INSIDE `attributes`, the only shape that catches the raw walk's dedicated `marks` branch dropping the flag. Both mutants now die.
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.
Closes the collision half of #125. An
idcarried under a block'sattributeswas being relabeled by §4.3.1 item 5, applying alpha-equivalence to third-party vocabulary data where an id is content rather than a label.The defect
Measured on
main:Two bibliographies whose citation keys differ entirely hashed identically, so a signature over one verified the other. Whether it fired turned on
typeversus@type, leaving well-formed JSON-LD accidentally immune and CSL-JSON exposed. Item 5 already exempts asemantic:bibliographyentry'sidfor exactly this reason; the same data underattributeswas not.PRESERVED, not excluded
The distinction is the whole fix. Excluding takes the id out of the namespace but leaves
rewriteIdsmapping it onto a generated name — trading a loud failure for a silent one. That approach was implemented and reverted on the #128 branch. A preserved id stays in the map's INPUT and out of the map, sorenameIdreturns it verbatim, reusing the mechanismsemantic:termandsemantic:footnotealready use.The duplicate cases still REJECT, and that is correct: the premise that nothing can reference material under
attributesis false as implemented — alinkmark'shrefresolves against such an id, so a duplicate there leaves a reference with two candidate referents.§4.3.1 item 5 is amended to license this, since it previously called the two type-keyed exceptions "the only namespace members whose ids are left as authored". Shipping the behaviour without the amendment is the defect #126 was filed about; this branch did it, and the review caught it.
What this does NOT fix
The collision class remains open, and the entry says so. The same harm is reachable through any extension data array not literally named
entries— avendor:refsblock with acitationarray still gives one document ID for two entirely different key sets. Item 5 already states the broader rule; the reference implements it as a one-element denylist. Membership also still turns ontypeversus@type. Both aredefinedIdkeying membership on element shape where item 5 says position, and both are #132.Review
Two cycles: adversarial-implementer, then correctness. Every finding closed or filed.
The correctness cycle's clean result is the important one. A 30,000-document differential fuzz against
main, coverage floor measured (6,718 documents carrying a preserved id, 3,277 carrying both preserved and relabeled):mainrejects that HEAD accepts: 0That is the axis this file has failed on twice, and it is clean.
What the cycles found, all fixed here:
b<number>prohibition, extended to position-preserved ids, rejectedb1/b2— the CrossRef/JATS reference convention, so ordinary CSL bibliographies. 1,590 of 30,000 documents, the only new rejection class on the branch. Worse, it was asymmetric: the identical data in anentriesarray was accepted. Now scoped to the type-keyed exception, and the two positions behave identically — which is what the erratum's prose already claimed.attributesrule reproduced the expected bytes. It now uses a singular sub-object, which nothing else reaches.attributes; the reference only covers whatdefinedIdadmits. Narrowed.linkhref resolves into it and is rewritten" was false — it stays verbatim, correctly, since a preserved id takes no canonical name.reject-while expecting INTEGRITY-ERROR. Renamed to match the two existing collision fixtures.preservedoutput was asserted by nothing: two barrier mutations survived all 30 gates. The differential now compares it, with two cases added to reach it — every pre-existingattributesrow REJECTs, and the comparison skips those.Verification
30/30 gates, plus the CI steps that are not npm scripts (
npm audit --audit-level=high→ 0 vulnerabilities,generate:templateinto a fresh dir).check:conformance: 380/380 (216 vectors + 164 fixtures), 0 failed, 0 advisory, 0 skipped. suiteVersion 0.2.4 → 0.2.5.The vector and fixture kill different mutants: relabeling returning is caught only by the vector; the reverted exclude-from-namespace design is caught only by the fixture, and by nothing in the suite before now — which is how that design reached a branch.