-
Notifications
You must be signed in to change notification settings - Fork 0
Harden legacy fragment decoding (zlib-wrapped deflate + arx dictionary drift) #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5d65f28
866fe5c
7e589e5
9697483
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { compressToEncodedURIComponent, decompressFromEncodedURIComponent } from "lz-string"; | ||
| import { deflateSync, inflateSync, strFromU8, strToU8 } from "fflate"; | ||
| import { deflateSync, inflateSync, strFromU8, strToU8, unzlibSync } from "fflate"; | ||
| import { normalizeEnvelope } from "@/lib/payload/envelope"; | ||
| import { packEnvelope, unpackEnvelope } from "@/lib/payload/wire-format"; | ||
| import { | ||
|
|
@@ -136,14 +136,34 @@ function encodePayload(json: string, codec: PayloadCodec): string { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Inflates a `deflate` payload, accepting both raw deflate (current encoder) and zlib-wrapped | ||
| * deflate (emitted by an older encoder version). Raw is tried first so current output is never at | ||
| * risk of header mis-detection; zlib is a back-compat fallback so historical shared links keep | ||
| * decoding instead of failing as invalid-json. | ||
| */ | ||
| function inflateDeflatePayload(bytes: Uint8Array): Uint8Array { | ||
| try { | ||
| return inflateSync(bytes); | ||
| } catch (rawDeflateError) { | ||
| // Fall back to zlib-wrapped (legacy) deflate. If that also fails the input is not valid deflate | ||
| // at all, so surface the original raw-inflate error rather than a misleading zlib one. | ||
| try { | ||
| return unzlibSync(bytes); | ||
| } catch { | ||
| throw rawDeflateError; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function decodePayload(encoded: string, codec: PayloadCodec): string | null { | ||
| switch (codec) { | ||
| case "plain": | ||
| return fromBase64Url(encoded); | ||
| case "lz": | ||
| return decompressFromEncodedURIComponent(encoded); | ||
| case "deflate": | ||
| return strFromU8(inflateSync(fromBase64UrlBytes(encoded))); | ||
| return strFromU8(inflateDeflatePayload(fromBase64UrlBytes(encoded))); | ||
| case "arx": | ||
| case "arx2": | ||
| case "arx3": | ||
|
|
@@ -531,8 +551,18 @@ export async function decodeFragmentAsync(hash: string, options?: DecodeOptions) | |
| if (error instanceof Error && error.name === "ArxDecodedPayloadTooLargeError") { | ||
| return { ok: false, code: "decoded-too-large", message: error.message }; | ||
| } | ||
| return { ok: false, code: "invalid-json", message: "The fragment payload could not be decoded as valid JSON." }; | ||
| const arxHint = | ||
| codec === "arx" || codec === "arx2" || codec === "arx3" | ||
| ? " It may have been encoded with a different ARX dictionary version." | ||
| : ""; | ||
| return { ok: false, code: "invalid-json", message: `The fragment payload could not be decoded as valid JSON.${arxHint}` }; | ||
| } | ||
|
|
||
| return resolveEnvelope(parsed, header.fragmentLength); | ||
| const resolved = resolveEnvelope(parsed, header.fragmentLength); | ||
| if (!resolved.ok && resolved.code === "invalid-envelope" && (codec === "arx" || codec === "arx2" || codec === "arx3")) { | ||
| // An ARX payload that decoded but is not a valid envelope almost always means the active | ||
| // dictionary differs from the one it was encoded with (see tests/arx-dictionary-pin.test.ts). | ||
| return { ...resolved, message: `${resolved.message} It may have been encoded with a different ARX dictionary version.` }; | ||
|
Comment on lines
+561
to
+565
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a versioned legacy ARX fragment carries a dictionary version that differs from the active dictionary, this new logic only adds the hint after decode/validation has already failed. Useful? React with 👍 / 👎. |
||
| } | ||
| return resolved; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| // loadArxDictionary() resolves -1 on a transient fetch/parse failure (it does not reject). The | ||
| // ensure-helper must treat that as a failure and clear its cached promise, so a later decode retries | ||
| // the load instead of reusing a poisoned no-op promise for the page's lifetime. | ||
| const { loadArxDictionaryMock } = vi.hoisted(() => ({ loadArxDictionaryMock: vi.fn() })); | ||
|
|
||
| vi.mock("@/lib/payload/arx-codec", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/lib/payload/arx-codec")>(); | ||
| return { | ||
| ...actual, | ||
| isExternalDictionaryLoaded: () => false, // force the fetch path so the cached promise is exercised | ||
| loadArxDictionary: loadArxDictionaryMock, | ||
| }; | ||
| }); | ||
|
|
||
| const { decodeFragmentAsync } = await import("@/lib/payload/fragment"); | ||
|
|
||
| describe("arx dictionary load retry", () => { | ||
| it("retries after a transient (-1) load failure instead of caching it for the page lifetime", async () => { | ||
| loadArxDictionaryMock.mockResolvedValueOnce(-1); // transient failure | ||
| const first = await decodeFragmentAsync("#bAAAA", { skipFragmentBudget: true }); | ||
| expect(first.ok).toBe(false); | ||
| expect(loadArxDictionaryMock).toHaveBeenCalledTimes(1); | ||
|
|
||
| // Endpoint recovers: the next decode must attempt the load again, not reuse a poisoned promise. | ||
| loadArxDictionaryMock.mockResolvedValueOnce(1); | ||
| await decodeFragmentAsync("#bAAAA", { skipFragmentBudget: true }); | ||
| expect(loadArxDictionaryMock).toHaveBeenCalledTimes(2); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { beforeEach, describe, expect, it } from "vitest"; | ||
| import arx2DictionaryJson from "../public/arx2-dictionary.json"; | ||
| import arxDictionaryJson from "../public/arx-dictionary.json"; | ||
| import { loadArx2OverlayDictionarySync, loadArxDictionarySync } from "@/lib/payload/arx-codec"; | ||
| import { decodeFragmentAsync, encodeEnvelopeAsync } from "@/lib/payload/fragment"; | ||
| import type { PayloadEnvelope } from "@/lib/payload/schema"; | ||
|
|
||
| // Compact arx links carry no dictionary version (the tag implies the current dictionary), so a build | ||
| // must refuse to decode with a skewed-version dictionary rather than silently mis-decode. These | ||
| // tests exercise that guard by making a different-version dictionary active and confirming decode | ||
| // hard-fails, then recovers when the expected version is restored. | ||
| const envelope: PayloadEnvelope = { | ||
| v: 1, | ||
| codec: "arx2", | ||
| activeArtifactId: "doc", | ||
| artifacts: [{ id: "doc", kind: "markdown", content: "# Title\n\nSome content for the arx dense path." }], | ||
| }; | ||
|
|
||
| describe("arx dictionary version guard", () => { | ||
| beforeEach(() => { | ||
| loadArx2OverlayDictionarySync(arx2DictionaryJson); | ||
| loadArxDictionarySync(arxDictionaryJson); // expected version (1) | ||
| }); | ||
|
|
||
| it("refuses to decode a compact arx link when the base dictionary version is skewed", async () => { | ||
| const fragment = await encodeEnvelopeAsync(envelope, { codec: "arx2", preferPacked: true }); | ||
|
|
||
| const okParsed = await decodeFragmentAsync(`#${fragment}`, { skipFragmentBudget: true }); | ||
| expect(okParsed.ok).toBe(true); | ||
|
|
||
| // Simulate asset/CDN skew or a future bump: a different-version dictionary becomes active. | ||
| loadArxDictionarySync({ ...arxDictionaryJson, version: 2 }); | ||
| const skewed = await decodeFragmentAsync(`#${fragment}`, { skipFragmentBudget: true }); | ||
| expect(skewed.ok).toBe(false); // hard-fail, not a silent mis-decode | ||
|
|
||
| // Recovers once the expected-version dictionary is active again. | ||
| loadArxDictionarySync(arxDictionaryJson); | ||
| const recovered = await decodeFragmentAsync(`#${fragment}`, { skipFragmentBudget: true }); | ||
| expect(recovered.ok).toBe(true); | ||
| }); | ||
|
|
||
| it("refuses to decode when the arx2 overlay dictionary version is skewed", async () => { | ||
| const fragment = await encodeEnvelopeAsync(envelope, { codec: "arx2", preferPacked: true }); | ||
|
|
||
| loadArx2OverlayDictionarySync({ ...arx2DictionaryJson, version: 2 }); | ||
| const skewed = await decodeFragmentAsync(`#${fragment}`, { skipFragmentBudget: true }); | ||
| expect(skewed.ok).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { createHash } from "node:crypto"; | ||
| import { readFileSync } from "node:fs"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| // Defended contract — learned the hard way: real shared arx/arx2 links broke because a dictionary's | ||
| // content was changed while its `version` field stayed 1, so every previously-encoded fragment | ||
| // silently mis-decoded into garbage. The dictionary content and its version MUST move together. | ||
| // | ||
| // If you change a dictionary's slots, you MUST bump its `version` AND update the pinned hash here in | ||
| // the same commit. The version bump is the only thing that lets a future build distinguish (and | ||
| // reject, rather than garble) a fragment that was encoded with an older dictionary. Do NOT just | ||
| // re-pin the hash to make this test pass — that re-creates the silent-drift bug. | ||
| // | ||
| // arx3 reuses public/arx-dictionary.json (via arx3DecompressEnvelope), so pinning it also covers | ||
| // arx3; if arx3 is ever given its own dictionary file, add a pin entry for it here. | ||
| const PINNED: Record<string, { version: number; sha256: string }> = { | ||
| "public/arx-dictionary.json": { | ||
| version: 1, | ||
| sha256: "16fe3f72dd5d282fd2f0271647a56c38c4b7eebb6e4723da670765f7d90380a9", | ||
| }, | ||
| "public/arx2-dictionary.json": { | ||
| version: 1, | ||
| sha256: "12d0166fda16ce9697831805d92af0f558b4fad5a882c471ba072118561a74eb", | ||
| }, | ||
| }; | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| describe("arx dictionary content is pinned to its version", () => { | ||
| for (const [file, pin] of Object.entries(PINNED)) { | ||
| it(`${file} matches its pinned version and content hash`, () => { | ||
| const obj = JSON.parse(readFileSync(file, "utf8")); | ||
| // Hash the canonical (re-serialized) form so whitespace/formatting changes don't trip it, only | ||
| // real content changes (slots, version) do. | ||
| const canonicalSha256 = createHash("sha256").update(JSON.stringify(obj)).digest("hex"); | ||
|
|
||
| expect(obj.version).toBe(pin.version); | ||
| expect(canonicalSha256).toBe(pin.sha256); | ||
| }); | ||
| } | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.