fix(napi): validate status before copying in remaining TypedArray fallback paths by Brooooooklyn · Pull Request #3076 · napi-rs/napi-rs

Brooooooklyn · 2025-12-30T03:18:21Z

Note

Strengthens error handling and memory safety across ArrayBuffer/TypedArray creation and fallback paths.

In fallback when external arraybuffers aren’t allowed, now check status before copying, only copy when length > 0, and always call finalize; propagate errors consistently
In &mut TypedArray::to_napi_value, reclaim leaked hint safely and immediately clear val fields before early returns to avoid dangling pointers; copy into newly allocated ArrayBuffer when needed
Add missing check_status! validations for buffer creation across typed array slices and Uint8ClampedSlice, including zero-length guards

^{Written by Cursor Bugbot for commit f2e1e07. This will update automatically on new commits. Configure here.}

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved buffer creation logic with enhanced error handling and validation.
- Added safeguards for zero-length buffer operations to prevent edge-case failures.
- Strengthened fallback mechanisms and error propagation in buffer creation paths.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-30T03:18:31Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Refactors buffer and arraybuffer creation logic across three files to add explicit error handling for edge cases, including guards against null pointer dereferences for zero-length buffers, status checks for external buffer fallback scenarios (napi_no_external_buffers_allowed), and conditional compilation adjustments excluding wasm targets.

Changes

Cohort / File(s)	Summary
ArrayBuffer Error Handling `crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs`	Adds guards for zero-length buffer operations to prevent null dereferences, introduces explicit status checks for external buffer creation paths, handles napi_no_external_buffers_allowed fallback by switching to regular arraybuffer creation with data copy, and extends conditional compilation to exclude wasm targets in debug assertion blocks.
Buffer Compilation Guards `crates/napi/src/bindgen_runtime/js_values/buffer.rs`	Narrows compilation scope of thread_local BUFFER_DATA static by adding non-wasm target gate alongside existing debug_assertions and windows conditions.
Environment Buffer Creation `crates/napi/src/env.rs`	Separates zero-length and positive-length buffer creation paths with explicit status checks, implements fallback behavior for napi_no_external_buffers_allowed by dropping hints and creating backing arraybuffers, ensures data copying precedes finalization, and adds consistent error messages for all buffer creation failure modes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Buffers now guarded from null-pointer's bite,
Zero-length edges handled just right,
Finalize before copy, errors in flight—
External buffers fall back with grace,
Safe bunny hops through memory space! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: validating status before copying in TypedArray fallback paths, which aligns with the core improvements across all three modified files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

crates/napi/src/bindgen_runtime/js_values/buffer.rs

Copilot

Pull request overview

This PR refactors typed array and arraybuffer creation logic to improve error handling, fix memory safety issues, and add WebAssembly support. The changes focus on making the code more robust when external buffers are not allowed by the JavaScript runtime.

Key changes:

Refactored error handling to check status codes immediately after each operation rather than at the end of complex expressions
Added safety checks to ensure data is only copied when length > 0, preventing potential issues with zero-length buffers
Fixed finalize callback ordering to ensure data is copied before finalizers run (since finalizers may free the source data) and that finalizers always run even on error

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/napi/src/env.rs	Refactored arraybuffer creation with improved error handling and added length checks before copy operations
crates/napi/src/bindgen_runtime/js_values/buffer.rs	Extended cfg attribute to exclude wasm targets from BUFFER_DATA thread local
crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs	Major refactoring of ArrayBuffer and TypedArray creation logic with improved error handling, safety checks, and finalize callback ordering; also updated cfg attributes for wasm support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/napi/src/env.rs

crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs

Brooooooklyn · 2025-12-30T03:30:52Z

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

ready-to-merge - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

graphite-app · 2025-12-30T05:30:16Z

Merge activity

Dec 30, 5:30 AM UTC: Brooooooklyn added this pull request to the Graphite merge queue.
Dec 30, 5:59 AM UTC: Merged by the Graphite merge queue.

