Repository navigation
Export versionOf so consumers can compute a document's version token - #20
Conversation
obsidian-local-rest-api is moving its ETags onto the document map's version token, so the token becomes something clients hold across requests and compare later. These tests pin the derivation to values computed outside the library (sha256sum), check it agrees with buildModel(...).version for CRLF, frontmatter, and empty documents, and cover a Uint8Array form that hashes bytes verbatim. They exercise a versionOf export that the next commit adds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
obsidian-local-rest-api is adding ETag / If-Match to every vault write endpoint, and the ETag has to be the same token as the document map's version. The only way to get that token was buildModel(content).version, which parses the whole document to produce a hash of its bytes. versionOf is now public, with its derivation documented as contract: sha256 over the UTF-8 bytes, lowercase hex, first 6 characters. The string path is unchanged, so every token clients already hold still validates. It also takes a Uint8Array, hashed verbatim, so a consumer holding a binary file or undecoded bytes gets the same token a text consumer gets for the same bytes. The playground's browser stand-in for crypto accepted only strings; given bytes it would have hashed their comma-joined decimal rendering. It now hashes a Uint8Array as Node does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
coddingtonbot
left a comment
There was a problem hiding this comment.
Reviewed the diff, ran tsc, the new test suites, and typedoc (all clean), and verified the three pinned tokens against sha256sum. Nothing blocking. Three inline notes below: one small correctness nit in the shim, one design question about freezing a 24-bit token as a public ETag contract, and one optional simplification.
| update(data: string | Uint8Array, encoding: ShimEncoding = "utf8"): Hash { | ||
| if (typeof data !== "string") { | ||
| // Copied, so a caller reusing its buffer cannot change what we hash. | ||
| chunks.push(data.slice()); |
There was a problem hiding this comment.
The "Copied" claim doesn't hold when data is a Node Buffer: Buffer.prototype.slice is an alias for subarray and returns a view of the same memory (it's also deprecated for exactly this reason), so a caller that reuses its buffer between update and digest would change what gets hashed. Only plain Uint8Array.prototype.slice copies.
In the browser bundle there is no Buffer, so this is latent there, but the shim's own tests run under Node and the Hash type advertises accepting a Uint8Array, which a Buffer is. new Uint8Array(data) copies for both kinds and matches the comment:
chunks.push(new Uint8Array(data));A one-line test pairing with the existing accepts a Buffer case (mutate the buffer after update, assert the digest is unchanged) would pin it.
| * bytes yield the same token. | ||
| * | ||
| * The derivation is part of the public contract: tokens held by clients must | ||
| * keep validating, so it will not change without a major version bump. |
There was a problem hiding this comment.
Design question rather than a defect: this PR promotes the 6-hex-character truncation from an internal detail to a public contract that can only change in a major release, and the stated consumer is HTTP If-Match on write endpoints. A 24-bit token means a concurrent edit that changed the document has roughly a 1-in-16.7M chance of slipping past the precondition, which is tiny but is a weaker guarantee than people normally assume an ETag carries, and once frozen it costs a major version to widen.
If the length is worth keeping short for the map's readability, it might be worth saying so explicitly in the doc comment and README ("24 bits; adequate for optimistic concurrency, not a content identifier") so consumers don't treat it as collision-free. Alternatively, freezing the derivation but not the width (e.g. documenting that a longer prefix of the same digest may be accepted in future, and having comparison use startsWith) would leave room to grow without breaking stored tokens.
| export const versionOf = (document: string | Uint8Array): string => | ||
| (typeof document === "string" | ||
| ? createHash("sha256").update(document, "utf8") | ||
| : createHash("sha256").update(document) |
There was a problem hiding this comment.
Optional: the ternary isn't needed. Node's Hash#update defaults a string to "utf8", and the shim's update has the same default, so both branches collapse to:
export const versionOf = (document: string | Uint8Array): string =>
createHash("sha256").update(document).digest("hex").slice(0, 6);If the explicit "utf8" is there to make the encoding visible at the call site, that's a fair reason to keep it; just noting the two calls do the same thing.
The shim's comment said bytes were copied so a caller reusing its buffer could not change what gets hashed, but it copied with data.slice(), and on a Node Buffer slice() is subarray(): a view of the same memory. new Uint8Array(data) copies for both a Uint8Array and a Buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Node's Hash#update already treats a string as UTF-8 and hashes a Uint8Array as-is, and the playground's shim has the same defaults, so branching on the input type to pass "utf8" explicitly bought nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
obsidian-local-rest-api is adding HTTP conditional writes (
ETag/If-Match/If-None-Match) to all of its vault write endpoints, not just PATCH, and those ETags need to be the same token as the document map'sversionso the two are interchangeable. Until now, the only way to get that token wasbuildModel(content).version, which parses the whole document just to hash it.This exports
versionOffrom the package root:PublicMap.versionand theifMatchtoken, and it won't change outside a major release. The string path is byte-for-byte the old implementation, so tokens clients are already holding keep validating.Uint8Array(orBuffer) and hashes those bytes as-is, so a consumer holding a binary file or undecoded bytes gets the same token a text consumer gets for the same bytes. I went with a singlestring | Uint8Arrayparameter rather than overloads since the return type doesn't vary.site/crypto-shim.tsonly accepted strings. Handed bytes, it would have hashed their"1,2,3"stringification and returned a plausible-looking but wrong token, so it now hashes aUint8Arraythe same way Node does.The new tests in
src/tests/version.test.tspin the derivation to values computed withsha256sumrather than by the library itself, check that it agrees withbuildModel(...).versionfor empty, CRLF, frontmatter, and non-ASCII documents, and check that a string and its UTF-8 bytes produce the same token.This adds public API, so it should go out as 2.1.0. I've left
package.jsonalone since releases here bump it in their own commit.🤖 Generated with Claude Code