Name the usage a 51Did was created for, and take the raw byte surface off FodId - #192
Merged
Conversation
FodId exposed the flags byte and a type read from bits 6-7, and nothing for the usage in bits 0-2, so a caller deciding whether an identifier may be passed to a demand source had to mask the byte by hand. The three usages are cumulative in the byte, non-marketing 0b001, standard 0b011 and personalized 0b111, so a caller masking for the non-marketing bit alone would read every marketing identifier as non-marketing, the wrong way round for a data protection decision. Usage names the four states, NONE where no bit is set, and the usage getter answers with the highest granted. usageFromConsent reads bit 3, which records that the usage came from a consent string rather than being stated. Usage.idUsage gives the cloud's id.usage value. The names are the same in every 51Did package, which all gain the accessor together. Found by the Trusted Server work, which must work only through this package. fiftyone.pipeline.did tests: 144 passed, 2 skipped, five new.
The package gave a caller two ways to read a 51Did, being a named accessor for each field and the raw byte with the offsets to read the same field by hand. The second way is the one that produces the wrong answer, because the usage bits are cumulative, so code masking the byte for the non-marketing bit alone reads every marketing identifier as non-marketing, which is backwards for a rule saying a non-marketing identifier must never reach a demand source. These packages are not widely deployed, so the raw way is removed rather than deprecated. Four things have gone from the public surface. 1. fodId.flags. Every bit of the byte now has a name, so the byte stays on the instance as the private _flags and the typed getters usage, usageFromConsent and type read it. 2. fodId.hash, HASH_OFFSET and HASH_LENGTH. These were the aliases kept when the match key was renamed, and nothing calls them. 3. The nine byte layout constants FLAGS_OFFSET, LICENSE_ID_OFFSET, LICENSE_ID_LENGTH, MATCH_KEY_OFFSET, MATCH_KEY_LENGTH, HEADER_LENGTH, GUID_LENGTH, RANDOM_PAYLOAD_LENGTH and PAYLOAD_LENGTH. They now live in internal/layout.js, which index.js does not export and which the new exports map in package.json puts out of reach of a deep path from outside the package. The package's own tests, the offline example and didClient reach it by relative path because they build or measure payloads. 4. fodId.dateMinutes. No caller uses it, the Rust package never had it, and the one purpose the documentation gave it, which was the OWID public key request, is built by DidClient itself. The unsigned reading it forced has moved into fodId.date, so date reports the same number. Every typed accessor is unchanged, being usage, usageFromConsent, type, licenseId, matchKey and the OWID level fields. The documentation now points at the specification instead of repeating the byte table, because the layout is written once for all languages at https://github.com/51Degrees/specifications/blob/main/did-specification/identifier-layout.md and what each package offers on top of it at https://github.com/51Degrees/specifications/blob/main/did-specification/package-surface.md Both pages sit on an open pull request and will not resolve until it merges. The readme now documents the usage accessor, which it did not before, and carries a short migration table for the removed names. The generated type declarations were rebuilt, so they now declare usage and usageFromConsent, which they were missing, and export Usage from the package entry point. Where the exact byte is the point of a test, the test reads the private _flags field, because the typed getters cannot see bits 4 and 5. The tests that asserted on dateMinutes assert the same facts through date, and a new test checks that no layout constant is reachable through the package entry point. Closes #193
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.
Two changes to the same surface, in the order they were made.
1. Name the usage a 51Did was created for
FodIdgainsusage(None, NonMarketing, Standard, Personalized) andusageFromConsent, reading bits 0 to 2 and bit 3 of the flags byte. The usage bits are cumulative, so the accessor answers the highest usage granted rather than leaving callers to mask bits and misread every marketing identifier as non-marketing. Same accessor, same names, in all six 51Did packages. Found by the Trusted Server work, which must work only through this package.2. Take the raw byte surface off FodId
This is issue 193, raised on all six packages so the surface stays the same in every language. Once every bit has a name, the raw byte and the offsets are the remaining way to get it wrong, so they go.
flagsgetter is removed. The byte stays as the private_flags, which the typed getters read and which the tests that pin the byte now read, because those tests exist to prove every bit survives the read and the typed getters cannot see bits 4 and 5.hashgetter,HASH_OFFSETandHASH_LENGTHare deleted.internal/layout.js, whichindex.jsdoes not export.package.jsonandremote_package.jsongain anexportsmap so.and./package.jsonare the only reachable subpaths, since without one the internal module would still have been reachable by path from outside. No package in this repository had anexportsmap before, and nothing here imports the 51Did package by name, so nothing is narrowed that was being used.dateMinutesgetter is removed. In this portdatewas already the same integer, being minutes since 2020 as the OWID JavaScript library represents it, sodateMinuteswas a duplicate. The unsigned reading it forced moves intodate, which keeps the behaviour for a date past the signed range.The open question, decided
The issue left the raw date minutes as a candidate rather than a definite. It goes, in all five packages that have it, and Node is the case where it was a duplicate of
daterather than a raw encoding beside a typed one.Regenerated type declarations
Rebuilding
types/for this package caught three things that had been changed in source and never regenerated:Usagewas not exported and had no declaration file,usageandusageFromConsentwere missing from theFodIddeclaration, and the client declaration lacked theMISCONFIGUREDandINVALID_DATEoutcomes. Those are fixed here. Only this package was rebuilt, so the stale declarations in the other four are left for their own change.The specification
The byte structure is now specified once for all six languages, on specifications 25, at Identifier layout with the surface each package exposes at Package surface. The class documentation and the README link there instead of repeating the offset table, and the README gains a table of every removed name and its replacement. Those links resolve once that pull request merges.
Verification
The warning count was 112 before this change and is 106 after, so none is new. The integration test needs a resource key and a live cloud, so it was not run. The offline example runs and prints the usage.
Closes #193
Produced with AI assistance and needs human review.