Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

buffer: remove lines setting indexes to integer value from Blob.slice #52588

Merged
merged 1 commit into from
May 6, 2024
Merged

buffer: remove lines setting indexes to integer value from Blob.slice #52588

merged 1 commit into from
May 6, 2024

Conversation

kylo5aby
Copy link
Contributor

@kylo5aby kylo5aby commented Apr 19, 2024 •
edited
Loading

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

@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
Open
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 24, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 24, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@kylo5aby
Copy link
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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2024
@nodejs-github-bot nodejs-github-bot merged commit bf8afe7 into nodejs:main May 6, 2024
53 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bf8afe7

Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@kylo5aby @Ch3nYuY
buffer: remove lines setting indexes to integer value
16b7de6 
PR-URL: nodejs#52588
Refs: nodejs#52585
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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 <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request May 13, 2024
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 <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@marco-ippolito marco-ippolito mentioned this pull request Jun 17, 2024
Merged
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
@kylo5aby @eliphazbouye
buffer: remove lines setting indexes to integer value
ac5bb3d 
PR-URL: nodejs#52588
Refs: nodejs#52585
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@kylo5aby @bmeck
buffer: remove lines setting indexes to integer value
2619301 
PR-URL: nodejs#52588
Refs: nodejs#52585
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.
4 participants
@kylo5aby @nodejs-github-bot @jasnell @aduh95