Skip to content

fix(rivetkit): copy Buffer to Uint8Array in ensureUint8Array to fix .slice() semantics#4506

Merged
NathanFlurry merged 1 commit intomainfrom
03-24-fix_rivetkit_copy_buffer_to_uint8array_in_ensureuint8array_to_fix_.slice_semantics
Mar 25, 2026
Merged

fix(rivetkit): copy Buffer to Uint8Array in ensureUint8Array to fix .slice() semantics#4506
NathanFlurry merged 1 commit intomainfrom
03-24-fix_rivetkit_copy_buffer_to_uint8array_in_ensureuint8array_to_fix_.slice_semantics

Conversation

@NathanFlurry
Copy link
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link

railway-app bot commented Mar 25, 2026

🚅 Deployed to the rivet-pr-4506 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-cloud 🕐 Queued (View Logs) Web Mar 25, 2026 at 8:04 pm
kitchen-sink 🕐 Queued (View Logs) Web Mar 25, 2026 at 8:04 pm
frontend-inspector 🕐 Queued (View Logs) Web Mar 25, 2026 at 8:04 pm
website 😴 Sleeping (View Logs) Web Mar 25, 2026 at 9:44 am
mcp-hub ✅ Success (View Logs) Web Mar 25, 2026 at 9:32 am
ladle ❌ Build Failed (View Logs) Web Mar 25, 2026 at 9:32 am

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 25, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4506

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4506

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4506

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4506

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4506

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4506

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4506

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4506

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4506

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4506

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4506

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4506

commit: bb97419

@claude
Copy link

claude bot commented Mar 25, 2026

PR Review: fix(rivetkit): copy Buffer to Uint8Array in ensureUint8Array

This is a correct and well-targeted bug fix. The root cause is real: Node.js Buffer extends Uint8Array but overrides .slice() to return a view instead of a copy, violating the Uint8Array spec. Libraries like vbare and cbor-x that depend on copy semantics can silently get aliased memory, causing subtle data corruption.

What looks good

  • Comment quality: The comment at lines 219-223 clearly explains the why (spec violation, affected libraries) rather than just the what.
  • Guard correctness: typeof Buffer !== "undefined" && Buffer.isBuffer(value) handles environments without Buffer (e.g. browsers, Deno) safely.
  • Ordering: The Buffer.isBuffer check sits inside the instanceof Uint8Array branch, so it only runs when needed and correctly precedes the plain return value.
  • Symmetry with normalizeParams: Input params go Uint8Array -> Buffer (line 45), output values now go Buffer -> Uint8Array. The round-trip is semantically correct.

Minor suggestions

Simpler copy idiom: The two-step copy is correct, but return new Uint8Array(value) achieves the same result in one line. The TypedArray constructor performs a copy when given another typed array. Either form is fine.

No test added: The driver test suite at src/driver-test-suite/tests/ has no coverage for the Buffer->Uint8Array copy path. A test that stores binary data, reads it back, and calls .slice() on the result to verify it returns a copy (not a view) would prevent a regression.

Other ArrayBufferView subtypes: The existing fallback at line 234-235 creates a view for non-Uint8Array ArrayBufferView inputs. This is fine for now since only Buffer is known to override .slice() semantics in practice.

Summary

The fix is correct and minimal. The suggestions above are optional — the change is mergeable as-is.

Copy link
Member Author

NathanFlurry commented Mar 25, 2026

Merge activity

  • Mar 25, 7:43 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 25, 8:04 PM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 25, 8:05 PM UTC: @NathanFlurry merged this pull request with Graphite.

@NathanFlurry NathanFlurry changed the base branch from 03-24-fix_rivetkit_setup_database_before_lifecycle_hooks_for_c.db_availability to graphite-base/4506 March 25, 2026 20:00
@NathanFlurry NathanFlurry changed the base branch from graphite-base/4506 to main March 25, 2026 20:02
@NathanFlurry NathanFlurry force-pushed the 03-24-fix_rivetkit_copy_buffer_to_uint8array_in_ensureuint8array_to_fix_.slice_semantics branch from bb97419 to db91593 Compare March 25, 2026 20:03
@NathanFlurry NathanFlurry merged commit 89703c3 into main Mar 25, 2026
13 of 20 checks passed
@NathanFlurry NathanFlurry deleted the 03-24-fix_rivetkit_copy_buffer_to_uint8array_in_ensureuint8array_to_fix_.slice_semantics branch March 25, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant