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

fs.read offset and length can only be up to 0x7FFFFFFF (max int32 instead of uint32) #26563

Closed
zbjornson opened this issue Mar 10, 2019 · 0 comments
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@zbjornson
Copy link
Contributor

  • Version: v10.15.0
  • Platform: Win64, Linux (all)
  • Subsystem: fs
const fs = require("fs");
const dest = new Uint16Array(1073741824);
// dest.byteLength > buffer.kMaxLength
const fd = fs.openSync("./temp.dat", "r");
fs.readSync(fd, dest, 0, dest.byteLength, 0);

throws

RangeError [ERR_OUT_OF_RANGE]: The value of "length" is out of range. It must be >= 0 && <= 2147483648. Received -2147483648

That's because length |= 0 here converts to signed int32.

Since the max TypedArray size in v8 is expanding to MAX_SAFE_INTEGER, switching to Math.round() I think makes sense (vs adding >>> 0, which would only support uint32)? Happy to open a PR if agreed.

(#21994 looks slightly related, but is about bigint position values.)

@vsemozhetbyt vsemozhetbyt added the buffer Issues and PRs related to the buffer subsystem. label Mar 10, 2019
zbjornson added a commit to zbjornson/node that referenced this issue Mar 18, 2019
Since v10.10.0, 'buf' can be any DataView, meaning the largest
byteLength can be Float64Array.BYTES_PER_ELEMENT * kMaxLength =
17,179,869,176.

'offset' can now be up to 2**53 - 1. This makes it possible to tile
reads into a large buffer.

Ref nodejs#26563
zbjornson added a commit to zbjornson/node that referenced this issue Mar 18, 2019
Since v10.10.0, 'buf' can be any DataView, meaning the largest
byteLength can be Float64Array.BYTES_PER_ELEMENT * kMaxLength =
17,179,869,176.

The underlying libuv functions only work with unsigned int. The
underlying Linux and Windows syscalls only support up to ~2 GB and
4 GB, respectively.

Ref nodejs#26563
zbjornson added a commit to zbjornson/node that referenced this issue Mar 27, 2019
Since v10.10.0, 'buf' can be any DataView, meaning the largest
byteLength can be Float64Array.BYTES_PER_ELEMENT * kMaxLength =
17,179,869,176.

'offset' can now be up to 2**53 - 1. This makes it possible to tile
reads into a large buffer.

This commit preserves the existing behavior as far as coercing values
to numbers instead of throwing.

Fixes nodejs#26563
zbjornson added a commit to zbjornson/node that referenced this issue Mar 27, 2019
zbjornson added a commit to zbjornson/node that referenced this issue Mar 28, 2019
Since v10.10.0, 'buf' can be any DataView, meaning the largest
byteLength can be Float64Array.BYTES_PER_ELEMENT * kMaxLength =
17,179,869,176.

'offset' can now be up to 2**53 - 1. This makes it possible to tile
reads into a large buffer.

Breaking: now throws if read offset is not a safe int, is null or
is undefined.

Fixes nodejs#26563
zbjornson added a commit to zbjornson/node that referenced this issue Mar 28, 2019
zbjornson added a commit to zbjornson/node that referenced this issue Aug 12, 2019
Since v10.10.0, 'buf' can be any DataView, meaning the largest
byteLength can be Float64Array.BYTES_PER_ELEMENT * kMaxLength =
17,179,869,176.

'offset' can now be up to 2**53 - 1. This makes it possible to tile
reads into a large buffer.

Breaking: now throws if read offset is not a safe int, is null or
is undefined.

Fixes nodejs#26563
zbjornson added a commit to zbjornson/node that referenced this issue Aug 12, 2019
@Trott Trott closed this as completed in 0bbda5e Aug 17, 2019
Trott pushed a commit that referenced this issue Aug 17, 2019
Ref #26563

PR-URL: #26572
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants