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.

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

fix(File): respect typed array byteOffset and byteLength #1601

Merged
merged 1 commit into from
Aug 14, 2022

Conversation

mrbbot
Copy link
Contributor

@mrbbot mrbbot commented Aug 13, 2022

File ignored the byteOffset and byteLength properties of typed arrays, meaning the entire backing ArrayBuffer was included. Notably, this caused issues when passing Buffers to the File constructor.

import { File } from "undici";

const arrayBuffer = new TextEncoder().encode("abcdef").buffer;
const array = new Uint8Array(arrayBuffer, 1, 3);
const arrayFile = new File([array], "file.txt");
console.log(await arrayFile.text()); // expected "bcd", got "abcdef"

const buffer = Buffer.from("test");
const bufferFile = new File([buffer], "file.txt");
console.log(await bufferFile.text()); // expected "test", got garbage

`File` ignored the `byteOffset` and `byteLength` properties of typed
arrays, meaning the entire backing `ArrayBuffer` was included.
Notably, this caused issues when passing `Buffer`s to the `File`
constructor.
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2022

Codecov Report

Merging #1601 (2f9d403) into main (ae6f554) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1601   +/-   ##
=======================================
  Coverage   95.21%   95.21%           
=======================================
  Files          50       50           
  Lines        4748     4748           
=======================================
  Hits         4521     4521           
  Misses        227      227           
Impacted Files Coverage Δ
lib/fetch/file.js 92.85% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bytes.push(element.buffer)
bytes.push(
new Uint8Array(element.buffer, element.byteOffset, element.byteLength)
)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is still wrong. We need to make a copy, not just a new reference. Could you fix that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KhafraDev is this something you could look into? The copy part.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. On vacation for the next 6 days but I'll see if I can get to it

@ronag ronag merged commit fa9fd90 into nodejs:main Aug 14, 2022
mrbbot added a commit to cloudflare/miniflare that referenced this pull request Aug 15, 2022
mrbbot added a commit to cloudflare/miniflare that referenced this pull request Aug 15, 2022
* Bump undici version and minimum node version

To target security vulnerabilities in undici, we update to the latest version.

From undici v5.7.0, we require a node >=16.8.0 - here we update the minimum to 16.13.0 (the first Node 16 LTS release)

* Pin undici version

We should pin the version of undici

* Wrap `Body#formData()` `Buffer` chunks in `Uint8Array`s

Workaround for nodejs/undici#1601

* Delay freed check in `HTMLRewriter` `ArrayBuffer(View)` chunk test

Co-authored-by: MrBBot <me@mrbbot.dev>
ken-matsui pushed a commit to poac-dev/website that referenced this pull request Aug 19, 2022
Bumps undici from 5.8.0 to 5.9.1.

Release notes
Sourced from undici's releases.

v5.9.1
What's Changed

fix: don't timeout while waiting for client to send request (#1604)
Fix array headers by @​mateonunez in nodejs/undici#1598
fix(fetch): implement fully read body algorithm by @​KhafraDev in nodejs/undici#1597
fix: add support for integrity option to Fetch by @​jelmervdl in nodejs/undici#1596
fix(File): respect typed array byteOffset and byteLength by @​mrbbot in nodejs/undici#1601

New Contributors

@​mateonunez made their first contribution in nodejs/undici#1598
@​jelmervdl made their first contribution in nodejs/undici#1596
@​mrbbot made their first contribution in nodejs/undici#1601

Full Changelog: nodejs/undici@v5.8.2...v5.9.1
v5.8.2
⚠️ Security Release ⚠️

CRLF Injection in Nodejs ‘undici’ via Content-Type GHSA-f772-66g8-q5h3 CVE-2022-35948
undici.request vulnerable to SSRF using absolute URL on pathname GHSA-8qr4-xgw6-wmr3 CVE-2022-35949

What's Changed

docs: mock different endpoints in a single file by @​ritvik130 in nodejs/undici#1589
feat(webidl): better error message for ByteString converter by @​KhafraDev in nodejs/undici#1591

New Contributors

@​ritvik130 made their first contribution in nodejs/undici#1589

Full Changelog: nodejs/undici@v5.8.1...v5.8.2
v5.8.1
What's Changed

