FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: fix creating an ArrayBuffer from a Blob created with `openAsBlob` by daeyeon · Pull Request #47691 · nodejs/node · GitHub

/ node Public

src: fix creating an ArrayBuffer from a Blob created with openAsBlob - #47691

Closed
daeyeon wants to merge 1 commit into
nodejs:mainfrom
daeyeon:main.fix/47683-230424.Mon.d73a
Closed

src: fix creating an ArrayBuffer from a Blob created with openAsBlob#47691
daeyeon wants to merge 1 commit into
nodejs:mainfrom
daeyeon:main.fix/47683-230424.Mon.d73a

Conversation

daeyeon commented Apr 24, 2023

Copy link
Copy Markdown
Member

Fixes: #47683

This fixes creating an ArrayBuffer from a Blob sliced from a Blob created with fs.openAsBlob.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

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 Apr 24, 2023
daeyeon marked this pull request as draft April 24, 2023 05:54
daeyeon force-pushed the main.fix/47683-230424.Mon.d73a branch from 2b97826 to babd5a0 Compare April 24, 2023 06:03
daeyeon marked this pull request as ready for review April 24, 2023 06:15

Copy link
Copy Markdown

Thanks! Assume the fix should also apply to .stream() - the issue manifested in both .arrayBuffer() and .stream() but reported for arrayBuffer() as that was simpler repro, but perhaps worth adding a test for stream as well?

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
daeyeon force-pushed the main.fix/47683-230424.Mon.d73a branch from babd5a0 to 6e40de0 Compare April 25, 2023 03:00

daeyeon commented Apr 25, 2023

Copy link
Copy Markdown
Member Author

@ikreymer That would be better. Updated the test for stream also, PTAL.

Copy link
Copy Markdown

@ikreymer That would be better. Updated the test for stream also, PTAL.

Yes, that looks good - thanks!

daeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 26, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 26, 2023

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

daeyeon added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 27, 2023
jasnell pushed a commit that referenced this pull request May 5, 2023
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: #47691
Fixes: #47683
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented May 5, 2023

Copy link
Copy Markdown
Member

Landed in af9b48a

jasnell closed this May 5, 2023
daeyeon deleted the main.fix/47683-230424.Mon.d73a branch May 5, 2023 02:18
targos pushed a commit that referenced this pull request May 12, 2023
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: #47691
Fixes: #47683
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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.

slice on blob created with fs.openAsBlob results in incorrect length (end + start instead of end - start?)

5 participants


Back | FazBrowse Home | New Git URL