GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — buffer: remove lines setting indexes to integer value from `Blob.slice` by kylo5aby · Pull Request #52588 · nodejs/node

Conversation

@kylo5aby

@kylo5aby kylo5aby commented Apr 19, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Since blob allows maximum kLength of 2GB, the indexes maybe out of signed integer range.
remove lines setting indexes to integer value from Blob.slice.
they will be checked in Cpp side(Blob::ToSlice) if they were not in unsigned integer range.
Refs: #52585

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 19, 2024
@kylo5aby
kylo5aby force-pushed the blob branch 2 times, most recently from d31f4f4 to a00470f Compare April 22, 2024 01:49
@kylo5aby kylo5aby mentioned this pull request Apr 23, 2024
fs.openAsBlob() does not work properly for files > 2GB #52585
Closed
aduh95
aduh95 approved these changes Apr 24, 2024
View reviewed changes
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Apr 24, 2024
@github-actions 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 Apr 24, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 24, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58676/
All reactions

Sorry, something went wrong.

Uh oh!

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

kylo5aby commented Apr 25, 2024

Copy link
Copy Markdown
Contributor Author

hi @aduh95 , I see an error related to test-error-sedes.js in CI. In the test cases I added, I created a potentially time-consuming large Buffer and placed it in the pummel directory, and skipped the test for 32-bit platforms just like other types of test cases. I believe my changes shouldn't affect test-error-sedes. Could you take a look?
I
image
aduh95 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell approved these changes Apr 27, 2024
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58946/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58949/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58951/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58958/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58964/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58967/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58971/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kylo5aby @aduh95
buffer: remove lines setting indexes to integer value
66c36b4
@aduh95
aduh95 force-pushed the blob branch from a00470f to 66c36b4 Compare May 6, 2024 07:08
@nodejs-github-bot

nodejs-github-bot commented May 6, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58978/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 approved these changes May 6, 2024
View reviewed changes
@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label May 6, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label May 6, 2024
@nodejs-github-bot
nodejs-github-bot merged commit bf8afe7 into nodejs:main May 6, 2024
@nodejs-github-bot

nodejs-github-bot commented May 6, 2024

Copy link
Copy Markdown
Collaborator

Landed in bf8afe7
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos pushed a commit that referenced this pull request May 8, 2024
@kylo5aby @targos
buffer: remove lines setting indexes to integer value
f1bc994 
PR-URL: #52588
Refs: #52585
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@targos targos mentioned this pull request May 13, 2024
v22.2.0 release proposal #52971
Merged
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
@kylo5aby @marco-ippolito
buffer: remove lines setting indexes to integer value
16ae2b2 
PR-URL: #52588
Refs: #52585
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Jun 17, 2024
v20.15.0 proposal #53486
Merged
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
@kylo5aby @soophoo
buffer: remove lines setting indexes to integer value
ac5bb3d 
PR-URL: nodejs#52588
Refs: nodejs#52585
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@christianaurichzm christianaurichzm mentioned this pull request Sep 22, 2026
src: avoid truncating Blob sizes to 32 bits #66207
Merged
nodejs-github-bot pushed a commit that referenced this pull request Oct 22, 2026
@christianaurichzm
src: avoid truncating Blob sizes to 32 bits
1c12ff1 
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 <[email protected]>
PR-URL: #66207
Fixes: #52585
Refs: #52588
Reviewed-By: Filip Skokan <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 12, 2026
@christianaurichzm @aduh95
src: avoid truncating Blob sizes to 32 bits
08ea9a1 
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 <[email protected]>
PR-URL: #66207
Fixes: #52585
Refs: #52588
Reviewed-By: Filip Skokan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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.

Uh oh!

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

4 participants

@kylo5aby @nodejs-github-bot @jasnell @aduh95
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.