fix(odsp-driver): remove invalid byteOffset === 0 assumption

[Lindsey Nguyen (lindsnguyen)]

Description

Fixes compact snapshot parsing when the input is a valid Uint8Array view with a non-zero byteOffset.

The ODSP compact snapshot parser asserted that its input must begin at offset zero:

assert(input.byteOffset === 0, 0x3e8 /* code below assumes no offset */);

The affected snapshots were valid, but their binary data was represented by a view into a larger backing buffer. The assertion closed the Fluid container during loading and caused eight of the 29 Office eDiscovery fixtures to fail with assertion 0x3e8.

Update: Why this started failing now

The assertion has existed since 2022, but the relevant inputs previously happened to have a zero byteOffset, leaving the invalid assumption hidden.

The failure started after Node.js 24.18.0 increased the default Buffer.poolSize from 8 KiB to 64 KiB. Node uses this pool for Buffer.allocUnsafe() allocations smaller than half the pool size. This increased the pooling threshold from approximately 4 KiB to 32 KiB.

The Office eDiscovery export path reads binary snapshots using:

fs.readFileSync(filePath)

Node's fs.readFileSync() allocates the returned Buffer using Buffer.allocUnsafe(fileSize). A Node Buffer is also a Uint8Array view.

Before Node 24.18:

• The default buffer pool was 8 KiB.
• Allocations of approximately 4 KiB or larger bypassed the pool.
• Relevant snapshot reads therefore received dedicated backing buffers, normally with byteOffset === 0.

Beginning with Node 24.18:

• The default buffer pool is 64 KiB.
• Allocations smaller than approximately 32 KiB can come from the shared pool.
• Those buffers can be slices of the shared backing allocation and therefore have a valid non-zero byteOffset.

Node 24.20 and 24.21 inherited this behavior. This explains why the failure appeared when the Office External Partner pipeline moved from Node 24.11 to newer Node 24 releases.

The Office External Partner integration pipeline selected Node using this floating range:

- task: UseNode@1
  inputs:
    version: ">=24.11.0 <25.0.0"

The range selects the newest available matching Node 24 release, not the lowest version in the range. Normal Office CI did not initially reproduce the issue because its managed build image still provided Node 24.11.0.

Controlled runs confirmed the Node version as the differentiating variable:

• Node 24.21.0 with Fluid build 422789: eight eDiscovery failures.
• Node 24.11.0 with the same Fluid build and Office test code: all 29 eDiscovery tests passed.

Node did not introduce malformed snapshot data or change the Uint8Array contract. The larger buffer pool caused more file reads to use valid offset views, exposing Fluid's existing assumption about the backing-buffer layout.

Pinning Node 24.11 would avoid the expanded pooling threshold and can serve as a temporary mitigation. However, it would leave Fluid dependent on an allocation detail that Node does not guarantee. Other consumers can also provide valid non-zero-offset Uint8Array views independently of Node's file-reading behavior.

Root cause

The assertion was originally added by Vlad Sudzilouski in #12058 as part of a string-parsing performance optimization. The PR discusses null-character handling and differences between browser and Node string decoding, but it does not document why a zero byte offset was required.

The parser does not require a zero-offset backing buffer:

• Uint8Array indexing is relative to the view.
• Uint8Array.subarray() is relative to the view.
• ReadBuffer positions and reads are relative to the supplied Uint8Array.
• Existing BlobShallowCopy handling explicitly accounts for data.byteOffset.

For example, if a view begins at offset 100 in its backing buffer, input[0] still reads the first byte of that view, not byte zero of the backing buffer. The string positions recorded by the parser are also relative to that same view.

A non-zero offset is therefore a supported memory layout rather than evidence of a malformed or corrupted snapshot. The assertion rejected valid input without providing additional snapshot-integrity protection.

This change removes that assertion. It does not remove validation of snapshot structure, string lengths, tree markers, skipped ranges, or unexpected end-of-file conditions.

Copying the input into a new zero-offset buffer would also avoid the assertion, but it would introduce an unnecessary full-snapshot allocation and preserve the incorrect parser restriction.

Changes

• Remove the byteOffset === 0 assertion from compact snapshot string loading.
• Add a regression test that:
  • Serializes a compact snapshot.
  • Embeds it inside a larger backing buffer.
  • Passes a non-zero-offset subarray() to TreeBuilder.load().
  • Confirms that the test input actually has a non-zero byteOffset.
  • Verifies that the resulting strings, blobs, and tree match the original snapshot.

Reproduction and integration validation

The issue was isolated using controlled Office External Partner pipeline runs:

Office build	Node	Fluid build	eDiscovery result
55007732	24.21.0	422789	21 passed, 8 failed with 0x3e8
55015480	24.11.0	422789	29/29 passed
55069760	24.21.0	422948 with this fix	29/29 passed; pipeline succeeded

The first two runs used the same Fluid build and Office test code, with only the Node version changed. This confirmed that the larger buffer pool in newer Node versions exposes the existing parser assumption.

The final run restored the triggering Node 24.21.0 environment and used Fluid build 422948 containing this change. All 29 eDiscovery tests passed, including the eight fixtures that previously failed. The overall integration pipeline also succeeded.

The ODSP driver build completed successfully, including compilation, linting, API checks, and test compilation. The Tree Representation test suite passed in both ESM and CommonJS configurations.