…lback paths (#3076)  > [!NOTE] > Strengthens error handling and memory safety across ArrayBuffer/TypedArray creation and fallback paths. > > - In fallback when external arraybuffers aren’t allowed, now check `status` before copying, only copy when `length > 0`, and always call `finalize`; propagate errors consistently > - In `&mut TypedArray::to_napi_value`, reclaim leaked hint safely and immediately clear `val` fields before early returns to avoid dangling pointers; copy into newly allocated ArrayBuffer when needed > - Add missing `check_status!` validations for buffer creation across typed array slices and `Uint8ClampedSlice`, including zero-length guards > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f2e1e07. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>   ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Improved buffer creation logic with enhanced error handling and validation. * Added safeguards for zero-length buffer operations to prevent edge-case failures. * Strengthened fallback mechanisms and error propagation in buffer creation paths. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Copilot AI review requested due to automatic review settings December 30, 2025 03:18

Copilot started reviewing on behalf of Brooooooklyn December 30, 2025 03:18 View session

chatgpt-codex-connector bot reviewed Dec 30, 2025

View reviewed changes

crates/napi/src/bindgen_runtime/js_values/buffer.rs Show resolved Hide resolved

Copilot AI reviewed Dec 30, 2025

View reviewed changes

crates/napi/src/env.rs Show resolved Hide resolved

crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs Show resolved Hide resolved

Brooooooklyn changed the base branch from main to graphite-base/3076 December 30, 2025 03:30

Brooooooklyn changed the base branch from graphite-base/3076 to bisect/typed-array-owned December 30, 2025 03:30

Brooooooklyn force-pushed the bisect/typed-array-owned branch from 9eb1cc0 to 4cc80d3 Compare December 30, 2025 03:36

Brooooooklyn force-pushed the bisect/typed-array-mut branch from 490be2e to d213203 Compare December 30, 2025 03:37

Brooooooklyn force-pushed the bisect/typed-array-owned branch from 4cc80d3 to 55a51d1 Compare December 30, 2025 03:40

Brooooooklyn force-pushed the bisect/typed-array-mut branch from d213203 to 7ebc052 Compare December 30, 2025 03:40

Brooooooklyn changed the title ~~Bisect/typed array mut~~ fix(napi): validate status before copying in remaining TypedArray fallback paths Dec 30, 2025

Brooooooklyn force-pushed the bisect/typed-array-owned branch from 55a51d1 to e48088b Compare December 30, 2025 04:17

Brooooooklyn force-pushed the bisect/typed-array-mut branch from 7ebc052 to 07b46c7 Compare December 30, 2025 04:17

Brooooooklyn force-pushed the bisect/typed-array-owned branch from e48088b to 3d92833 Compare December 30, 2025 04:58

Brooooooklyn force-pushed the bisect/typed-array-mut branch from 07b46c7 to f2e1e07 Compare December 30, 2025 04:59

graphite-app bot force-pushed the bisect/typed-array-owned branch from 3d92833 to 6ac1d6b Compare December 30, 2025 05:38

graphite-app bot force-pushed the bisect/typed-array-mut branch from f2e1e07 to 51e65eb Compare December 30, 2025 05:39

Base automatically changed from bisect/typed-array-owned to main December 30, 2025 05:58

graphite-app bot merged commit 51e65eb into main Dec 30, 2025
75 checks passed

graphite-app bot deleted the bisect/typed-array-mut branch December 30, 2025 05:59

github-actions bot mentioned this pull request Dec 30, 2025

chore: release #3073

Merged

coderabbitai bot mentioned this pull request Jan 7, 2026

fix(napi): Add TypeName implementation for ArrayBuffer #3087

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix(napi): validate status before copying in remaining TypedArray fallback paths#3076

fix(napi): validate status before copying in remaining TypedArray fallback paths#3076
graphite-app[bot] merged 1 commit intomainfrom
bisect/typed-array-mut

Brooooooklyn commented Dec 30, 2025 •

edited by cursor bot

Loading

Uh oh!

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

Review skipped

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Brooooooklyn commented Dec 30, 2025 •

edited

Loading

Uh oh!

graphite-app bot commented Dec 30, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Comments

Conversation

Brooooooklyn commented Dec 30, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Brooooooklyn commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

graphite-app bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Brooooooklyn commented Dec 30, 2025 •

edited by cursor bot

Loading

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

Brooooooklyn commented Dec 30, 2025 •

edited

Loading

graphite-app bot commented Dec 30, 2025 •

edited

Loading