From 6bbe28552ced571bec3a21861cf16987927fa056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Apr 2021 17:20:30 +0200 Subject: [PATCH] fs: use byteLength to handle ArrayBuffer views Unlike TypedArray, DataView doesn't have a length property. PR-URL: https://github.com/nodejs/node/pull/38187 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Darshan Sen --- doc/api/fs.md | 48 +++++++++++++++------------ lib/fs.js | 10 +++--- lib/internal/fs/promises.js | 16 ++++----- test/parallel/test-fs-write-buffer.js | 17 ++++++++++ 4 files changed, 57 insertions(+), 34 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 6ac6e01202f55b..dc31bced554cd9 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -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. @@ -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. @@ -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) @@ -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 @@ -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'` @@ -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'` @@ -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 @@ -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} @@ -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} diff --git a/lib/fs.js b/lib/fs.js index 937b45cdb549a3..e6cf8527e8720d 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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); } @@ -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 { @@ -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); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 683969e2ec2b50..8b864874eb1114 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -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, @@ -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; } @@ -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; @@ -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)) { @@ -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); diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js index 96e26ef0afebf2..a70c44eaceffe5 100644 --- a/test/parallel/test-fs-write-buffer.js +++ b/test/parallel/test-fs-write-buffer.js @@ -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); + })); +}