Do not decode the body while we are following a redirect by @​mcollina in nodejs/undici#1554
docs: Fix spelling/grammar in "Mocking Request" by @​meyfa in nodejs/undici#1555
fix(MockInterceptor): callback options.headers w/ fetch by @​KhafraDev in nodejs/undici#1559
fix(fetch): ByteString checks & conversion in Headers by @​KhafraDev in nodejs/undici#1560
test: update client certificates with ones that expires in 100 years by @​jodevsa in nodejs/undici#1566
fix: x-www-form-urlencoded parser keep the BOM by @​cola119 in nodejs/undici#1563
fix: prioritise error events over timeouts by @​jodevsa in nodejs/undici#1551
fix: add isErrorLike by @​KhafraDev in nodejs/undici#1570
fix(types): add missing pool stats by @​SkeLLLa in nodejs/undici#1573
fix: fetch a long base64 url will crash and nothing happens (close: #1574) by @​ahaoboy in nodejs/undici#1575
fix: follow signal.reason in Request by @​LiviaMedeiros in nodejs/undici#1580
docs: Fix DiagnosticsChannel sidebar link by @​trentm in nodejs/undici#1582
fix: make mock headers case-insensitive by @​cola119 in nodejs/undici#1585

New Contributors

@​meyfa made their first contribution in nodejs/undici#1555
@​cola119 made their first contribution in nodejs/undici#1563
@​SkeLLLa made their first contribution in nodejs/undici#1573



... (truncated)


Commits

5890e16 5.9.1
ecae314 fix: don't timeout while waiting for client to send request (#1604)
fa9fd90 fix(File): respect typed array byteOffset and byteLength (#1601)
ae6f554 fix: add support for integrity option to Fetch (#1596)
deed628 fix(fetch): implement fully read body algorithm (#1597)
0d1419c Fix array headers (#1598)
52d1ce5 Bumped v5.8.2
66165d6 Merge pull request from GHSA-f772-66g8-q5h3
124f7eb Merge pull request from GHSA-8qr4-xgw6-wmr3
aef314c feat(webidl): better error message for ByteString converter (#1591)
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the Security Alerts page.

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
patrickdevivo added a commit to mergestat/mergestat that referenced this pull request Aug 19, 2022
Bumps undici from 5.8.1 to 5.9.1.

Release notes
Sourced from undici's releases.

v5.9.1
What's Changed

fix: don't timeout while waiting for client to send request (#1604)
Fix array headers by @​mateonunez in nodejs/undici#1598
fix(fetch): implement fully read body algorithm by @​KhafraDev in nodejs/undici#1597
fix: add support for integrity option to Fetch by @​jelmervdl in nodejs/undici#1596
fix(File): respect typed array byteOffset and byteLength by @​mrbbot in nodejs/undici#1601

New Contributors

@​mateonunez made their first contribution in nodejs/undici#1598
@​jelmervdl made their first contribution in nodejs/undici#1596
@​mrbbot made their first contribution in nodejs/undici#1601

Full Changelog: nodejs/undici@v5.8.2...v5.9.1
v5.8.2
⚠️ Security Release ⚠️

CRLF Injection in Nodejs ‘undici’ via Content-Type GHSA-f772-66g8-q5h3 CVE-2022-35948
undici.request vulnerable to SSRF using absolute URL on pathname GHSA-8qr4-xgw6-wmr3 CVE-2022-35949

What's Changed

docs: mock different endpoints in a single file by @​ritvik130 in nodejs/undici#1589
feat(webidl): better error message for ByteString converter by @​KhafraDev in nodejs/undici#1591

New Contributors

@​ritvik130 made their first contribution in nodejs/undici#1589

Full Changelog: nodejs/undici@v5.8.1...v5.8.2



Commits

5890e16 5.9.1
ecae314 fix: don't timeout while waiting for client to send request (#1604)
fa9fd90 fix(File): respect typed array byteOffset and byteLength (#1601)
ae6f554 fix: add support for integrity option to Fetch (#1596)
deed628 fix(fetch): implement fully read body algorithm (#1597)
0d1419c Fix array headers (#1598)
52d1ce5 Bumped v5.8.2
66165d6 Merge pull request from GHSA-f772-66g8-q5h3
124f7eb Merge pull request from GHSA-8qr4-xgw6-wmr3
aef314c feat(webidl): better error message for ByteString converter (#1591)
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
@dependabot use these labels will set the current labels as the default for future PRs for this repo and language
@dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
@dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
@dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.
@mrbbot mrbbot deleted the file-byte-range branch September 11, 2022 14:56
metcoder95 pushed a commit to metcoder95/undici that referenced this pull request Dec 26, 2022
…1601)

`File` ignored the `byteOffset` and `byteLength` properties of typed
arrays, meaning the entire backing `ArrayBuffer` was included.
Notably, this caused issues when passing `Buffer`s to the `File`
constructor.
crysmags pushed a commit to crysmags/undici that referenced this pull request Feb 27, 2024
…1601)

`File` ignored the `byteOffset` and `byteLength` properties of typed
arrays, meaning the entire backing `ArrayBuffer` was included.
Notably, this caused issues when passing `Buffer`s to the `File`
constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants