From edb86342001f66bf291fb4b600ef7504fb7d14ab Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 15:27:09 +0100 Subject: [PATCH 1/6] FEAT: Carry the terms a 51Did was created under in the identifier A 51Did created for marketing may only be used by a receiver that has accepted the terms it was created under, and until now the identifier did not say which terms those are, so the answer had to travel beside it. In OpenRTB there is somewhere to put it, being the Terms Document Locator on the eids entry, and everywhere else there is not. An identifier passed as a query string parameter arrives on its own and any hop can drop a sidecar without the identifier looking any different, so a receiver that gets one without its terms cannot act on it safely. One byte after the match key now answers that, and it is an index into a table in the specification rather than a version number, so a later document can live at any address rather than only at one a number could compose. Terms names the values, being NOT_STATED for index 0, MODEL_TERMS_FOR_MARKETING_2 for index 1 and UNKNOWN for an index this package does not know. FodId.getTerms() answers the named value, getTermsIndex() the raw byte and getTermsUrl() the address, which is null for both NOT_STATED and UNKNOWN and is never an empty string and never built from the index. The package never fetches the address, it returns it. UNKNOWN is deliberately not NOT_STATED. Zero says no terms are stated whilst an unknown index says terms are stated that this package cannot name, and a receiver confusing the two would read an identifier created under terms as one created under none. The raw index is exposed alongside the named value so that a caller meeting an index added after this release can look the document up by hand and can report which index it could not read. Existing identifiers are unaffected. One issued before the byte existed has a payload that ends at the match key, and a missing byte reads as index 0, which says the terms are not stated in the identifier, so absence and zero mean the same thing, no reader has to tell them apart and no presence flag is needed. The byte adds nothing to the minimum payload lengths this package enforces, so no identifier that read before fails to read now, and every existing test passes untouched. Depends on 51Degrees/specifications#27, which must merge first. pipeline.did tests: 137 run, 0 failures, 2 skipped (the live cloud tests), five new. The fodid developer example prints the three members and its tests run 9, 0 failures. --- .../developerexamples/fodid/Main.java | 15 +- pipeline.did/README.md | 70 ++++++++- .../java/fiftyone/pipeline/did/FodId.java | 85 ++++++++++- .../java/fiftyone/pipeline/did/Terms.java | 127 ++++++++++++++++ .../fiftyone/pipeline/did/package-info.java | 11 +- .../pipeline/did/FodIdParseTests.java | 137 ++++++++++++++++++ .../pipeline/did/FodIdTestFactory.java | 43 ++++++ 7 files changed, 470 insertions(+), 18 deletions(-) create mode 100644 pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java diff --git a/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java b/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java index 4612ed689..784b04e51 100644 --- a/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java +++ b/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java @@ -33,8 +33,8 @@ *

* The 51Degrees Cloud service issues real 51Dids. To keep this example * self-contained and offline, it builds a sample 51Did in process - generate - * an ECDSA P-256 key pair, sign a canonical 37-byte payload - then parses it - * back with {@link FodId} and prints the three payload fields. + * an ECDSA P-256 key pair, sign a canonical 38-byte payload - then parses it + * back with {@link FodId} and prints the four payload fields. *

