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: use byteLength to handle ArrayBuffer views #38187

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 27 additions & 21 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,18 +265,20 @@ added: v10.0.0
added: v10.0.0
-->

* `buffer` {Buffer|Uint8Array} A buffer that will be filled with the file
data read.
* `buffer` {Buffer|TypedArray|DataView} A buffer that will be filled with the
file data read.
* `offset` {integer} The location in the buffer at which to start filling.
**Default:** `0`
* `length` {integer} The number of bytes to read. **Default:** `buffer.length`
* `length` {integer} The number of bytes to read. **Default:**
`buffer.byteLength`
* `position` {integer} The location where to begin reading data from the
file. If `null`, data will be read from the current file position, and
the position will be updated. If `position` is an integer, the current
file position will remain unchanged.
* Returns: {Promise} Fulfills upon success with an object with two properties:
* `bytesRead` {integer} The number of bytes read
* `buffer` {Buffer|Uint8Array} A reference to the passed in `buffer` argument.
* `buffer` {Buffer|TypedArray|DataView} A reference to the passed in `buffer`
argument.

Reads data from the file and stores that in the given buffer.

Expand All @@ -290,19 +292,20 @@ added:
- v12.17.0
-->
* `options` {Object}
* `buffer` {Buffer|Uint8Array} A buffer that will be filled with the file
data read. **Default:** `Buffer.alloc(16384)`
* `buffer` {Buffer|TypedArray|DataView} A buffer that will be filled with the
file data read. **Default:** `Buffer.alloc(16384)`
* `offset` {integer} The location in the buffer at which to start filling.
**Default:** `0`
* `length` {integer} The number of bytes to read. **Default:** `buffer.length`
* `length` {integer} The number of bytes to read. **Default:**
`buffer.byteLength`
* `position` {integer} The location where to begin reading data from the
file. If `null`, data will be read from the current file position, and
the position will be updated. If `position` is an integer, the current
file position will remain unchanged. **Default:**: `null`
* Returns: {Promise} Fulfills upon success with an object with two properties:
* `bytesRead` {integer} The number of bytes read
* `buffer` {Buffer|Uint8Array} A reference to the passed in `buffer`
argument.
* `buffer` {Buffer|TypedArray|DataView} A reference to the passed in `buffer`
argument.

Reads data from the file and stores that in the given buffer.

Expand Down Expand Up @@ -434,10 +437,11 @@ changes:
buffers anymore.
-->

* `buffer` {Buffer|Uint8Array|string|Object}
* `buffer` {Buffer|TypedArray|DataView|string|Object}
* `offset` {integer} The start position from within `buffer` where the data
to write begins.
* `length` {integer} The number of bytes from `buffer` to write.
to write begins. **Default:** `0`
* `length` {integer} The number of bytes from `buffer` to write. **Default:**
`buffer.byteLength`
* `position` {integer} The offset from the beginning of the file where the
data from `buffer` should be written. If `position` is not a `number`,
the data will be written at the current position. See the POSIX pwrite(2)
Expand All @@ -449,8 +453,8 @@ Write `buffer` to the file.
The promise is resolved with an object containing two properties:

* `bytesWritten` {integer} the number of bytes written
* `buffer` {Buffer|Uint8Array|string|Object} a reference to the `buffer`
written.
* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the
`buffer` written.

It is unsafe to use `filehandle.write()` multiple times on the same file
without waiting for the promise to be resolved (or rejected). For this
Expand Down Expand Up @@ -512,7 +516,7 @@ changes:
strings anymore.
-->

* `data` {string|Buffer|Uint8Array|Object}
* `data` {string|Buffer|TypedArray|DataView|Object}
* `options` {Object|string}
* `encoding` {string|null} The expected character encoding when `data` is a
string. **Default:** `'utf8'`
Expand Down Expand Up @@ -1270,7 +1274,7 @@ changes:
-->

* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle`
* `data` {string|Buffer|Uint8Array|Object|AsyncIterable|Iterable
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
|Stream}
* `options` {Object|string}
* `encoding` {string|null} **Default:** `'utf8'`
Expand Down Expand Up @@ -2722,9 +2726,11 @@ changes:

* `fd` {integer}
* `buffer` {Buffer|TypedArray|DataView} The buffer that the data will be
written to.
* `offset` {integer} The position in `buffer` to write the data to.
* `length` {integer} The number of bytes to read.
written to. **Default:** `Buffer.alloc(16384)`
* `offset` {integer} The position in `buffer` to write the data to. **Default:**
`0`
* `length` {integer} The number of bytes to read. **Default:**
`buffer.byteLength`
* `position` {integer|bigint} Specifies where to begin reading from in the
file. If `position` is `null` or `-1 `, data will be read from the current
file position, and the file position will be updated. If `position` is an
Expand Down Expand Up @@ -2761,7 +2767,7 @@ changes:
* `options` {Object}
* `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
* `offset` {integer} **Default:** `0`
* `length` {integer} **Default:** `buffer.length`
* `length` {integer} **Default:** `buffer.byteLength`
* `position` {integer|bigint} **Default:** `null`
* `callback` {Function}
* `err` {Error}
Expand Down Expand Up @@ -4689,7 +4695,7 @@ changes:
* `buffer` {Buffer|TypedArray|DataView}
* `options` {Object}
* `offset` {integer} **Default:** `0`
* `length` {integer} **Default:** `buffer.length`
* `length` {integer} **Default:** `buffer.byteLength`
* `position` {integer|bigint} **Default:** `null`
* Returns: {number}

