Skip to content

src: avoid truncating Blob sizes to 32 bits - #66207

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:blob-size-truncation
Open

christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:blob-size-truncation

Conversation

@christianaurichzm

Copy link
Copy Markdown
Contributor

Blob sizes cross between C++ and JavaScript as uint32 values, while the
underlying DataQueue tracks them as uint64. openAsBlob() on a 5 GiB file
therefore returned a Blob of size 1073741824.

Reporting the real size is not enough on its own. The inbound direction
assumes a uint32 as well, so a correct size reaches CHECK(IsUint32()) in
ToSlice() and StoreDataObject() and aborts blob.slice() and
URL.createObjectURL(). StoreDataObject() does not need openAsBlob() to be
reached, since an in-memory Blob is bounded by Uint8Array::kMaxLength
rather than by 4 GiB.

Pass the sizes as doubles and keep them as uint64_t in C++. Everything
below this boundary, FdEntry and FileHandle included, is already 64-bit,
so this only stops the narrowing at the edge. It also removes the
truncation on 32-bit platforms, where size_t was too narrow as well.

Fixes: #52585
Refs: #52588

Blob sizes cross between C++ and JavaScript as uint32 values, while the
underlying DataQueue tracks them as uint64. openAsBlob() on a 5 GiB
file therefore returned a Blob of size 1073741824.

Reporting the real size is not enough on its own. The inbound direction
assumes a uint32 as well, so a correct size reaches CHECK(IsUint32())
in ToSlice() and StoreDataObject() and aborts blob.slice() and
URL.createObjectURL(). Pass the sizes as doubles and keep them as
uint64_t in C++. Everything below this boundary is already 64-bit, so
this only stops the narrowing at the edge.

Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 22, 2026
@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.29%. Comparing base (1eacebb) to head (86ec7c9).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/node_blob.cc 76.92% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #66207    +/-   ##
========================================
  Coverage   90.29%   90.29%            
========================================
  Files         790      790            
  Lines      272529   272883   +354     
  Branches    52037    52111    +74     
========================================
+ Hits       246074   246404   +330     
- Misses      16908    16940    +32     
+ Partials     9547     9539     -8     
Files with missing lines Coverage Δ
src/node_blob.h 37.50% <100.00%> (ø)
src/node_blob.cc 73.49% <76.92%> (ø)

... and 39 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.

@christianaurichzm

Copy link
Copy Markdown
Contributor Author

@jasnell review appreciated when you have time. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.openAsBlob() does not work properly for files > 2GB

2 participants