* It also demonstrates the headline use case: a 51Did is re-issued fresh on * every call (the envelope, hence the base64, changes), but the match key @@ -53,8 +53,9 @@ public class Main { private static final int LICENSE_ID_OFFSET = 1; private static final int MATCH_KEY_OFFSET = 5; private static final int MATCH_KEY_LENGTH = 32; - private static final int PAYLOAD_LENGTH = + private static final int TERMS_OFFSET = MATCH_KEY_OFFSET + MATCH_KEY_LENGTH; + private static final int PAYLOAD_LENGTH = TERMS_OFFSET + 1; public static class Example { @@ -77,6 +78,9 @@ public void run() throws Exception { System.out.println(" LicenseId : " + fodId.getLicenseId()); System.out.println(" Match key : " + toHex(fodId.getMatchKey())); + System.out.println(" Terms : " + fodId.getTerms() + + " (index " + fodId.getTermsIndex() + ")"); + System.out.println(" Terms url : " + fodId.getTermsUrl()); System.out.println(" Verifies : " + fodId.verify(crypto.publicKeyPem())); @@ -128,6 +132,11 @@ private byte[] samplePayload() { for (int i = 0; i < MATCH_KEY_LENGTH; i++) { payload[MATCH_KEY_OFFSET + i] = (byte) (0x20 + i); } + // Terms index 1, being the Model Terms for Marketing version 2, + // which is the document a marketing identifier is created + // under. An identifier issued before this byte existed ends at + // the match key and reads as NOT_STATED. + payload[TERMS_OFFSET] = 1; return payload; } diff --git a/pipeline.did/README.md b/pipeline.did/README.md index 5561edb3a..4beaa6da9 100644 --- a/pipeline.did/README.md +++ b/pipeline.did/README.md @@ -32,9 +32,9 @@ layout out from this package. What follows is a summary of the part that changes how this package behaves. The identifier carries a five byte header of Flags and License Id, then the -match key, then an optional creator context section. Bits 6-7 of Flags -select the type, which decides how long the match key is and so what the -least a payload can hold is. +match key, then the Terms byte, then an optional creator context section. +Bits 6-7 of Flags select the type, which decides how long the match key is +and so what the least a payload can hold is. | Bits 7-6 | `IdType` | Match key length | Minimum payload | |---------:|-----------------|-------------:|----------------:| @@ -46,6 +46,12 @@ least a payload can hold is. Identifiers issued before the type tag existed have bits 6-7 zeroed and decode as `PROBABILISTIC`. +The Terms byte is not counted in those minimums. An identifier issued before +the byte existed has a payload that ends at the match key, and a missing +byte is read as index zero, which says the terms are not stated in the +identifier. Absence and zero mean the same thing, so no reader has to tell +them apart and no presence flag exists. See the terms section below. + The minimums in that table are the only lengths this package enforces. There is no upper bound. An identifier carrying a creator context is longer than the minimum, its extra bytes have a shape only the cloud knows, and a reader @@ -177,6 +183,7 @@ apart from "the signature could not be checked" (`KEY_UNAVAILABLE`, ```java import fiftyone.pipeline.did.FodId; import fiftyone.pipeline.did.IdType; +import fiftyone.pipeline.did.Terms; import fiftyone.pipeline.did.Usage; import java.time.Instant; @@ -187,6 +194,9 @@ Usage usage = fodId.getUsage(); // what the identifier may be used fo boolean fromConsent = fodId.isUsageFromConsent(); long licenseId = fodId.getLicenseId(); byte[] matchKey = fodId.getMatchKey(); // SHA-256 or GUID bytes, see type +Terms terms = fodId.getTerms(); // which terms it was created under +int termsIndex = fodId.getTermsIndex(); +String termsUrl = fodId.getTermsUrl(); // null where none is stated // Delegated OWID-level fields and operations. String domain = fodId.getDomain(); @@ -244,6 +254,60 @@ if (fodId.getUsage() == Usage.NON_MARKETING) { } ``` +## Which terms a 51Did was created under + +`getTerms()` answers which terms document the identifier was created under. +The answer travels inside the identifier, so a receiver always has it, +rather than depending on the surrounding protocol to carry the terms +alongside the identifier where any hop can drop them without the identifier +looking any different. + +The byte after the match key is an index into a table in the specification +and is not a version number, so that a later document can live at any +address rather than only at one a number could compose. An index is never +reused and never repointed once published, because repointing one would +rewrite what an identifier already issued says it agreed to. + +| `Terms` | Index | `getTermsUrl()` | What it means | +|---|---|---|---| +| `NOT_STATED` | `0` | `null` | The terms are not stated in the identifier, so take them from the data accompanying it. | +| `MODEL_TERMS_FOR_MARKETING_2` | `1` | `https://m4ow.uk/mtm/2.txt` | The Model Terms for Marketing, version 2. | +| `UNKNOWN` | any other | `null` | An index added to the specification after this package was released. | + +`UNKNOWN` is not `NOT_STATED`. Zero says no terms are stated, whilst an +unknown index says terms are stated that this package cannot name, and code +that treated the two alike would read an identifier created under terms as +one created under none. `getTermsIndex()` gives the raw index whatever the +answer is, which is the one raw value a 51Did offers, so a caller meeting an +index this package does not know can look the document up in the +specification by hand and can report which index it could not read. + +```java +if (fodId.getTerms() == Terms.UNKNOWN) { + // Terms are stated that this package cannot name. Take a newer package + // or refuse the identifier, reporting fodId.getTermsIndex(). +} +``` + +`NOT_STATED` does not mean the identifier is unrestricted. It means only +that the identifier does not carry the answer, so the answer has to come +from somewhere else, being the Terms Document Locator in an OpenRTB request +or whatever the surrounding protocol provides. Carrying the terms in the +identifier does not remove the need to carry a locator where a protocol has +one, and where the two disagree the identifier's own value is the one that +describes the identifier, because it is inside the signature and the +accompanying data is not. + +The terms and the usage answer different questions and a receiver needs +both. `getUsage()` says where an identifier may go and `getTerms()` says +which document it was created under. An identifier created for non-marketing +carries `NOT_STATED`, because the Model Terms govern marketing use and a +non-marketing identifier is not created under them, and it stays barred from +a demand source by its usage. + +This package never fetches the address. It returns it and the receiver +decides what to do with it. + ## Verifying on your server `DidClient` handles every manipulation of a 51Did a server needs against the diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java index 9f12be7cf..c216a25cb 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java @@ -50,12 +50,16 @@ * Payload layout. Read a 51Did through the typed accessors below, never by * walking the payload bytes. The identifier carries a five byte header of * Flags and License Id, then the match key, whose length the identifier - * type in bits 6-7 of Flags decides, and then an optional creator context + * type in bits 6-7 of Flags decides, then the Terms byte naming the terms + * document it was created under, and then an optional creator context * section that binds the identifier to the browser and connection it was * created on. Only 51Degrees can read that section, so this reader exposes - * it only as the part of {@link #getPayload()} beyond the match key, its + * it only as the part of {@link #getPayload()} beyond the Terms, its * lengths belong to the cloud, and this reader therefore puts no upper - * bound on a payload. The byte layout is specified at + * bound on a payload. An identifier issued before the Terms existed has a + * payload that ends at the match key, and a missing byte reads as + * {@link Terms#NOT_STATED}, so nothing about such an identifier changes. + * The byte layout is specified at * identifier-layout.md, * which is the authority for it, and the surface every 51Did package * offers is specified at @@ -130,21 +134,35 @@ public final class FodId { static final int PAYLOAD_LENGTH = MATCH_KEY_OFFSET + MATCH_KEY_LENGTH; + /** + * Byte length of the Terms field, which follows the match key. It is + * not part of any minimum above, because an identifier issued before + * the field existed ends at the match key and reads as a Terms of zero. + */ + static final int TERMS_LENGTH = 1; + private final Owid owid; private final int flags; private final long licenseId; private final byte[] matchKey; + private final int termsIndex; /** * Built only by {@link #read(Owid)} once the payload has passed the * 51Did rules, so an instance never exists for a payload that failed * them. */ - private FodId(Owid owid, int flags, long licenseId, byte[] matchKey) { + private FodId( + Owid owid, + int flags, + long licenseId, + byte[] matchKey, + int termsIndex) { this.owid = owid; this.flags = flags; this.licenseId = licenseId; this.matchKey = matchKey; + this.termsIndex = termsIndex; } // ----- Reading without throwing ----- @@ -206,8 +224,10 @@ private static FodIdParseResult read(OwidParseResult envelope) { * The rules are lower bounds only. The header must be present before the * type can be read, and the type then sets the least the payload can * hold. Anything longer is accepted as it stands, because the bytes past - * the match key are a creator context section whose shape the cloud - * judges. + * the match key are the Terms and then a creator context section whose + * shape the cloud judges. The Terms adds nothing to those bounds, since + * an identifier issued before the field existed ends at the match key + * and reads as a Terms of zero. */ private static FodIdParseResult read(Owid owid) { byte[] payload = owid.getPayload(); @@ -246,8 +266,17 @@ private static FodIdParseResult read(Owid owid) { // bytes. byte[] matchKey = Arrays.copyOfRange( payload, MATCH_KEY_OFFSET, MATCH_KEY_OFFSET + matchKeyLength); + // The Terms byte follows the match key, wherever the type put its + // end. A payload that stops there was issued before the field + // existed, and a missing byte is read as zero, which says the terms + // are not stated in the identifier. Absence and zero therefore mean + // the same thing and nothing has to tell them apart. + int termsOffset = MATCH_KEY_OFFSET + matchKeyLength; + int termsIndex = payload.length > termsOffset + ? payload[termsOffset] & 0xFF + : 0; return FodIdParseResult.parsed( - new FodId(owid, flags, licenseId, matchKey)); + new FodId(owid, flags, licenseId, matchKey, termsIndex)); } // ----- Reading with exceptions ----- @@ -440,6 +469,48 @@ public byte[] getMatchKey() { return matchKey.clone(); } + /** + * The terms document this 51Did was created under, from the Terms byte + * that follows the match key. See {@link Terms} for what each answer + * means, and read {@link #getTermsIndex()} alongside this where the + * answer is {@link Terms#UNKNOWN}. + * + * @return the terms document, {@link Terms#NOT_STATED} where the + * identifier does not state them, or {@link Terms#UNKNOWN} + * where it states an index this package does not know + */ + public Terms getTerms() { + return Terms.fromIndex(termsIndex); + } + + /** + * The raw Terms index (0 to 255). This is the one raw value a 51Did + * offers, where the Flags byte is not offered at all, and it is here + * because a caller meeting an index added to the specification after + * this package was released otherwise holds {@link Terms#UNKNOWN} and + * has no way to find out what it stands for. With the index that caller + * can look the document up in the specification by hand and can report + * which index it could not read. An identifier whose payload ends at + * the match key answers 0. + * + * @return the Terms index from the payload (0-255) + */ + public int getTermsIndex() { + return termsIndex; + } + + /** + * The address of the terms document this 51Did was created under. This + * package never fetches it and the receiver decides what to do with it. + * + * @return the address, or null where the terms are not stated and where + * the index is one this package does not know, which is never + * an empty string and is never an address built from the index + */ + public String getTermsUrl() { + return getTerms().getUrl(); + } + /** @return the OWID version. */ public Version getVersion() { return owid.getVersion(); diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java new file mode 100644 index 000000000..df723ef12 --- /dev/null +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java @@ -0,0 +1,127 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.did; + +/** + * The terms document a 51Did was created under, carried in the byte after + * the match key. It travels inside the identifier so that a receiver always + * has the terms the identifier was created under, rather than depending on + * the surrounding protocol to carry them alongside it, which any hop can + * drop without the identifier looking any different. + *

+ * The byte is an index into a table in the specification and is not a + * version number, so that a later document can live at any address rather + * than only at one a number could compose. An index is never reused and + * never repointed once published, because repointing one would rewrite what + * an identifier already issued says it agreed to. A new document is a new + * index, and every 51Did package has to be released to know it, which is + * the cost of a receiver being able to trust what it reads. + *

+ * {@link #UNKNOWN} is not {@link #NOT_STATED}. This package will meet an + * index added to the specification after it was released, and reading that + * as {@link #NOT_STATED} would read an identifier created under terms as + * one created under none. {@link FodId#getTermsIndex()} gives the index + * whatever the answer is, so a caller meeting {@link #UNKNOWN} can look the + * document up in the specification by hand and can report which index it + * could not read, and it should treat the identifier as covered by terms it + * cannot yet read and either take a newer package or refuse the identifier. + *

+ * {@link #NOT_STATED} does not mean the identifier is unrestricted. It + * means only that the identifier does not carry the answer, so the answer + * has to come from somewhere else, being the Terms Document Locator in an + * OpenRTB request or whatever the surrounding protocol provides. Carrying + * the terms in the identifier does not remove the need to carry a locator + * where a protocol has one, and where the two disagree the identifier's own + * value is the one that describes the identifier, because it is inside the + * signature and the accompanying data is not. + *

+ * The terms and the {@link Usage} answer different questions and a receiver + * needs both. The usage says where an identifier may go and the terms say + * which document it was created under. An identifier created for + * non-marketing carries {@link #NOT_STATED}, because the Model Terms govern + * marketing use and a non-marketing identifier is not created under them, + * and it stays barred from a demand source by its usage. + *

+ * The values are the same in every 51Did package. The table is specified at + * identifier-layout.md, + * which is the authority for it. + */ +public enum Terms { + /** + * Index 0, the terms are not stated in the identifier and the receiver + * has to take them from the data accompanying it. An identifier issued + * before the byte existed ends at the match key and reads as this, so + * absence and zero mean the same thing. + */ + NOT_STATED(null), + + /** + * Index 1, the Model Terms for Marketing, version 2. + */ + MODEL_TERMS_FOR_MARKETING_2("https://m4ow.uk/mtm/2.txt"), + + /** + * An index added to the specification after this package was released. + * Terms are stated and this package cannot name them, so it answers + * with no address, and {@link FodId#getTermsIndex()} says which index it + * could not read. + */ + UNKNOWN(null); + + private final String url; + + Terms(String url) { + this.url = url; + } + + /** + * Reads the terms from the Terms index carried after the match key. An + * index this package does not know answers {@link #UNKNOWN} rather than + * {@link #NOT_STATED}, because the two say different things. + * + * @param index the 1-byte Terms index (0-255) + * @return the terms document the index stands for + */ + public static Terms fromIndex(int index) { + switch (index) { + case 0: + return NOT_STATED; + case 1: + return MODEL_TERMS_FOR_MARKETING_2; + default: + return UNKNOWN; + } + } + + /** + * The address of the terms document. No package ever fetches it, and + * the receiver decides what to do with it. + * + * @return the address, or null for {@link #NOT_STATED} and for + * {@link #UNKNOWN}, which is never an empty string and is never + * an address built from the index + */ + public String getUrl() { + return url; + } +} diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/package-info.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/package-info.java index 89208e901..f417df2de 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/package-info.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/package-info.java @@ -30,11 +30,12 @@ * {@link fiftyone.pipeline.did.FodIdParseResult} instead of throwing, whose * {@link fiftyone.pipeline.did.FodIdParseStatus} names why an input is not * a 51Did, and the {@code from} readers make the same read and throw. A - * 51Did exposes the three payload fields (Flags, License Id and the match - * key) and the identifier {@link fiftyone.pipeline.did.IdType}, and - * delegates OWID-level concerns to the envelope it holds. Reading never - * checks the signature. Compare 51Dids by their match key - * ({@code getMatchKey()}), never by their envelopes. + * 51Did exposes the four payload fields (Flags, License Id, the match key + * and the {@link fiftyone.pipeline.did.Terms} it was created under) and the + * identifier {@link fiftyone.pipeline.did.IdType}, and delegates OWID-level + * concerns to the envelope it holds. Reading never checks the signature. + * Compare 51Dids by their match key ({@code getMatchKey()}), never by their + * envelopes. *

* {@link fiftyone.pipeline.did.DidClient} is what a server uses against the * 51Degrees cloud: it fetches and holds the published signing keys, verifies diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index 76f19756b..0eede7f0a 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java @@ -39,7 +39,10 @@ import static fiftyone.pipeline.did.FodIdTestFactory.TEST_DOMAIN; import static fiftyone.pipeline.did.FodIdTestFactory.canonicalPayload; import static fiftyone.pipeline.did.FodIdTestFactory.canonicalPayloadWithSection; +import static fiftyone.pipeline.did.FodIdTestFactory.canonicalPayloadWithTerms; +import static fiftyone.pipeline.did.FodIdTestFactory.canonicalPayloadWithTermsAndSection; import static fiftyone.pipeline.did.FodIdTestFactory.canonicalRandomPayload; +import static fiftyone.pipeline.did.FodIdTestFactory.canonicalRandomPayloadWithTerms; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -211,6 +214,140 @@ public void isUsageFromConsent_IsBitThree() throws Exception { assertEquals(Usage.STANDARD, fodId.getUsage()); } + /** + * An identifier issued before the Terms byte existed has a payload that + * ends at the match key. A missing byte is read as index zero, which + * says the terms are not stated in the identifier, so absence and zero + * mean the same thing and such an identifier reads as it always did. + */ + @Test + public void getTerms_NoByteAfterTheMatchKey_NotStatedWithNoUrl() + throws Exception { + for (byte[] payload : new byte[][] { + canonicalPayload(), canonicalRandomPayload() }) { + FodId fodId = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(payload, DATE).asBase64())); + + assertEquals(Terms.NOT_STATED, fodId.getTerms()); + assertEquals(0, fodId.getTermsIndex()); + assertNull(fodId.getTermsUrl()); + } + } + + /** + * Index one is the Model Terms for Marketing version 2, whose address is + * answered exactly as the specification writes it and is never fetched. + * The byte is read after the match key, whose length the type sets, so + * both match key lengths are checked and neither loses a byte to the + * Terms. + */ + @Test + public void getTerms_IndexOne_ModelTermsWithItsUrlForBothKeyLengths() + throws Exception { + FodId probabilistic = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalPayloadWithTerms(1), DATE) + .asBase64())); + FodId random = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalRandomPayloadWithTerms(1), DATE) + .asBase64())); + + for (FodId fodId : new FodId[] { probabilistic, random }) { + assertEquals(Terms.MODEL_TERMS_FOR_MARKETING_2, fodId.getTerms()); + assertEquals(1, fodId.getTermsIndex()); + assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTermsUrl()); + } + // The 32-byte match key and the 16-byte one are both the same as + // they are without the Terms byte, so nothing was taken from either. + assertEquals(IdType.HASHED_EMAIL, probabilistic.getType()); + assertArrayEquals(CANONICAL_MATCH_KEY, probabilistic.getMatchKey()); + assertEquals(IdType.RANDOM, random.getType()); + assertEquals(FodId.GUID_LENGTH, random.getMatchKey().length); + assertArrayEquals( + assertParsed(FodId.tryFromBase64(factory + .signedOwidAt(canonicalRandomPayload(), DATE).asBase64())) + .getMatchKey(), + random.getMatchKey()); + } + + /** + * An index added to the specification after this package was released is + * reported as itself and answers with no address, and it is not the + * value for zero. Zero says no terms are stated whilst an unknown index + * says terms are stated that this package cannot name, and a receiver + * confusing the two would read an identifier created under terms as one + * created under none. + */ + @Test + public void getTerms_IndexThisPackageDoesNotKnow_ReportedAndNotZero() + throws Exception { + FodId notStated = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalPayloadWithTerms(0), DATE) + .asBase64())); + + for (int index : new int[] { 200, 255 }) { + FodId unknown = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalPayloadWithTerms(index), DATE) + .asBase64())); + + // The index is reported as written, so a caller can name the one + // it could not read. The byte is unsigned, so 200 is 200 and 255 + // is 255 rather than a negative number. + assertEquals("terms index " + index, + index, unknown.getTermsIndex()); + assertEquals("terms index " + index, + Terms.UNKNOWN, unknown.getTerms()); + assertNull("terms index " + index, unknown.getTermsUrl()); + // Zero and an unknown index are told apart, by the named value + // and by the index. + assertNotEquals(notStated.getTerms(), unknown.getTerms()); + assertNotEquals( + notStated.getTermsIndex(), unknown.getTermsIndex()); + } + assertEquals(Terms.NOT_STATED, notStated.getTerms()); + assertEquals(0, notStated.getTermsIndex()); + assertNull(notStated.getTermsUrl()); + } + + /** + * Every index the table does not carry is unknown and has no address, + * across the whole byte, so none of them can be read as zero. + */ + @Test + public void terms_EveryIndexOutsideTheTable_IsUnknownWithNoUrl() { + assertEquals(Terms.NOT_STATED, Terms.fromIndex(0)); + assertNull(Terms.NOT_STATED.getUrl()); + assertEquals(Terms.MODEL_TERMS_FOR_MARKETING_2, Terms.fromIndex(1)); + assertEquals("https://m4ow.uk/mtm/2.txt", + Terms.MODEL_TERMS_FOR_MARKETING_2.getUrl()); + for (int index = 2; index <= 255; index++) { + assertEquals("terms index " + index, + Terms.UNKNOWN, Terms.fromIndex(index)); + assertNull("terms index " + index, + Terms.fromIndex(index).getUrl()); + } + } + + /** + * The Terms sits before the creator context section, so a payload + * carrying both still reads the match key and the Terms from the places + * they are written at, and the section is exposed as it always was. + */ + @Test + public void getTerms_ByteThenContextSection_ReadAtTheRightOffset() + throws Exception { + byte[] payload = canonicalPayloadWithTermsAndSection(1, 512); + + FodId fodId = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(payload, DATE).asBase64())); + + assertArrayEquals(CANONICAL_MATCH_KEY, fodId.getMatchKey()); + assertEquals(Terms.MODEL_TERMS_FOR_MARKETING_2, fodId.getTerms()); + assertEquals(1, fodId.getTermsIndex()); + assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTermsUrl()); + assertArrayEquals(payload, fodId.getPayload()); + assertTrue(fodId.verify(factory.publicPem)); + } + @Test public void tryFromBase64_ReservedHeaderOnly_ParsedBestEffort() throws Exception { diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java index 083c9f26f..5736c5672 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java @@ -101,6 +101,49 @@ static byte[] canonicalRandomPayload() { return payload; } + /** + * The canonical payload with a Terms byte written after its 32-byte + * match key, which is how the cloud issues one now. + */ + static byte[] canonicalPayloadWithTerms(int termsIndex) { + byte[] payload = + new byte[FodId.PAYLOAD_LENGTH + FodId.TERMS_LENGTH]; + System.arraycopy( + canonicalPayload(), 0, payload, 0, FodId.PAYLOAD_LENGTH); + payload[FodId.PAYLOAD_LENGTH] = (byte) termsIndex; + return payload; + } + + /** + * The canonical Random payload with a Terms byte written after its + * 16-byte match key, so that the byte sits at a different offset from + * the one {@link #canonicalPayloadWithTerms(int)} puts it at. + */ + static byte[] canonicalRandomPayloadWithTerms(int termsIndex) { + byte[] payload = + new byte[FodId.RANDOM_PAYLOAD_LENGTH + FodId.TERMS_LENGTH]; + System.arraycopy( + canonicalRandomPayload(), 0, payload, 0, + FodId.RANDOM_PAYLOAD_LENGTH); + payload[FodId.RANDOM_PAYLOAD_LENGTH] = (byte) termsIndex; + return payload; + } + + /** + * The canonical payload with a Terms byte and then a creator context + * section after it, which is the order the two sit in. + */ + static byte[] canonicalPayloadWithTermsAndSection( + int termsIndex, int sectionLength) { + byte[] withTerms = canonicalPayloadWithTerms(termsIndex); + byte[] payload = new byte[withTerms.length + sectionLength]; + System.arraycopy(withTerms, 0, payload, 0, withTerms.length); + for (int i = withTerms.length; i < payload.length; i++) { + payload[i] = (byte) 0xCC; + } + return payload; + } + static byte[] canonicalPayloadWithSection(int sectionLength) { byte[] payload = new byte[FodId.PAYLOAD_LENGTH + sectionLength]; System.arraycopy( From 3621ba6cbe0dafb003210ae34b82a9a19d9a0bd5 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 16:59:41 +0100 Subject: [PATCH 2/6] Keep the change inside the 51Did package The example module is outside pipeline.did and printing the new members there is a separate change, so it is taken back out and the diff is the package alone. --- .../pipeline/developerexamples/fodid/Main.java | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java b/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java index 784b04e51..4612ed689 100644 --- a/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java +++ b/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/Main.java @@ -33,8 +33,8 @@ *