Expand Down
10 changes: 5 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ function read(fd, buffer, offset, length, position, callback) {
({
buffer = Buffer.alloc(16384),
offset = 0,
length = buffer.length,
length = buffer.byteLength,
position
} = options);
}
Expand Down Expand Up @@ -586,15 +586,15 @@ ObjectDefineProperty(read, internalUtil.customPromisifyArgs,
function readSync(fd, buffer, offset, length, position) {
fd = getValidatedFd(fd);

validateBuffer(buffer);

if (arguments.length <= 3) {
// Assume fs.read(fd, buffer, options)
const options = offset || {};

({ offset = 0, length = buffer.length, position } = options);
({ offset = 0, length = buffer.byteLength, position } = options);
}

validateBuffer(buffer);

if (offset == null) {
offset = 0;
} else {
Expand Down Expand Up @@ -681,7 +681,7 @@ function write(fd, buffer, offset, length, position, callback) {
validateInteger(offset, 'offset');
}
if (typeof length !== 'number')
length = buffer.length - offset;
length = buffer.byteLength - offset;
if (typeof position !== 'number')
position = null;
validateOffsetLengthWrite(offset, length, buffer.byteLength);
Expand Down
16 changes: 8 additions & 8 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,19 +279,19 @@ async function writeFileHandle(filehandle, data, signal, encoding) {
checkAborted(signal);
await write(
filehandle, buf, undefined,
isArrayBufferView(buf) ? buf.length : encoding);
isArrayBufferView(buf) ? buf.byteLength : encoding);
checkAborted(signal);
}
return;
}
data = new Uint8Array(data.buffer, data.byteOffset, data.byteLength);
let remaining = data.length;
let remaining = data.byteLength;
if (remaining === 0) return;
do {
checkAborted(signal);
const { bytesWritten } =
await write(filehandle, data, 0,
MathMin(kWriteFileMaxChunkSize, data.length));
MathMin(kWriteFileMaxChunkSize, data.byteLength));
remaining -= bytesWritten;
data = new Uint8Array(
data.buffer,
Expand Down Expand Up @@ -403,7 +403,7 @@ async function read(handle, bufferOrOptions, offset, length, position) {
buffer = Buffer.alloc(16384);
}
offset = bufferOrOptions.offset || 0;
length = buffer.length;
length = buffer.byteLength;
position = bufferOrOptions.position || null;
}

Expand All @@ -418,12 +418,12 @@ async function read(handle, bufferOrOptions, offset, length, position) {
if (length === 0)
return { bytesRead: length, buffer };

if (buffer.length === 0) {
if (buffer.byteLength === 0) {
throw new ERR_INVALID_ARG_VALUE('buffer', buffer,
'is empty and cannot be written');
}

validateOffsetLengthRead(offset, length, buffer.length);
validateOffsetLengthRead(offset, length, buffer.byteLength);

if (!NumberIsSafeInteger(position))
position = -1;
Expand All @@ -446,7 +446,7 @@ async function readv(handle, buffers, position) {
}

async function write(handle, buffer, offset, length, position) {
if (buffer?.length === 0)
if (buffer?.byteLength === 0)
return { bytesWritten: 0, buffer };

if (isArrayBufferView(buffer)) {
Expand All @@ -456,7 +456,7 @@ async function write(handle, buffer, offset, length, position) {
validateInteger(offset, 'offset');
}
if (typeof length !== 'number')
length = buffer.length - offset;
length = buffer.byteLength - offset;
if (typeof position !== 'number')
position = null;
validateOffsetLengthWrite(offset, length, buffer.byteLength);
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-fs-write-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,20 @@ tmpdir.refresh();
fs.closeSync(fd);
}));
}

// fs.write with a DataView, without the offset and length parameters:
{
const filename = path.join(tmpdir.path, 'write8.txt');
fs.open(filename, 'w', 0o644, common.mustSucceed((fd) => {
const cb = common.mustSucceed((written) => {
assert.strictEqual(written, expected.length);
fs.closeSync(fd);

const found = fs.readFileSync(filename, 'utf8');
assert.strictEqual(found, expected.toString());
});

const uint8 = Uint8Array.from(expected);
fs.write(fd, new DataView(uint8.buffer), cb);
}));
}