GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — ffi: reject detached ArrayBufferViews by trivikr · Pull Request #65086 · nodejs/node

Conversation

trivikr commented Aug 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Fixes: #65085

Detached ArrayBuffers were rejected with ERR_INVALID_ARG_VALUE, but detached views were not: typed arrays exported zero bytes and DataViews threw a bare TypeError from byteLength.

Track detachment in ArrayBufferViewContents::Read() and skip the JS length check for detached buffers, so every detached input is rejected.

Assisted-by: codex:gpt-5.6-sol

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

nodejs-github-bot commented Aug 6, 2026

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/ffi

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

ffi

Issues and PRs related to experimental Foreign Function Interface support.

needs-ci PRs that need a full CI run. labels

Aug 6, 2026

trivikr force-pushed the ffi-export-arraybufferview-detached-handling branch 2 times, most recently from 6a14197 to ebb66f2 Compare

August 7, 2026 02:59

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 7, 2026

codecov Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (175cd52) to head (2b7d621).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65086      +/-   ##
==========================================
- Coverage   92.06%   90.33%   -1.73%     
==========================================
  Files         399      760     +361     
  Lines      176328   248541   +72213     
  Branches    27212    46907   +19695     
==========================================
+ Hits       162328   224513   +62185     
- Misses      13689    15464    +1775     
- Partials      311     8564    +8253

Files with missing lines	Coverage Δ
lib/ffi.js	`94.77% <100.00%> (ø)`
src/util-inl.h	`83.88% <100.00%> (ø)`

... and 491 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This comment was marked as low quality.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trivikr removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 10, 2026

This comment was marked as outdated.


          ffi: reject detached ArrayBufferViews

2b7d621

Detached ArrayBuffers were rejected with ERR_INVALID_ARG_VALUE, but
detached views were not: typed arrays exported zero bytes and DataViews
threw a bare TypeError from byteLength.

Track detachment in ArrayBufferViewContents::Read() and skip the JS
length check for detached buffers, so every detached input is rejected.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: codex:gpt-5.6-sol

trivikr force-pushed the ffi-export-arraybufferview-detached-handling branch from ebb66f2 to 2b7d621 Compare

August 11, 2026 15:25

ShogunPanda approved these changes

Aug 12, 2026

View reviewed changes

trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 12, 2026

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels

Aug 12, 2026

nodejs-github-bot commented Aug 12, 2026

Copy link

Copy Markdown

Collaborator

Commit Queue failed

- Loading data for nodejs/node/pull/65086
✔  Done loading data for nodejs/node/pull/65086
----------------------------------- PR info ------------------------------------
Title      ffi: reject detached ArrayBufferViews (#65086)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     trivikr:ffi-export-arraybufferview-detached-handling -> nodejs:main
Labels     c++, needs-ci, commit-queue, ffi
Commits    1
 - ffi: reject detached ArrayBufferViews
Committers 1
 - Kamat, Trivikram <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/65086
Fixes: https://github.com/nodejs/node/issues/65085
Reviewed-By: Paolo Insogna <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65086
Fixes: https://github.com/nodejs/node/issues/65085
Reviewed-By: Paolo Insogna <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 06 Aug 2026 15:16:59 GMT
   ✔  Approvals: 1
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/65086#pullrequestreview-4914088434
   ✘  This PR needs to wait 24 more hours to land (or 0 minutes if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-08-10T20:56:16Z: https://ci.nodejs.org/job/node-test-pull-request/75752/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - ffi: reject detached ArrayBufferViews
- Querying data for job/node-test-pull-request/75752/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/31614572726

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trivikr added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels

Aug 12, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 12, 2026

This comment was marked as outdated.

nodejs-github-bot commented Aug 13, 2026

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75806/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 13, 2026

nodejs-github-bot merged commit c47ad66 into nodejs:main

Aug 13, 2026

79 checks passed

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Aug 13, 2026

Copy link

Copy Markdown

Collaborator

Landed in c47ad66

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 13, 2026

trivikr deleted the ffi-export-arraybufferview-detached-handling branch

August 16, 2026 16:01

aduh95 pushed a commit that referenced this pull request

Aug 25, 2026


          ffi: reject detached ArrayBufferViews

f2c8454

Detached ArrayBuffers were rejected with ERR_INVALID_ARG_VALUE, but
detached views were not: typed arrays exported zero bytes and DataViews
threw a bare TypeError from byteLength.

Track detachment in ArrayBufferViewContents::Read() and skip the JS
length check for detached buffers, so every detached input is rejected.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: codex:gpt-5.6-sol
PR-URL: #65086
Fixes: #65085
Reviewed-By: Paolo Insogna <[email protected]>

aduh95 pushed a commit that referenced this pull request

Aug 25, 2026


          ffi: reject detached ArrayBufferViews

eacd65e

Detached ArrayBuffers were rejected with ERR_INVALID_ARG_VALUE, but
detached views were not: typed arrays exported zero bytes and DataViews
threw a bare TypeError from byteLength.

Track detachment in ArrayBufferViewContents::Read() and skip the JS
length check for detached buffers, so every detached input is rejected.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: codex:gpt-5.6-sol
PR-URL: #65086
Fixes: #65085
Reviewed-By: Paolo Insogna <[email protected]>

github-actions Bot mentioned this pull request

Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Merged

Reviewers

ShogunPanda

ShogunPanda approved these changes

+1 more reviewer

MILLERMARRU

MILLERMARRU left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++

Issues and PRs that require attention from people who are familiar with C++.

ffi

Issues and PRs related to experimental Foreign Function Interface support.

needs-ci

PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ffi: exportArrayBufferView() inconsistently handles detached views

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

trivikr commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

codecov Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as low quality.

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Aug 12, 2026

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Aug 13, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

trivikr commented Aug 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

codecov Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.