Skip to content

Commit

Permalink
fs: use byteLength to handle ArrayBuffer views
Browse files Browse the repository at this point in the history
Unlike TypedArray, DataView doesn't have a length property.

PR-URL: #38187
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
  • Loading branch information
targos authored and jasnell committed Apr 12, 2021
1 parent 474fbb5 commit 6bbe285
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 34 deletions.
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 @@ -532,7 +532,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 @@ -587,15 +587,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 @@ -682,7 +682,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 @@ -280,19 +280,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 @@ -404,7 +404,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 @@ -419,12 +419,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 @@ -447,7 +447,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 @@ -457,7 +457,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);
}));
}

0 comments on commit 6bbe285

Please sign in to comment.