* The 51Degrees Cloud service issues real 51Dids. To keep this example * self-contained and offline, it builds a sample 51Did in process - generate - * an ECDSA P-256 key pair, sign a canonical 38-byte payload - then parses it - * back with {@link FodId} and prints the four payload fields. + * an ECDSA P-256 key pair, sign a canonical 37-byte payload - then parses it + * back with {@link FodId} and prints the three payload fields. *

* It also demonstrates the headline use case: a 51Did is re-issued fresh on * every call (the envelope, hence the base64, changes), but the match key @@ -53,9 +53,8 @@ public class Main { private static final int LICENSE_ID_OFFSET = 1; private static final int MATCH_KEY_OFFSET = 5; private static final int MATCH_KEY_LENGTH = 32; - private static final int TERMS_OFFSET = + private static final int PAYLOAD_LENGTH = MATCH_KEY_OFFSET + MATCH_KEY_LENGTH; - private static final int PAYLOAD_LENGTH = TERMS_OFFSET + 1; public static class Example { @@ -78,9 +77,6 @@ public void run() throws Exception { System.out.println(" LicenseId : " + fodId.getLicenseId()); System.out.println(" Match key : " + toHex(fodId.getMatchKey())); - System.out.println(" Terms : " + fodId.getTerms() - + " (index " + fodId.getTermsIndex() + ")"); - System.out.println(" Terms url : " + fodId.getTermsUrl()); System.out.println(" Verifies : " + fodId.verify(crypto.publicKeyPem())); @@ -132,11 +128,6 @@ private byte[] samplePayload() { for (int i = 0; i < MATCH_KEY_LENGTH; i++) { payload[MATCH_KEY_OFFSET + i] = (byte) (0x20 + i); } - // Terms index 1, being the Model Terms for Marketing version 2, - // which is the document a marketing identifier is created - // under. An identifier issued before this byte existed ends at - // the match key and reads as NOT_STATED. - payload[TERMS_OFFSET] = 1; return payload; } From 2f122bead10f18e841afe18e6b84f572696bcdb2 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 18:23:30 +0100 Subject: [PATCH 3/6] DOC: Describe the Terms byte as it is, rather than as a change No reader has ever seen a 51Did without the Terms byte, so describing the field as a change from a previous state gives a reader history they cannot use. Every rule the history was wrapped around is kept and reworded to describe the payload instead. A payload that ends at the match key has no Terms byte, and a missing byte reads as index zero, which says the terms are not stated in the identifier, so absence and zero mean the same thing and no presence flag is needed. The Terms still adds nothing to the minimum payload lengths the package enforces. --- pipeline.did/README.md | 10 ++++----- .../java/fiftyone/pipeline/did/FodId.java | 21 +++++++++---------- .../java/fiftyone/pipeline/did/Terms.java | 6 +++--- .../pipeline/did/FodIdParseTests.java | 8 +++---- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/pipeline.did/README.md b/pipeline.did/README.md index 4beaa6da9..89fb2fd6c 100644 --- a/pipeline.did/README.md +++ b/pipeline.did/README.md @@ -46,11 +46,11 @@ and so what the least a payload can hold is. Identifiers issued before the type tag existed have bits 6-7 zeroed and decode as `PROBABILISTIC`. -The Terms byte is not counted in those minimums. An identifier issued before -the byte existed has a payload that ends at the match key, and a missing -byte is read as index zero, which says the terms are not stated in the -identifier. Absence and zero mean the same thing, so no reader has to tell -them apart and no presence flag exists. See the terms section below. +The Terms byte is not counted in those minimums. An identifier whose +payload ends at the match key has no Terms byte, and a missing byte is read +as index zero, which says the terms are not stated in the identifier. +Absence and zero mean the same thing, so no reader has to tell them apart +and no presence flag exists. See the terms section below. The minimums in that table are the only lengths this package enforces. There is no upper bound. An identifier carrying a creator context is longer than diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java index c216a25cb..a558fde16 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java @@ -56,9 +56,9 @@ * created on. Only 51Degrees can read that section, so this reader exposes * it only as the part of {@link #getPayload()} beyond the Terms, its * lengths belong to the cloud, and this reader therefore puts no upper - * bound on a payload. An identifier issued before the Terms existed has a - * payload that ends at the match key, and a missing byte reads as - * {@link Terms#NOT_STATED}, so nothing about such an identifier changes. + * bound on a payload. A payload that ends at the match key has no Terms + * byte, and a missing byte reads as {@link Terms#NOT_STATED}, so absence + * and zero mean the same thing. * The byte layout is specified at * identifier-layout.md, * which is the authority for it, and the surface every 51Did package @@ -136,8 +136,8 @@ public final class FodId { /** * Byte length of the Terms field, which follows the match key. It is - * not part of any minimum above, because an identifier issued before - * the field existed ends at the match key and reads as a Terms of zero. + * not part of any minimum above, because a payload that ends at the + * match key reads as a Terms of zero. */ static final int TERMS_LENGTH = 1; @@ -226,8 +226,7 @@ private static FodIdParseResult read(OwidParseResult envelope) { * hold. Anything longer is accepted as it stands, because the bytes past * the match key are the Terms and then a creator context section whose * shape the cloud judges. The Terms adds nothing to those bounds, since - * an identifier issued before the field existed ends at the match key - * and reads as a Terms of zero. + * a payload that ends at the match key reads as a Terms of zero. */ private static FodIdParseResult read(Owid owid) { byte[] payload = owid.getPayload(); @@ -267,10 +266,10 @@ private static FodIdParseResult read(Owid owid) { byte[] matchKey = Arrays.copyOfRange( payload, MATCH_KEY_OFFSET, MATCH_KEY_OFFSET + matchKeyLength); // The Terms byte follows the match key, wherever the type put its - // end. A payload that stops there was issued before the field - // existed, and a missing byte is read as zero, which says the terms - // are not stated in the identifier. Absence and zero therefore mean - // the same thing and nothing has to tell them apart. + // end. A payload that stops there has no Terms byte, and a missing + // byte is read as zero, which says the terms are not stated in the + // identifier. Absence and zero therefore mean the same thing and + // nothing has to tell them apart. int termsOffset = MATCH_KEY_OFFSET + matchKeyLength; int termsIndex = payload.length > termsOffset ? payload[termsOffset] & 0xFF diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java index df723ef12..daf3d20ff 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java @@ -69,9 +69,9 @@ public enum Terms { /** * Index 0, the terms are not stated in the identifier and the receiver - * has to take them from the data accompanying it. An identifier issued - * before the byte existed ends at the match key and reads as this, so - * absence and zero mean the same thing. + * has to take them from the data accompanying it. An identifier whose + * payload ends at the match key reads as this, so absence and zero + * mean the same thing. */ NOT_STATED(null), diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index 0eede7f0a..ad83a6459 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java @@ -215,10 +215,10 @@ public void isUsageFromConsent_IsBitThree() throws Exception { } /** - * An identifier issued before the Terms byte existed has a payload that - * ends at the match key. A missing byte is read as index zero, which - * says the terms are not stated in the identifier, so absence and zero - * mean the same thing and such an identifier reads as it always did. + * An identifier whose payload ends at the match key has no Terms byte. + * A missing byte is read as index zero, which says the terms are not + * stated in the identifier, so absence and zero mean the same thing and + * nothing has to tell them apart. */ @Test public void getTerms_NoByteAfterTheMatchKey_NotStatedWithNoUrl() From cda58efe0edb4fd64cd6657b037ea45088274113 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 19:05:22 +0100 Subject: [PATCH 4/6] FEAT: Refuse a payload version this package cannot read, and answer the Terms with its address Bits 4 and 5 of the Flags byte are the payload version. This package reads version 0 and refuses any other with FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, with the throwing readers naming the version they found. No field is read under the layout this package knows once the version says otherwise, because a later version exists precisely because a field moved, so reading such a payload here would answer with values that are wrong rather than absent. The version is not exposed, because either the package read the layout or there is no identifier to read fields from. FodIdParseResult carries the version it found so the message can name it, and that field is not public. The Terms is one member rather than three. getTerms() answers with the address of the document the identifier was created under, and the package turns the index into the address so a caller never handles the byte. The raw index and the separate address member are gone and the Terms enumeration is package-private. An index of zero and an index this package cannot name both answer with no address, which a caller cannot tell apart, and that is deliberate because both say the identifier does not give the terms and the answer has to come from somewhere else. No address is ever built from an index, since that would name a document nobody wrote. The test factory is the creating side, so it writes both new fields. The canonical Flags byte carries version 0, the canonical payload carries the Terms of a personalized marketing identifier and the canonical Random payload carries the zero a non-marketing identifier carries. A payload that ends at the match key is now a fixture of its own, since a reader takes it as a Terms of zero and no issuer would write one. pipeline.did tests: 142 run, 0 failures, 0 errors, 2 skipped, being the live cloud tests. Javadoc builds with no warnings. --- pipeline.did/README.md | 73 ++++--- .../java/fiftyone/pipeline/did/FodId.java | 110 +++++++---- .../pipeline/did/FodIdParseResult.java | 38 +++- .../pipeline/did/FodIdParseStatus.java | 11 +- .../java/fiftyone/pipeline/did/Terms.java | 25 +-- .../pipeline/did/FodIdParseTests.java | 186 ++++++++++++++---- .../pipeline/did/FodIdTestFactory.java | 52 ++++- .../fiftyone/pipeline/did/FodIdTests.java | 11 +- 8 files changed, 379 insertions(+), 127 deletions(-) diff --git a/pipeline.did/README.md b/pipeline.did/README.md index 89fb2fd6c..227c01eb2 100644 --- a/pipeline.did/README.md +++ b/pipeline.did/README.md @@ -52,6 +52,23 @@ as index zero, which says the terms are not stated in the identifier. Absence and zero mean the same thing, so no reader has to tell them apart and no presence flag exists. See the terms section below. +## The payload version + +Bits 4 and 5 of the Flags byte say which payload layout the identifier +follows, and this package reads version 0. A payload naming version 1, 2 +or 3 is refused with `FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION`, and +the throwing readers name the version they found in the message. + +The fields are never read under the layout this package knows once the +version says otherwise. A later version exists precisely because a field +moved, so reading such a payload here would answer with values that are +wrong rather than absent, which is worse than refusing. A version that +nothing checks protects nothing. + +The version is not exposed. Either this package read the layout, in which +case the accessors are the answer, or it did not, in which case there is +no identifier to read fields from. + The minimums in that table are the only lengths this package enforces. There is no upper bound. An identifier carrying a creator context is longer than the minimum, its extra bytes have a shape only the cloud knows, and a reader @@ -183,7 +200,6 @@ apart from "the signature could not be checked" (`KEY_UNAVAILABLE`, ```java import fiftyone.pipeline.did.FodId; import fiftyone.pipeline.did.IdType; -import fiftyone.pipeline.did.Terms; import fiftyone.pipeline.did.Usage; import java.time.Instant; @@ -194,9 +210,10 @@ Usage usage = fodId.getUsage(); // what the identifier may be used fo boolean fromConsent = fodId.isUsageFromConsent(); long licenseId = fodId.getLicenseId(); byte[] matchKey = fodId.getMatchKey(); // SHA-256 or GUID bytes, see type -Terms terms = fodId.getTerms(); // which terms it was created under -int termsIndex = fodId.getTermsIndex(); -String termsUrl = fodId.getTermsUrl(); // null where none is stated +String terms = fodId.getTerms(); // address of the terms document + // it was created under, null + // where it names none this + // package knows // Delegated OWID-level fields and operations. String domain = fodId.getDomain(); @@ -256,11 +273,12 @@ if (fodId.getUsage() == Usage.NON_MARKETING) { ## Which terms a 51Did was created under -`getTerms()` answers which terms document the identifier was created under. -The answer travels inside the identifier, so a receiver always has it, -rather than depending on the surrounding protocol to carry the terms -alongside the identifier where any hop can drop them without the identifier -looking any different. +`getTerms()` answers with the address of the terms document the identifier +was created under. The answer travels inside the identifier, so a receiver +always has it, rather than depending on the surrounding protocol to carry +the terms alongside the identifier where any hop can drop them without the +identifier looking any different. The package turns the index into the +address, so a caller never handles the byte. The byte after the match key is an index into a table in the specification and is not a version number, so that a later document can live at any @@ -268,28 +286,27 @@ address rather than only at one a number could compose. An index is never reused and never repointed once published, because repointing one would rewrite what an identifier already issued says it agreed to. -| `Terms` | Index | `getTermsUrl()` | What it means | -|---|---|---|---| -| `NOT_STATED` | `0` | `null` | The terms are not stated in the identifier, so take them from the data accompanying it. | -| `MODEL_TERMS_FOR_MARKETING_2` | `1` | `https://m4ow.uk/mtm/2.txt` | The Model Terms for Marketing, version 2. | -| `UNKNOWN` | any other | `null` | An index added to the specification after this package was released. | +| Index | Document | `getTerms()` | +|---|---|---| +| `0` | Not stated in the identifier | `null` | +| `1` | Model Terms for Marketing, version 2 | `https://m4ow.uk/mtm/2.txt` | +| any other | One this package cannot name | `null` | -`UNKNOWN` is not `NOT_STATED`. Zero says no terms are stated, whilst an -unknown index says terms are stated that this package cannot name, and code -that treated the two alike would read an identifier created under terms as -one created under none. `getTermsIndex()` gives the raw index whatever the -answer is, which is the one raw value a 51Did offers, so a caller meeting an -index this package does not know can look the document up in the -specification by hand and can report which index it could not read. +An index added to the specification after this package was released answers +with no address, and the package never builds an address from the index, +because that would name a document nobody wrote and a receiver would record +having accepted terms that do not exist. A caller therefore cannot tell an +index of zero from an index this package cannot name, which is deliberate, +since both lead to the same place. ```java -if (fodId.getTerms() == Terms.UNKNOWN) { - // Terms are stated that this package cannot name. Take a newer package - // or refuse the identifier, reporting fodId.getTermsIndex(). +if (fodId.getTerms() == null) { + // The identifier does not say which terms it was created under, so the + // answer has to come from the data accompanying it. } ``` -`NOT_STATED` does not mean the identifier is unrestricted. It means only +No address does not mean the identifier is unrestricted. It means only that the identifier does not carry the answer, so the answer has to come from somewhere else, being the Terms Document Locator in an OpenRTB request or whatever the surrounding protocol provides. Carrying the terms in the @@ -301,9 +318,9 @@ accompanying data is not. The terms and the usage answer different questions and a receiver needs both. `getUsage()` says where an identifier may go and `getTerms()` says which document it was created under. An identifier created for non-marketing -carries `NOT_STATED`, because the Model Terms govern marketing use and a -non-marketing identifier is not created under them, and it stays barred from -a demand source by its usage. +carries index zero and so answers with no address, because the Model Terms +govern marketing use and a non-marketing identifier is not created under +them, and it stays barred from a demand source by its usage. This package never fetches the address. It returns it and the receiver decides what to do with it. diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java index a558fde16..3136751b6 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java @@ -57,8 +57,17 @@ * it only as the part of {@link #getPayload()} beyond the Terms, its * lengths belong to the cloud, and this reader therefore puts no upper * bound on a payload. A payload that ends at the match key has no Terms - * byte, and a missing byte reads as {@link Terms#NOT_STATED}, so absence - * and zero mean the same thing. + * byte, and a missing byte reads as a Terms of zero, so absence and zero + * mean the same thing. + *

+ * Bits 4 and 5 of the Flags byte say which payload layout the identifier + * follows, and this package reads version 0. A payload naming any other + * version is refused with + * {@link FodIdParseStatus#UNSUPPORTED_PAYLOAD_VERSION} rather than read + * under the layout this package knows, because a later version exists + * precisely because a field moved, so reading one here would answer with + * values that are wrong rather than absent. The version is not exposed, + * because a caller has nothing to decide with it. * The byte layout is specified at * identifier-layout.md, * which is the authority for it, and the surface every 51Did package @@ -141,6 +150,13 @@ public final class FodId { */ static final int TERMS_LENGTH = 1; + /** + * The payload layout version this package reads, carried in bits 4 and + * 5 of the Flags byte. Any other version is refused rather than read + * under this layout. + */ + static final int SUPPORTED_PAYLOAD_VERSION = 0; + private final Owid owid; private final int flags; private final long licenseId; @@ -234,6 +250,16 @@ private static FodIdParseResult read(Owid owid) { return FodIdParseResult.failed(FodIdParseStatus.PAYLOAD_TOO_SHORT); } int flags = payload[FLAGS_OFFSET] & 0xFF; + // The version is read before any field, because a later version + // exists precisely because a field moved. Reading a payload of a + // version this package does not know under the layout it does know + // would answer with values that are wrong rather than absent, + // which is worse than refusing, and a version that nothing checks + // protects nothing. + int payloadVersion = payloadVersionOf(flags); + if (payloadVersion != SUPPORTED_PAYLOAD_VERSION) { + return FodIdParseResult.unsupportedPayloadVersion(payloadVersion); + } int matchKeyLength; switch (IdType.fromFlags(flags)) { case RANDOM: @@ -377,6 +403,19 @@ public static FodId fromOwid(Owid owid) throws OwidException { * failure is an OWID one, which is the split the readers have always * made. The message names the status and the parameter, never the input. */ + /** + * Bits 4 and 5 of the Flags byte, being the version of the payload + * layout the identifier follows. The envelope carries a version of its + * own at its first byte, which versions the envelope, whilst this one + * versions the payload. + * + * @param flags the Flags byte + * @return the payload layout version (0 to 3) + */ + private static int payloadVersionOf(int flags) { + return (flags >> 4) & 0b11; + } + private static FodId valueOrThrow(FodIdParseResult result, String paramName) throws OwidException { switch (result.getStatus()) { @@ -390,6 +429,11 @@ private static FodId valueOrThrow(FodIdParseResult result, String paramName) throw new IllegalArgumentException( "51Did payload is shorter than the minimum for its " + "identifier type (" + paramName + ")."); + case UNSUPPORTED_PAYLOAD_VERSION: + throw new IllegalArgumentException( + "51Did payload version " + result.getPayloadVersion() + + " is not one this package can read (" + + paramName + ")."); default: throw new OwidException( "The value is not an OWID envelope: " @@ -469,45 +513,33 @@ public byte[] getMatchKey() { } /** - * The terms document this 51Did was created under, from the Terms byte - * that follows the match key. See {@link Terms} for what each answer - * means, and read {@link #getTermsIndex()} alongside this where the - * answer is {@link Terms#UNKNOWN}. - * - * @return the terms document, {@link Terms#NOT_STATED} where the - * identifier does not state them, or {@link Terms#UNKNOWN} - * where it states an index this package does not know - */ - public Terms getTerms() { - return Terms.fromIndex(termsIndex); - } - - /** - * The raw Terms index (0 to 255). This is the one raw value a 51Did - * offers, where the Flags byte is not offered at all, and it is here - * because a caller meeting an index added to the specification after - * this package was released otherwise holds {@link Terms#UNKNOWN} and - * has no way to find out what it stands for. With the index that caller - * can look the document up in the specification by hand and can report - * which index it could not read. An identifier whose payload ends at - * the match key answers 0. - * - * @return the Terms index from the payload (0-255) - */ - public int getTermsIndex() { - return termsIndex; - } - - /** - * The address of the terms document this 51Did was created under. This - * package never fetches it and the receiver decides what to do with it. + * The address of the terms document this 51Did was created under, from + * the Terms byte that follows the match key. + *

+ * The byte is an index into a table in the specification and this + * package turns the index into the address, so a caller never handles + * the byte. The address is answered and never fetched, and the receiver + * decides what to do with the document. + *

+ * Null covers both an index of zero, which says the terms are not + * stated in the identifier, and an index added to the specification + * after this package was released, which it cannot name. A caller + * cannot tell those two apart, which is deliberate, because both lead + * to the same place, being that the identifier does not say which terms + * it was created under and the answer has to come from somewhere else. + * No package may build an address from an index it does not know, since + * that would name a document nobody wrote. + *

+ * No address does not mean the identifier is unrestricted. Where an + * identifier may go is a separate question {@link #getUsage()} answers, + * which still bars a non-marketing identifier from a demand source. * - * @return the address, or null where the terms are not stated and where - * the index is one this package does not know, which is never - * an empty string and is never an address built from the index + * @return the address of the terms document, or null where the + * identifier names no document this package knows, which is + * never an empty string and is never built from the index */ - public String getTermsUrl() { - return getTerms().getUrl(); + public String getTerms() { + return Terms.fromIndex(termsIndex).getUrl(); } /** @return the OWID version. */ diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseResult.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseResult.java index 545a2a92b..e282b880b 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseResult.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseResult.java @@ -49,17 +49,49 @@ public final class FodIdParseResult { private final FodIdParseStatus status; - private FodIdParseResult(FodId value, FodIdParseStatus status) { + private final int payloadVersion; + + private FodIdParseResult( + FodId value, FodIdParseStatus status, int payloadVersion) { this.value = value; this.status = status; + this.payloadVersion = payloadVersion; } static FodIdParseResult parsed(FodId value) { - return new FodIdParseResult(value, FodIdParseStatus.PARSED); + return new FodIdParseResult( + value, FodIdParseStatus.PARSED, FodId.SUPPORTED_PAYLOAD_VERSION); } static FodIdParseResult failed(FodIdParseStatus status) { - return new FodIdParseResult(null, status); + return new FodIdParseResult( + null, status, FodId.SUPPORTED_PAYLOAD_VERSION); + } + + /** + * A read refused because the payload names a layout version this + * package does not know, carrying the version so that the throwing + * readers can name it in their message. The version is not public, + * because a caller has nothing to decide with it. + * + * @param payloadVersion the version the payload named + * @return the refused read + */ + static FodIdParseResult unsupportedPayloadVersion(int payloadVersion) { + return new FodIdParseResult( + null, + FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, + payloadVersion); + } + + /** + * The payload layout version a refused read found, for the message the + * throwing readers give. Zero for every other outcome. + * + * @return the version the payload named + */ + int getPayloadVersion() { + return payloadVersion; } /** diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseStatus.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseStatus.java index 447f1c07a..64858b810 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseStatus.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/FodIdParseStatus.java @@ -114,7 +114,16 @@ public enum FodIdParseStatus { * anything past the match key is a creator context section whose lengths * belong to the cloud. */ - INVALID_TYPE_PAYLOAD_LENGTH; + INVALID_TYPE_PAYLOAD_LENGTH, + + /** + * Bits 4 and 5 of the Flags byte name a payload layout version this + * package does not know, so no field is read. A later version exists + * precisely because a field moved, so reading the payload under the + * layout this package knows would answer with values that are wrong + * rather than absent. + */ + UNSUPPORTED_PAYLOAD_VERSION; /** * Carries an OWID status across unchanged. diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java index daf3d20ff..355cbc12e 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java @@ -37,14 +37,17 @@ * index, and every 51Did package has to be released to know it, which is * the cost of a receiver being able to trust what it reads. *

- * {@link #UNKNOWN} is not {@link #NOT_STATED}. This package will meet an - * index added to the specification after it was released, and reading that - * as {@link #NOT_STATED} would read an identifier created under terms as - * one created under none. {@link FodId#getTermsIndex()} gives the index - * whatever the answer is, so a caller meeting {@link #UNKNOWN} can look the - * document up in the specification by hand and can report which index it - * could not read, and it should treat the identifier as covered by terms it - * cannot yet read and either take a newer package or refuse the identifier. + * This enumeration is not public, and neither is the index behind it. The + * package turns the index into the address that {@link FodId#getTerms()} + * answers with, so a caller never handles the byte, and the names here are + * the ones the specification gives so that every package describes one + * document the same way. + *

+ * {@link #UNKNOWN} is an index added to the specification after this + * package was released, so the package cannot name the document. It + * answers with no address, as {@link #NOT_STATED} does, because no package + * may build an address from an index it does not know, since that would + * name a document nobody wrote. *

* {@link #NOT_STATED} does not mean the identifier is unrestricted. It * means only that the identifier does not carry the answer, so the answer @@ -66,7 +69,7 @@ * identifier-layout.md, * which is the authority for it. */ -public enum Terms { +enum Terms { /** * Index 0, the terms are not stated in the identifier and the receiver * has to take them from the data accompanying it. An identifier whose @@ -75,6 +78,7 @@ public enum Terms { */ NOT_STATED(null), + /** * Index 1, the Model Terms for Marketing, version 2. */ @@ -83,8 +87,7 @@ public enum Terms { /** * An index added to the specification after this package was released. * Terms are stated and this package cannot name them, so it answers - * with no address, and {@link FodId#getTermsIndex()} says which index it - * could not read. + * with no address rather than building one from the index. */ UNKNOWN(null); diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index ad83a6459..25601ba7d 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java @@ -43,7 +43,10 @@ import static fiftyone.pipeline.did.FodIdTestFactory.canonicalPayloadWithTermsAndSection; import static fiftyone.pipeline.did.FodIdTestFactory.canonicalRandomPayload; import static fiftyone.pipeline.did.FodIdTestFactory.canonicalRandomPayloadWithTerms; +import static fiftyone.pipeline.did.FodIdTestFactory.payloadEndingAtMatchKey; +import static fiftyone.pipeline.did.FodIdTestFactory.randomPayloadEndingAtMatchKey; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.fail; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -218,22 +221,39 @@ public void isUsageFromConsent_IsBitThree() throws Exception { * An identifier whose payload ends at the match key has no Terms byte. * A missing byte is read as index zero, which says the terms are not * stated in the identifier, so absence and zero mean the same thing and - * nothing has to tell them apart. + * nothing has to tell them apart, and the identifier answers with no + * address. */ @Test - public void getTerms_NoByteAfterTheMatchKey_NotStatedWithNoUrl() + public void getTerms_NoByteAfterTheMatchKey_HasNoAddress() throws Exception { for (byte[] payload : new byte[][] { - canonicalPayload(), canonicalRandomPayload() }) { + payloadEndingAtMatchKey(), + randomPayloadEndingAtMatchKey() }) { FodId fodId = assertParsed(FodId.tryFromBase64( factory.signedOwidAt(payload, DATE).asBase64())); - assertEquals(Terms.NOT_STATED, fodId.getTerms()); - assertEquals(0, fodId.getTermsIndex()); - assertNull(fodId.getTermsUrl()); + assertNull(fodId.getTerms()); } } + /** + * A Terms byte holding zero answers exactly as no byte at all does, so + * the two never have to be told apart. + */ + @Test + public void getTerms_ZeroByte_AnswersAsAbsenceDoes() throws Exception { + FodId absent = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(payloadEndingAtMatchKey(), DATE) + .asBase64())); + FodId zero = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalPayloadWithTerms(0), DATE) + .asBase64())); + + assertEquals(absent.getTerms(), zero.getTerms()); + assertNull(zero.getTerms()); + } + /** * Index one is the Model Terms for Marketing version 2, whose address is * answered exactly as the specification writes it and is never fetched. @@ -242,7 +262,7 @@ public void getTerms_NoByteAfterTheMatchKey_NotStatedWithNoUrl() * Terms. */ @Test - public void getTerms_IndexOne_ModelTermsWithItsUrlForBothKeyLengths() + public void getTerms_IndexOne_ModelTermsAddressForBothKeyLengths() throws Exception { FodId probabilistic = assertParsed(FodId.tryFromBase64( factory.signedOwidAt(canonicalPayloadWithTerms(1), DATE) @@ -252,9 +272,7 @@ public void getTerms_IndexOne_ModelTermsWithItsUrlForBothKeyLengths() .asBase64())); for (FodId fodId : new FodId[] { probabilistic, random }) { - assertEquals(Terms.MODEL_TERMS_FOR_MARKETING_2, fodId.getTerms()); - assertEquals(1, fodId.getTermsIndex()); - assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTermsUrl()); + assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTerms()); } // The 32-byte match key and the 16-byte one are both the same as // they are without the Terms byte, so nothing was taken from either. @@ -264,48 +282,38 @@ public void getTerms_IndexOne_ModelTermsWithItsUrlForBothKeyLengths() assertEquals(FodId.GUID_LENGTH, random.getMatchKey().length); assertArrayEquals( assertParsed(FodId.tryFromBase64(factory - .signedOwidAt(canonicalRandomPayload(), DATE).asBase64())) + .signedOwidAt(randomPayloadEndingAtMatchKey(), DATE) + .asBase64())) .getMatchKey(), random.getMatchKey()); } /** - * An index added to the specification after this package was released is - * reported as itself and answers with no address, and it is not the - * value for zero. Zero says no terms are stated whilst an unknown index - * says terms are stated that this package cannot name, and a receiver - * confusing the two would read an identifier created under terms as one - * created under none. + * An index added to the specification after this package was released + * answers with no address, and no address is ever built from the index, + * because that would name a document nobody wrote. A caller cannot tell + * such an index from zero, which is deliberate, since both say the + * identifier does not give the terms and the answer has to come from + * somewhere else. The byte is unsigned in the read, so 255 is 255 and + * not a negative number. */ @Test - public void getTerms_IndexThisPackageDoesNotKnow_ReportedAndNotZero() + public void getTerms_IndexThisPackageDoesNotKnow_HasNoAddress() throws Exception { FodId notStated = assertParsed(FodId.tryFromBase64( factory.signedOwidAt(canonicalPayloadWithTerms(0), DATE) .asBase64())); - for (int index : new int[] { 200, 255 }) { + for (int index : new int[] { 2, 127, 200, 255 }) { FodId unknown = assertParsed(FodId.tryFromBase64( factory.signedOwidAt(canonicalPayloadWithTerms(index), DATE) .asBase64())); - // The index is reported as written, so a caller can name the one - // it could not read. The byte is unsigned, so 200 is 200 and 255 - // is 255 rather than a negative number. + assertNull("terms index " + index, unknown.getTerms()); assertEquals("terms index " + index, - index, unknown.getTermsIndex()); - assertEquals("terms index " + index, - Terms.UNKNOWN, unknown.getTerms()); - assertNull("terms index " + index, unknown.getTermsUrl()); - // Zero and an unknown index are told apart, by the named value - // and by the index. - assertNotEquals(notStated.getTerms(), unknown.getTerms()); - assertNotEquals( - notStated.getTermsIndex(), unknown.getTermsIndex()); + Terms.UNKNOWN, Terms.fromIndex(index)); } - assertEquals(Terms.NOT_STATED, notStated.getTerms()); - assertEquals(0, notStated.getTermsIndex()); - assertNull(notStated.getTermsUrl()); + assertNull(notStated.getTerms()); } /** @@ -327,6 +335,112 @@ public void terms_EveryIndexOutsideTheTable_IsUnknownWithNoUrl() { } } + // ----- The payload version ----- + + /** + * The payload with its version bits set to the given version, leaving + * every other bit of the Flags byte alone. + */ + private static byte[] withVersion(byte[] payload, int version) { + byte[] withVersion = payload.clone(); + withVersion[FodId.FLAGS_OFFSET] = (byte) ( + (payload[FodId.FLAGS_OFFSET] & 0b1100_1111) + | (version << 4)); + return withVersion; + } + + /** + * A Flags byte with bits 4 and 5 clear is version 0, which is the + * layout this package reads, so every field reads as it does on the + * canonical payload. + */ + @Test + public void version_Zero_ReadsEveryField() throws Exception { + FodId fodId = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(canonicalPayload(), DATE).asBase64())); + + assertEquals(IdType.HASHED_EMAIL, fodId.getType()); + assertEquals(Usage.PERSONALIZED, fodId.getUsage()); + assertArrayEquals(CANONICAL_MATCH_KEY, fodId.getMatchKey()); + assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTerms()); + } + + /** + * Versions 1, 2 and 3 are not assigned, so a payload naming one is + * refused rather than read under the layout this package knows. + */ + @Test + public void version_NotZero_IsRefused() throws Exception { + for (int version : new int[] { 1, 2, 3 }) { + FodIdParseResult result = FodId.tryFromBase64(factory + .signedOwidAt(withVersion(canonicalPayload(), version), DATE) + .asBase64()); + + assertFalse("version " + version, result.isSuccess()); + assertEquals("version " + version, + FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, + result.getStatus()); + // Nothing is handed back, rather than a value with some fields + // filled in, because there is no identifier to expose fields + // for when the layout was not understood. + assertNull("version " + version, result.getValue()); + } + } + + /** + * The throwing readers name the version they found, so whoever reads + * the message knows which layout the identifier claims rather than only + * that some version was refused. + */ + @Test + public void version_NotZero_MessageNamesTheVersion() throws Exception { + for (int version : new int[] { 1, 2, 3 }) { + String base64 = factory + .signedOwidAt(withVersion(canonicalPayload(), version), DATE) + .asBase64(); + try { + FodId.fromBase64(base64); + fail("version " + version + " should have been refused"); + } catch (IllegalArgumentException thrown) { + assertTrue(thrown.getMessage(), + thrown.getMessage().contains("version " + version)); + } + } + } + + /** + * The version bits are read on their own, so an identifier of version 0 + * still reads whatever its usage and type bits hold, and one of another + * version is refused whatever they hold. A reader masking the wrong + * bits would fail one of these. + */ + @Test + public void version_IsReadApartFromTheUsageAndTypeBits() + throws Exception { + for (int usage : new int[] { 0b000, 0b001, 0b011, 0b111 }) { + for (int type : new int[] { 0b00, 0b10, 0b11 }) { + int flags = (type << 6) | usage; + byte[] payload = payloadEndingAtMatchKey(); + payload[FodId.FLAGS_OFFSET] = (byte) flags; + + assertTrue("flags " + flags, FodId.tryFromBase64( + factory.signedOwidAt(payload, DATE).asBase64()) + .isSuccess()); + + for (int version : new int[] { 1, 2, 3 }) { + FodIdParseResult refused = FodId.tryFromBase64(factory + .signedOwidAt(withVersion(payload, version), DATE) + .asBase64()); + + assertEquals("flags " + flags + " version " + version, + FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, + refused.getStatus()); + assertNull(refused.getValue()); + } + } + } + } + /** * The Terms sits before the creator context section, so a payload * carrying both still reads the match key and the Terms from the places @@ -341,9 +455,7 @@ public void getTerms_ByteThenContextSection_ReadAtTheRightOffset() factory.signedOwidAt(payload, DATE).asBase64())); assertArrayEquals(CANONICAL_MATCH_KEY, fodId.getMatchKey()); - assertEquals(Terms.MODEL_TERMS_FOR_MARKETING_2, fodId.getTerms()); - assertEquals(1, fodId.getTermsIndex()); - assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTermsUrl()); + assertEquals("https://m4ow.uk/mtm/2.txt", fodId.getTerms()); assertArrayEquals(payload, fodId.getPayload()); assertTrue(fodId.verify(factory.publicPem)); } diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java index 5736c5672..971e017ca 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTestFactory.java @@ -49,7 +49,20 @@ final class FodIdTestFactory { static final String TEST_DOMAIN = "51degrees.com"; - static final int CANONICAL_FLAGS = 0xA5; + static final int CANONICAL_FLAGS = 0x85; + + /** + * The Terms index a marketing identifier carries, being the Model Terms + * for Marketing version 2. + */ + static final int MARKETING_TERMS_INDEX = 1; + + /** + * The Terms index a non-marketing identifier carries, since the Model + * Terms govern marketing use and a non-marketing identifier is not + * created under them. + */ + static final int NON_MARKETING_TERMS_INDEX = 0; static final long CANONICAL_LICENSE_ID = 0x12345678L; @@ -81,7 +94,23 @@ private static byte[] canonicalMatchKey() { return matchKey; } + /** + * The canonical payload as an issuer writes one, carrying the payload + * version 0 in its Flags byte and the Terms byte of the document a + * personalized marketing identifier is created under. This is the + * creating side, so it writes every field an issuer writes. + */ static byte[] canonicalPayload() { + return canonicalPayloadWithTerms(MARKETING_TERMS_INDEX); + } + + /** + * The canonical payload cut off at the end of the match key, so it + * carries no Terms byte. A reader takes that as a Terms of zero, and + * this is the fixture for that rule rather than anything an issuer + * would write. + */ + static byte[] payloadEndingAtMatchKey() { byte[] payload = new byte[FodId.PAYLOAD_LENGTH]; payload[FodId.FLAGS_OFFSET] = (byte) CANONICAL_FLAGS; writeCanonicalLicenseId(payload); @@ -91,7 +120,20 @@ static byte[] canonicalPayload() { return payload; } + /** + * The canonical Random payload as an issuer writes one, carrying the + * payload version 0 and the zero Terms byte a non-marketing identifier + * carries. + */ static byte[] canonicalRandomPayload() { + return canonicalRandomPayloadWithTerms(NON_MARKETING_TERMS_INDEX); + } + + /** + * The canonical Random payload cut off at the end of its GUID, so it + * carries no Terms byte. + */ + static byte[] randomPayloadEndingAtMatchKey() { byte[] payload = new byte[FodId.RANDOM_PAYLOAD_LENGTH]; payload[FodId.FLAGS_OFFSET] = (byte) ((1 << 6) | 0b001); writeCanonicalLicenseId(payload); @@ -103,13 +145,13 @@ static byte[] canonicalRandomPayload() { /** * The canonical payload with a Terms byte written after its 32-byte - * match key, which is how the cloud issues one now. + * match key, which is how the cloud issues one. */ static byte[] canonicalPayloadWithTerms(int termsIndex) { byte[] payload = new byte[FodId.PAYLOAD_LENGTH + FodId.TERMS_LENGTH]; System.arraycopy( - canonicalPayload(), 0, payload, 0, FodId.PAYLOAD_LENGTH); + payloadEndingAtMatchKey(), 0, payload, 0, FodId.PAYLOAD_LENGTH); payload[FodId.PAYLOAD_LENGTH] = (byte) termsIndex; return payload; } @@ -123,7 +165,7 @@ static byte[] canonicalRandomPayloadWithTerms(int termsIndex) { byte[] payload = new byte[FodId.RANDOM_PAYLOAD_LENGTH + FodId.TERMS_LENGTH]; System.arraycopy( - canonicalRandomPayload(), 0, payload, 0, + randomPayloadEndingAtMatchKey(), 0, payload, 0, FodId.RANDOM_PAYLOAD_LENGTH); payload[FodId.RANDOM_PAYLOAD_LENGTH] = (byte) termsIndex; return payload; @@ -147,7 +189,7 @@ static byte[] canonicalPayloadWithTermsAndSection( static byte[] canonicalPayloadWithSection(int sectionLength) { byte[] payload = new byte[FodId.PAYLOAD_LENGTH + sectionLength]; System.arraycopy( - canonicalPayload(), 0, payload, 0, FodId.PAYLOAD_LENGTH); + payloadEndingAtMatchKey(), 0, payload, 0, FodId.PAYLOAD_LENGTH); for (int i = FodId.PAYLOAD_LENGTH; i < payload.length; i++) { payload[i] = (byte) 0xCC; } diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTests.java index 1bca88b54..b3234176b 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdTests.java @@ -178,13 +178,18 @@ public void flags_ZeroValue_Exposed() throws Exception { } @Test - public void flags_AllBitsSet_Exposed() throws Exception { + public void flags_EveryBitOutsideTheVersionSet_Exposed() + throws Exception { + // Bits 4 and 5 are the payload version and only version 0 is read, + // so every other bit is set and those two are left clear. A payload + // with them set is refused rather than read, which FodIdParseTests + // covers. byte[] payload = canonicalPayload(); - payload[FodId.FLAGS_OFFSET] = (byte) 0xFF; + payload[FodId.FLAGS_OFFSET] = (byte) 0xCF; FodId fodId = FodId.fromBase64(factory.signedOwidBase64(payload)); - assertEquals(255, fodId.getFlags()); + assertEquals(0xCF, fodId.getFlags()); } @Test From 4ad6cdb661f2aef29bd19714efe9701ff2f83fff Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 20:29:00 +0100 Subject: [PATCH 5/6] TEST: A Reserved identifier states no terms The Reserved type takes every byte after the header as the match key, so no byte is left to read as the Terms and getTerms() answers null. The other five packages each test this and Java did not. --- .../pipeline/did/FodIdParseTests.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index 25601ba7d..a2cf58712 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java @@ -460,6 +460,32 @@ public void getTerms_ByteThenContextSection_ReadAtTheRightOffset() assertTrue(fodId.verify(factory.publicPem)); } + /** + * The Reserved type has no defined match key length, so the reader + * takes every byte after the header as the match key and leaves none + * to read as the Terms. Such an identifier therefore states no terms, + * which is the right answer rather than a gap to close, because an + * identifier of a type this package cannot lay out is one whose Terms + * it cannot place either. + */ + @Test + public void getTerms_ReservedType_StatesNoTerms() throws Exception { + byte[] payload = canonicalPayloadWithTerms(1); + payload[FodId.FLAGS_OFFSET] = + (byte) ((CANONICAL_FLAGS & 0b0011_1111) | 0b1100_0000); + + FodId fodId = assertParsed(FodId.tryFromBase64( + factory.signedOwidAt(payload, DATE).asBase64())); + + assertEquals(IdType.RESERVED, fodId.getType()); + assertNull(fodId.getTerms()); + // The byte that would have been the Terms is inside the match key, + // which is what taking every byte after the header means. + assertEquals( + payload.length - FodId.HEADER_LENGTH, + fodId.getMatchKey().length); + } + @Test public void tryFromBase64_ReservedHeaderOnly_ParsedBestEffort() throws Exception { From 945f7e15a5beccefb64641fc6af262d581798b85 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 9 Sep 2026 21:32:10 +0100 Subject: [PATCH 6/6] REFACTOR: One terms table, so a new document is one member Adding a terms document meant editing the enumeration and a second switch that repeated the index, so the two could disagree. Each member now carries its index alongside its address, and the index to member map is built from the members in a static block rather than written out again, so fromIndex is one lookup and a new document is one new member and nothing else. A test walks every member and fails if one does not read back from its own index, or names a document with no address, which is what would happen if a member and its address were added apart. mvn test -pl pipeline.did: 34 in FodIdParseTests, 40 in FodIdTests, 0 failures. --- .../java/fiftyone/pipeline/did/Terms.java | 60 +++++++++++++++---- .../pipeline/did/FodIdParseTests.java | 37 ++++++++++++ 2 files changed, 85 insertions(+), 12 deletions(-) diff --git a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java index 355cbc12e..56a2fb14c 100644 --- a/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java +++ b/pipeline.did/src/main/java/fiftyone/pipeline/did/Terms.java @@ -22,6 +22,10 @@ package fiftyone.pipeline.did; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + /** * The terms document a 51Did was created under, carried in the byte after * the match key. It travels inside the identifier so that a receiver always @@ -76,27 +80,56 @@ enum Terms { * payload ends at the match key reads as this, so absence and zero * mean the same thing. */ - NOT_STATED(null), + NOT_STATED(0, null), /** * Index 1, the Model Terms for Marketing, version 2. */ - MODEL_TERMS_FOR_MARKETING_2("https://m4ow.uk/mtm/2.txt"), + MODEL_TERMS_FOR_MARKETING_2(1, "https://m4ow.uk/mtm/2.txt"), /** * An index added to the specification after this package was released. * Terms are stated and this package cannot name them, so it answers * with no address rather than building one from the index. */ - UNKNOWN(null); + UNKNOWN(-1, null); + + /** + * The Terms index, and -1 for {@link #UNKNOWN}, which stands for every + * index this package does not name and so has no index of its own. A + * Terms index read from a payload is one byte, so it is 0 to 255 and + * can never be negative, which is what makes -1 safe as the value that + * is not in the table. A negative index is left out of + * {@link #BY_INDEX} for that reason. + */ + private final int index; private final String url; - Terms(String url) { + Terms(int index, String url) { + this.index = index; this.url = url; } + /** + * The table above, read by index. It is built from the members rather + * than written out a second time, so a member and its index can never + * disagree, and a new terms document is one new member above and + * nothing here. + */ + private static final Map BY_INDEX; + + static { + final Map byIndex = new HashMap<>(); + for (Terms terms : values()) { + if (terms.index >= 0) { + byIndex.put(terms.index, terms); + } + } + BY_INDEX = Collections.unmodifiableMap(byIndex); + } + /** * Reads the terms from the Terms index carried after the match key. An * index this package does not know answers {@link #UNKNOWN} rather than @@ -106,14 +139,8 @@ enum Terms { * @return the terms document the index stands for */ public static Terms fromIndex(int index) { - switch (index) { - case 0: - return NOT_STATED; - case 1: - return MODEL_TERMS_FOR_MARKETING_2; - default: - return UNKNOWN; - } + final Terms terms = BY_INDEX.get(index); + return terms == null ? UNKNOWN : terms; } /** @@ -127,4 +154,13 @@ public static Terms fromIndex(int index) { public String getUrl() { return url; } + + /** + * The Terms index this member is carried as in a payload. + * + * @return the index, or -1 for {@link #UNKNOWN}, which has none + */ + public int getIndex() { + return index; + } } diff --git a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index a2cf58712..6be4959b2 100644 --- a/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java @@ -460,6 +460,43 @@ public void getTerms_ByteThenContextSection_ReadAtTheRightOffset() assertTrue(fodId.verify(factory.publicPem)); } + /** + * The terms table is the one place the package says which index is + * which document, and the index to member map is built from the + * members rather than written out again. This walks every member and + * fails if one does not read back from its own index, or if a member + * that names a document has no address, which is what would happen if + * a member and its address were ever added apart. + */ + @Test + public void terms_EveryMemberAgreesWithTheTable() { + for (Terms terms : Terms.values()) { + if (terms == Terms.UNKNOWN) { + // It stands for every index not in the table, so it has no + // index of its own and no address. + assertEquals(-1, terms.getIndex()); + assertNull(terms.getUrl()); + continue; + } + assertEquals( + "member " + terms + " does not read back from its index", + terms, + Terms.fromIndex(terms.getIndex())); + if (terms == Terms.NOT_STATED) { + // Names no document, so it has no address. + assertEquals(0, terms.getIndex()); + assertNull(terms.getUrl()); + } else { + assertNotNull( + "member " + terms + " names a document with no address", + terms.getUrl()); + assertTrue( + "address for " + terms + " is not an https address", + terms.getUrl().startsWith("https://")); + } + } + } + /** * The Reserved type has no defined match key length, so the reader * takes every byte after the header as the match key and leaves none