A Fluid Real Service End to End Tests run was also queued using client build 422948 to exercise the change against an actual ODSP tenant:

• Fluid Real Service E2E build 423091

Reviewer Guidance

Please focus on whether any compact snapshot parsing operation depends on the backing buffer starting at offset zero.

The relevant string positions are relative to the supplied Uint8Array view, and the added regression test exercises the parser using a non-zero-offset view and compares the fully parsed result with the original tree.

The original assertion author was Vlad Sudzilouski (vladsud) in #12058. The original PR does not contain an explanation of a required zero-offset invariant.

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (18 lines, 2 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[Copilot]

🟢 Approval recommended

The change removes a demonstrably unnecessary assertion and is covered by a focused regression test that exercises the previously failing non-zero-offset Uint8Array scenario.

Pull request overview

Removes an invalid compact snapshot parsing restriction in the ODSP driver that incorrectly asserted the input Uint8Array must have byteOffset === 0, and adds a regression test to ensure parsing works correctly when given a valid non-zero-offset view into a larger backing buffer.

Changes:

• Removed the byteOffset === 0 assertion from the compact snapshot string-loading path.
• Added a regression test that loads a serialized snapshot from a Uint8Array.subarray() with a non-zero byteOffset and validates the parsed tree matches.

File summaries

File	Description
packages/drivers/odsp-driver/src/zipItDataRepresentationUtils.ts	Removes the incorrect byteOffset === 0 assumption from loadStrings() so non-zero-offset Uint8Array views are accepted.
packages/drivers/odsp-driver/src/test/zipItDataRepresentationTests.spec.ts	Adds a targeted test that constructs a non-zero-offset Uint8Array view and verifies TreeBuilder.load() produces an equivalent tree.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[shlevari]

Are you confident that the offset change with the Node upgrade is not indicative of a bigger issue? I'm surprised a Node change would effect this, but also not my expertise area.

[github-actions]

Bundle size comparison

Base commit: c1f54dbc51170408ea4e4707aff1a5df4a0e48a1
Head commit: 283cec40d4b0a90075822443aea3b69bc79b5b79

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

• fluidFrameworkAllAlpha.js: parsed 804364 → 804420 (+56), gzip 220823 → 220881 (+58)
• azureClient.js: parsed 634204 → 634199 (-5), gzip 169924 → 170011 (+87)
• odspClient.js: parsed 605450 → 605530 (+80), gzip 162719 → 162858 (+139)
• aqueduct.js: parsed 538081 → 538092 (+11), gzip 144406 → 144452 (+46)
• fluidFramework.js: parsed 413678 → 413711 (+33), gzip 117282 → 117286 (+4)
• sharedTree.js: parsed 403057 → 403083 (+26), gzip 114718 → 114724 (+6)
• containerRuntime.js: parsed 314896 → 314878 (-18), gzip 86384 → 86384 (0)
• sharedString.js: parsed 175191 → 175198 (+7), gzip 49636 → 49644 (+8)
• experimentalSharedTree.js: parsed 161846 → 161846 (0), gzip 46722 → 46722 (0)
• matrix.js: parsed 153720 → 153727 (+7), gzip 44381 → 44388 (+7)
• loader.js: parsed 147327 → 147343 (+16), gzip 40038 → 40048 (+10)
• odspDriver.js: parsed 105689 → 105716 (+27), gzip 32932 → 32990 (+58)
• directory.js: parsed 65669 → 65676 (+7), gzip 18493 → 18501 (+8)
• 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
• odspPrefetchSnapshot.js: parsed 45921 → 45878 (-43), gzip 15346 → 15347 (+1)
• map.js: parsed 45820 → 45827 (+7), gzip 14119 → 14126 (+7)
• 252.js: parsed 44384 → 44384 (0), gzip 13741 → 13741 (0)
• summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
• socketModule.js: parsed 26992 → 26962 (-30), gzip 8017 → 8052 (+35)
• createNewModule.js: parsed 12464 → 12464 (0), gzip 4792 → 4805 (+13)
• summaryModule.js: parsed 3888 → 3888 (0), gzip 1874 → 1874 (0)
• connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
• sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 496 → 505 (+9)
• debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
• FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

[Jason Hartman (jason-ha)]

I see no reason that assert was ever added. At least from my understanding of how the code works a non-zero offset was never problematic. Perhaps there was just a misunderstanding of the logic when originally written.

[Lindsey Nguyen (lindsnguyen)]

Are you confident that the offset change with the Node upgrade is not indicative of a bigger issue? I'm surprised a Node change would effect this, but also not my expertise area.
shlevari

We discussed this offline a bit but the explanation to why this upgrade (Node 24.18 increasing the Buffer pool from 8 KiB to 64 KiB) caused a problem as I understand it is this:
Node generally places Buffer allocations smaller than half the pool size into a shared backing buffer. With the old 8 KiB pool, the affected snapshots were larger than the roughly 4 KiB threshold (half the pool size), so they received dedicated buffers starting at offset 0. Increasing the pool to 64 KiB raised the threshold to roughly 32 KiB, so the same snapshots are now more often placed after another allocation in a shared buffer and therefore have non-zero offsets. The snapshot data itself is unchanged. This exposed an assumption that was likely unnecessary: the ODSP parser accepts a view and reads relative to it, so index 0 remains the first snapshot byte regardless of where the view begins in its backing buffer.