fix(identity)!: spec-conformant identifier types and a monotonic TID generator #114

New issue

Closed

opened 2026-09-24 12:28:56 +00:00 by Grandiras · 0 comments

Grandiras commented

2026-09-24 12:28:56 +00:00

Owner

Part of the v0.7.0 update cycle. The identifier types are meant to guarantee validity, but several accept invalid input. One generator produces out-of-order and duplicate TIDs.

Bugs

Tid.Next() is neither monotonic nor collision-free. 20,000 calls produced 9,992 values out of order and 22 exact duplicates, because each call adds Random.Shared.Next(0,1024) to the millisecond timestamp. The spec requires monotonic TIDs per generator. Add TidGenerator (fixed clock id per generator, Interlocked increment over the last µs timestamp, TimeProvider-injectable). Tid.Next() delegates to a shared instance, and RecordKey.NewTid() inherits the fix.
Validation against the official atproto-interop-tests/syntax fixtures. Vendor the fixtures into tests/ and run every valid/invalid line. Today the SDK wrongly accepts:
- 48/72 invalid AT URIs. The authority, collection and rkey are not validated, and trailing slashes and # fragments are accepted.
- 4/11 invalid record keys. The regex allows @!$&'()*+,;=; the spec allows only A-Za-z0-9 . - _ : ~, excluding . and ...
- 2/9 invalid TIDs. The first character must be in 234567abcdefghij.
- 2/18 invalid DIDs. A trailing % is accepted.
- 1/27 invalid NSIDs. There is no 63-character limit on the name segment.
Cid.Parse validates nothing (Cid.Parse("hello") succeeds). Validate a base32-lower CIDv1 with codec 0x71 or 0x55, sha2-256 and a 32-byte digest. Expose Codec, Digest and ToBytes().
AtUri: validate the authority as an AtIdentifier at parse time. Add typed accessors Nsid? Collection / RecordKey? RecordKey / AtIdentifier Repo, computed once. Create(AtIdentifier, Nsid?, RecordKey?).
JSON converters: an invalid value currently escapes as ArgumentException with no JSON path. Throw JsonException instead.

API shape

Implement IParsable<T>, ISpanParsable<T>, IComparable<T> and explicit string → T conversions on all identifier types (Did, Handle, Nsid, Cid, Tid, RecordKey, AtUri, AtIdentifier, plus SpaceUri / SpaceRecordUri). This makes minimal-API binding and generic parsing work.
Remove duplication: turn the value types into sealed records (or a shared base) instead of hand-written equality/operators/CompareTo; implement Parse as TryParse ? x : throw; use one generic converter over a static abstract parse interface instead of 8 identical JSON converters; delete the 7 unused internal UnsafeCreate methods.
Keep: ordinal equality, handle lower-casing and @ stripping, and conversions that throw staying explicit.

Acceptance

Every interop syntax fixture passes (valid lines accepted, invalid lines rejected).
A TidGenerator test over 100k calls, including concurrent callers, is strictly increasing.
CHANGELOG: Fixed for the bugs; Breaking changes for the stricter parsing (a migration note that legacy or foreign data should use TryParse) and for the record/interface changes.

Part of the v0.7.0 update cycle. The identifier types are meant to guarantee validity, but several accept invalid input. One generator produces out-of-order and duplicate TIDs. ## Bugs - [ ] **`Tid.Next()` is neither monotonic nor collision-free.** 20,000 calls produced 9,992 values out of order and 22 exact duplicates, because each call adds `Random.Shared.Next(0,1024)` to the millisecond timestamp. The spec requires monotonic TIDs per generator. Add `TidGenerator` (fixed clock id per generator, `Interlocked` increment over the last µs timestamp, `TimeProvider`-injectable). `Tid.Next()` delegates to a shared instance, and `RecordKey.NewTid()` inherits the fix. - [ ] **Validation against the official `atproto-interop-tests/syntax` fixtures.** Vendor the fixtures into `tests/` and run every valid/invalid line. Today the SDK wrongly accepts: - **48/72 invalid AT URIs.** The authority, collection and rkey are not validated, and trailing slashes and `#` fragments are accepted. - **4/11 invalid record keys.** The regex allows `@!$&'()*+,;=`; the spec allows only `A-Za-z0-9 . - _ : ~`, excluding `.` and `..`. - **2/9 invalid TIDs.** The first character must be in `234567abcdefghij`. - **2/18 invalid DIDs.** A trailing `%` is accepted. - **1/27 invalid NSIDs.** There is no 63-character limit on the name segment. - [ ] **`Cid.Parse` validates nothing** (`Cid.Parse("hello")` succeeds). Validate a base32-lower CIDv1 with codec `0x71` or `0x55`, sha2-256 and a 32-byte digest. Expose `Codec`, `Digest` and `ToBytes()`. - [ ] **`AtUri`:** validate the authority as an `AtIdentifier` at parse time. Add typed accessors `Nsid? Collection` / `RecordKey? RecordKey` / `AtIdentifier Repo`, computed once. `Create(AtIdentifier, Nsid?, RecordKey?)`. - [ ] **JSON converters:** an invalid value currently escapes as `ArgumentException` with no JSON path. Throw `JsonException` instead. ## API shape - [ ] Implement `IParsable<T>`, `ISpanParsable<T>`, `IComparable<T>` and explicit `string → T` conversions on all identifier types (`Did`, `Handle`, `Nsid`, `Cid`, `Tid`, `RecordKey`, `AtUri`, `AtIdentifier`, plus `SpaceUri` / `SpaceRecordUri`). This makes minimal-API binding and generic parsing work. - [ ] Remove duplication: turn the value types into `sealed record`s (or a shared base) instead of hand-written equality/operators/`CompareTo`; implement `Parse` as `TryParse ? x : throw`; use one generic converter over a `static abstract` parse interface instead of 8 identical JSON converters; delete the 7 unused internal `UnsafeCreate` methods. - [ ] Keep: ordinal equality, handle lower-casing and `@` stripping, and conversions that throw staying explicit. ## Acceptance - Every interop syntax fixture passes (valid lines accepted, invalid lines rejected). - A `TidGenerator` test over 100k calls, including concurrent callers, is strictly increasing. - CHANGELOG: `Fixed` for the bugs; `Breaking changes` for the stricter parsing (a migration note that legacy or foreign data should use `TryParse`) and for the record/interface changes.

Grandiras referenced this issue

2026-09-24 12:39:03 +00:00

feat!: typed identifiers and uniform conventions across the public API #119

Grandiras referenced this issue

2026-09-24 12:39:33 +00:00

Tracking: v0.7.0 update cycle — protocol catch-up, optimization, debloat, API reshaping #141

Grandiras referenced this issue

2026-09-24 18:00:11 +00:00

revert: drop the union merge driver for CHANGELOG.md #145

Grandiras referenced this issue from a commit

2026-09-24 18:00:55 +00:00

fix(identity)!: spec-conformant identifier types and a monotonic TID generator (closes #114)