Skip to content

Refuse a short 51Did payload of another version naming that version, and mark the raw flags byte deprecated - #115

Merged
jwrosewell merged 2 commits into
rcfrom
fix/did-reader-conformance
Nov 14, 2026
Merged

jwrosewell merged 2 commits into
rcfrom
fix/did-reader-conformance

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  1. A short payload that names another version is refused as too short, and the version it names is not reported. unpack checks the length against version 0's header before it reads the version (pipeline/fiftyone.pipeline.did/internal/payload.js lines 88 to 113 at 6d5d4fef), so a payload of 1 to 4 bytes whose flags byte names version 1, 2 or 3 gets PayloadTooShort, measured against a header that a later version need not have. A payload of normal length is refused naming its version, and tested (tests/fodId.test.js lines 611 to 632).
  2. The raw flags byte is part of the typed surface. FodId declares _flags: number (types/fodId.d.ts line 189), set at fodId.js line 132 and line 480. The published fiftyone.pipeline.did 4.5.102 declares it too (its types/fodId.d.ts line 177), where the offsets and the Terms index are kept in modules that are not exported.

What the specification says

From did-specification/identifier-layout.md lines 260 to 265 at 706de1f0:

A reader MUST refuse a Payload whose version it does not know. It MUST report it the way it reports a Payload it cannot read, naming the version it found, and it MUST NOT read the fields as though the version were 0. A later version exists precisely because a field moved, so reading it under the old layout returns values that are wrong rather than absent, which is worse than refusing.

From did-specification/package-surface.md lines 129 to 134 at 706de1f0:

A package MUST NOT make any of the following public, in any language.

| The raw flags byte | Every bit of it has a name. The byte is the way back to masking, and masking for the non-marketing bit reads every marketing identifier as non-marketing. |

The Rust package reads the version first and tests it (fodid/src/fodid.rs lines 460 to 477 at 113b963b).

What changed

  1. unpack reads the version from the flags byte before it checks the length, whenever the payload has a flags byte, and refuses a version it does not know with UnsupportedPayloadVersion naming the version. A payload of version 0 that is too short is still PayloadTooShort, and an empty payload still is too.
  2. _flags is marked deprecated in fodId.js and in types/fodId.d.ts, pointing at type, usage and usageIsIndirect, which name its bits. It is not removed, because a patch release never removes a public name, and _flags is in the published 4.5.102 types. A later release removes it.

Tests

The new block "FodId short payloads of another version" in pipeline/fiftyone.pipeline.did/tests/fodId.test.js checks that payloads of 1, 2 and 4 bytes naming versions 1, 2 and 3 are refused by tryParse with UnsupportedPayloadVersion and no value, and that fromBase64 throws with that status and a message naming the version. The existing test that payloads of 0 to 4 bytes of version 0 are refused as too short is unchanged and passes.

Red, run 43614524319 on 92f5f2f, the test commit on rc's code. On Ubuntu and on Windows the four new tests fail, each payload refused as PayloadTooShort, and the 864 other tests pass.

Green, run 43614540094 on 06aadbd, this head, where every test passes on Ubuntu and on Windows.

The deprecation is a comment, so no test covers it.

Full build

Run 43615889364 on tmp/pr-matrix-115, started by tools/pr-matrix.py without macOS. All 7 Setting 'PullRequestSha' lines give this head, 06aadbd, and Configure printed "Left out 2 macOS entries". Each of the six Build and Test jobs, Ubuntu, Ubuntu ARM and Windows on Node.js 22 and 24, passed 1,497 unit tests with 2 skipped and 96 integration tests with 4 skipped.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

@jwrosewell
jwrosewell marked this pull request as ready for review November 14, 2026 23:04
@jwrosewell
jwrosewell merged commit 4abd77b into rc Nov 14, 2026
3 checks passed
@jwrosewell
jwrosewell deleted the fix/did-reader-conformance branch November 14, 2026 23:04
@jwrosewell jwrosewell mentioned this pull request Nov 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant