mirror of https://github.com/nodejs/node.git
fs: make params in writing methods optional
This change allows passing objects as "named parameters": - `fs.write(fd, buffer[, options], callback)` - `fs.writeSync(fd, buffer[, options])` - `filehandle.write(buffer[, options])` Fixes: https://github.com/nodejs/node/issues/41666 PR-URL: https://github.com/nodejs/node/pull/42601 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>pull/42601/head
parent
ace89d9a89
commit
c100f9ad29
|
@ -608,7 +608,7 @@ added: v10.0.0
|
|||
Change the file system timestamps of the object referenced by the {FileHandle}
|
||||
then resolves the promise with no arguments upon success.
|
||||
|
||||
#### `filehandle.write(buffer[, offset[, length[, position]]])`
|
||||
#### `filehandle.write(buffer, offset[, length[, position]])`
|
||||
|
||||
<!-- YAML
|
||||
added: v10.0.0
|
||||
|
@ -621,7 +621,7 @@ changes:
|
|||
|
||||
* `buffer` {Buffer|TypedArray|DataView}
|
||||
* `offset` {integer} The start position from within `buffer` where the data
|
||||
to write begins. **Default:** `0`
|
||||
to write begins.
|
||||
* `length` {integer} The number of bytes from `buffer` to write. **Default:**
|
||||
`buffer.byteLength - offset`
|
||||
* `position` {integer|null} The offset from the beginning of the file where the
|
||||
|
@ -646,6 +646,25 @@ On Linux, positional writes do not work when the file is opened in append mode.
|
|||
The kernel ignores the position argument and always appends the data to
|
||||
the end of the file.
|
||||
|
||||
#### `filehandle.write(buffer[, options])`
|
||||
|
||||
<!-- YAML
|
||||
added: REPLACEME
|
||||
-->
|
||||
|
||||
* `buffer` {Buffer|TypedArray|DataView}
|
||||
* `options` {Object}
|
||||
* `offset` {integer} **Default:** `0`
|
||||
* `length` {integer} **Default:** `buffer.byteLength - offset`
|
||||
* `position` {integer} **Default:** `null`
|
||||
* Returns: {Promise}
|
||||
|
||||
Write `buffer` to the file.
|
||||
|
||||
Similar to the above `filehandle.write` function, this version takes an
|
||||
optional `options` object. If no `options` object is specified, it will
|
||||
default with the above values.
|
||||
|
||||
#### `filehandle.write(string[, position[, encoding]])`
|
||||
|
||||
<!-- YAML
|
||||
|
@ -4376,7 +4395,7 @@ This happens when:
|
|||
* the file is deleted, followed by a restore
|
||||
* the file is renamed and then renamed a second time back to its original name
|
||||
|
||||
### `fs.write(fd, buffer[, offset[, length[, position]]], callback)`
|
||||
### `fs.write(fd, buffer, offset[, length[, position]], callback)`
|
||||
|
||||
<!-- YAML
|
||||
added: v0.0.2
|
||||
|
@ -4443,6 +4462,29 @@ On Linux, positional writes don't work when the file is opened in append mode.
|
|||
The kernel ignores the position argument and always appends the data to
|
||||
the end of the file.
|
||||
|
||||
### `fs.write(fd, buffer[, options], callback)`
|
||||
|
||||
<!-- YAML
|
||||
added: REPLACEME
|
||||
-->
|
||||
|
||||
* `fd` {integer}
|
||||
* `buffer` {Buffer|TypedArray|DataView}
|
||||
* `options` {Object}
|
||||
* `offset` {integer} **Default:** `0`
|
||||
* `length` {integer} **Default:** `buffer.byteLength - offset`
|
||||
* `position` {integer} **Default:** `null`
|
||||
* `callback` {Function}
|
||||
* `err` {Error}
|
||||
* `bytesWritten` {integer}
|
||||
* `buffer` {Buffer|TypedArray|DataView}
|
||||
|
||||
Write `buffer` to the file specified by `fd`.
|
||||
|
||||
Similar to the above `fs.write` function, this version takes an
|
||||
optional `options` object. If no `options` object is specified, it will
|
||||
default with the above values.
|
||||
|
||||
### `fs.write(fd, string[, position[, encoding]], callback)`
|
||||
|
||||
<!-- YAML
|
||||
|
@ -5793,7 +5835,7 @@ for more details.
|
|||
For detailed information, see the documentation of the asynchronous version of
|
||||
this API: [`fs.writeFile()`][].
|
||||
|
||||
### `fs.writeSync(fd, buffer[, offset[, length[, position]]])`
|
||||
### `fs.writeSync(fd, buffer, offset[, length[, position]])`
|
||||
|
||||
<!-- YAML
|
||||
added: v0.1.21
|
||||
|
@ -5824,6 +5866,23 @@ changes:
|
|||
For detailed information, see the documentation of the asynchronous version of
|
||||
this API: [`fs.write(fd, buffer...)`][].
|
||||
|
||||
### `fs.writeSync(fd, buffer[, options])`
|
||||
|
||||
<!-- YAML
|
||||
added: REPLACEME
|
||||
-->
|
||||
|
||||
* `fd` {integer}
|
||||
* `buffer` {Buffer|TypedArray|DataView}
|
||||
* `options` {Object}
|
||||
* `offset` {integer} **Default:** `0`
|
||||
* `length` {integer} **Default:** `buffer.byteLength - offset`
|
||||
* `position` {integer} **Default:** `null`
|
||||
* Returns: {number} The number of bytes written.
|
||||
|
||||
For detailed information, see the documentation of the asynchronous version of
|
||||
this API: [`fs.write(fd, buffer...)`][].
|
||||
|
||||
### `fs.writeSync(fd, string[, position[, encoding]])`
|
||||
|
||||
<!-- YAML
|
||||
|
|
37
lib/fs.js
37
lib/fs.js
|
@ -634,7 +634,7 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
|
|||
({
|
||||
offset = 0,
|
||||
length = buffer.byteLength - offset,
|
||||
position = null
|
||||
position = null,
|
||||
} = params ?? ObjectCreate(null));
|
||||
}
|
||||
|
||||
|
@ -705,7 +705,7 @@ function readSync(fd, buffer, offset, length, position) {
|
|||
({
|
||||
offset = 0,
|
||||
length = buffer.byteLength - offset,
|
||||
position = null
|
||||
position = null,
|
||||
} = options);
|
||||
}
|
||||
|
||||
|
@ -801,7 +801,7 @@ function readvSync(fd, buffers, position) {
|
|||
* Writes `buffer` to the specified `fd` (file descriptor).
|
||||
* @param {number} fd
|
||||
* @param {Buffer | TypedArray | DataView | string | object} buffer
|
||||
* @param {number} [offset]
|
||||
* @param {number | object} [offsetOrOptions]
|
||||
* @param {number} [length]
|
||||
* @param {number | null} [position]
|
||||
* @param {(
|
||||
|
@ -811,7 +811,7 @@ function readvSync(fd, buffers, position) {
|
|||
* ) => any} callback
|
||||
* @returns {void}
|
||||
*/
|
||||
function write(fd, buffer, offset, length, position, callback) {
|
||||
function write(fd, buffer, offsetOrOptions, length, position, callback) {
|
||||
function wrapper(err, written) {
|
||||
// Retain a reference to buffer so that it can't be GC'ed too soon.
|
||||
callback(err, written || 0, buffer);
|
||||
|
@ -819,8 +819,18 @@ function write(fd, buffer, offset, length, position, callback) {
|
|||
|
||||
fd = getValidatedFd(fd);
|
||||
|
||||
let offset = offsetOrOptions;
|
||||
if (isArrayBufferView(buffer)) {
|
||||
callback = maybeCallback(callback || position || length || offset);
|
||||
|
||||
if (typeof offset === 'object') {
|
||||
({
|
||||
offset = 0,
|
||||
length = buffer.byteLength - offset,
|
||||
position = null,
|
||||
} = offsetOrOptions ?? ObjectCreate(null));
|
||||
}
|
||||
|
||||
if (offset == null || typeof offset === 'function') {
|
||||
offset = 0;
|
||||
} else {
|
||||
|
@ -869,16 +879,27 @@ ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
|
|||
* specified `fd` (file descriptor).
|
||||
* @param {number} fd
|
||||
* @param {Buffer | TypedArray | DataView | string} buffer
|
||||
* @param {number} [offset]
|
||||
* @param {number} [length]
|
||||
* @param {number | null} [position]
|
||||
* @param {{
|
||||
* offset?: number;
|
||||
* length?: number;
|
||||
* position?: number | null;
|
||||
* }} [offsetOrOptions]
|
||||
* @returns {number}
|
||||
*/
|
||||
function writeSync(fd, buffer, offset, length, position) {
|
||||
function writeSync(fd, buffer, offsetOrOptions, length, position) {
|
||||
fd = getValidatedFd(fd);
|
||||
const ctx = {};
|
||||
let result;
|
||||
|
||||
let offset = offsetOrOptions;
|
||||
if (isArrayBufferView(buffer)) {
|
||||
if (typeof offset === 'object') {
|
||||
({
|
||||
offset = 0,
|
||||
length = buffer.byteLength - offset,
|
||||
position = null
|
||||
} = offsetOrOptions ?? ObjectCreate(null));
|
||||
}
|
||||
if (position === undefined)
|
||||
position = null;
|
||||
if (offset == null) {
|
||||
|
|
|
@ -569,11 +569,20 @@ async function readv(handle, buffers, position) {
|
|||
return { bytesRead, buffers };
|
||||
}
|
||||
|
||||
async function write(handle, buffer, offset, length, position) {
|
||||
async function write(handle, buffer, offsetOrOptions, length, position) {
|
||||
if (buffer?.byteLength === 0)
|
||||
return { bytesWritten: 0, buffer };
|
||||
|
||||
let offset = offsetOrOptions;
|
||||
if (isArrayBufferView(buffer)) {
|
||||
if (typeof offset === 'object') {
|
||||
({
|
||||
offset = 0,
|
||||
length = buffer.byteLength - offset,
|
||||
position = null
|
||||
} = offsetOrOptions ?? ObjectCreate(null));
|
||||
}
|
||||
|
||||
if (offset == null) {
|
||||
offset = 0;
|
||||
} else {
|
||||
|
|
|
@ -0,0 +1,95 @@
|
|||
'use strict';
|
||||
|
||||
const common = require('../common');
|
||||
|
||||
// This test ensures that filehandle.write accepts "named parameters" object
|
||||
// and doesn't interpret objects as strings
|
||||
|
||||
const assert = require('assert');
|
||||
const fsPromises = require('fs').promises;
|
||||
const path = require('path');
|
||||
const tmpdir = require('../common/tmpdir');
|
||||
|
||||
tmpdir.refresh();
|
||||
|
||||
const dest = path.resolve(tmpdir.path, 'tmp.txt');
|
||||
const buffer = Buffer.from('zyx');
|
||||
|
||||
async function testInvalid(dest, expectedCode, ...params) {
|
||||
let fh;
|
||||
try {
|
||||
fh = await fsPromises.open(dest, 'w+');
|
||||
await assert.rejects(
|
||||
fh.write(...params),
|
||||
{ code: expectedCode });
|
||||
} finally {
|
||||
await fh?.close();
|
||||
}
|
||||
}
|
||||
|
||||
async function testValid(dest, buffer, options) {
|
||||
let fh;
|
||||
try {
|
||||
fh = await fsPromises.open(dest, 'w+');
|
||||
const writeResult = await fh.write(buffer, options);
|
||||
const writeBufCopy = Uint8Array.prototype.slice.call(writeResult.buffer);
|
||||
|
||||
const readResult = await fh.read(buffer, options);
|
||||
const readBufCopy = Uint8Array.prototype.slice.call(readResult.buffer);
|
||||
|
||||
assert.ok(writeResult.bytesWritten >= readResult.bytesRead);
|
||||
if (options.length !== undefined && options.length !== null) {
|
||||
assert.strictEqual(writeResult.bytesWritten, options.length);
|
||||
}
|
||||
if (options.offset === undefined || options.offset === 0) {
|
||||
assert.deepStrictEqual(writeBufCopy, readBufCopy);
|
||||
}
|
||||
assert.deepStrictEqual(writeResult.buffer, readResult.buffer);
|
||||
} finally {
|
||||
await fh?.close();
|
||||
}
|
||||
}
|
||||
|
||||
(async () => {
|
||||
// Test if first argument is not wrongly interpreted as ArrayBufferView|string
|
||||
for (const badBuffer of [
|
||||
undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {},
|
||||
Promise.resolve(new Uint8Array(1)),
|
||||
{},
|
||||
{ buffer: 'amNotParam' },
|
||||
{ string: 'amNotParam' },
|
||||
{ buffer: new Uint8Array(1).buffer },
|
||||
new Date(),
|
||||
new String('notPrimitive'),
|
||||
{ toString() { return 'amObject'; } },
|
||||
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
|
||||
]) {
|
||||
await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', badBuffer, {});
|
||||
}
|
||||
|
||||
// First argument (buffer or string) is mandatory
|
||||
await testInvalid(dest, 'ERR_INVALID_ARG_TYPE');
|
||||
|
||||
// Various invalid options
|
||||
await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 5 });
|
||||
await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 });
|
||||
await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 });
|
||||
await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: -1 });
|
||||
await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 });
|
||||
await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false });
|
||||
await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true });
|
||||
|
||||
// Test compatibility with filehandle.read counterpart
|
||||
for (const options of [
|
||||
{},
|
||||
{ length: 1 },
|
||||
{ position: 5 },
|
||||
{ length: 1, position: 5 },
|
||||
{ length: 1, position: -1, offset: 2 },
|
||||
{ length: null },
|
||||
{ position: null },
|
||||
{ offset: 1 },
|
||||
]) {
|
||||
await testValid(dest, buffer, options);
|
||||
}
|
||||
})().then(common.mustCall());
|
|
@ -0,0 +1,102 @@
|
|||
'use strict';
|
||||
|
||||
const common = require('../common');
|
||||
|
||||
// This test ensures that fs.write accepts "named parameters" object
|
||||
// and doesn't interpret objects as strings
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const tmpdir = require('../common/tmpdir');
|
||||
const util = require('util');
|
||||
|
||||
tmpdir.refresh();
|
||||
|
||||
const destInvalid = path.resolve(tmpdir.path, 'rwopt_invalid');
|
||||
const buffer = Buffer.from('zyx');
|
||||
|
||||
function testInvalidCb(fd, expectedCode, buffer, options, callback) {
|
||||
assert.throws(
|
||||
() => fs.write(fd, buffer, options, common.mustNotCall()),
|
||||
{ code: expectedCode }
|
||||
);
|
||||
callback(0);
|
||||
}
|
||||
|
||||
function testValidCb(buffer, options, index, callback) {
|
||||
const dest = path.resolve(tmpdir.path, `rwopt_valid_${index}`);
|
||||
fs.open(dest, 'w+', common.mustSucceed((fd) => {
|
||||
fs.write(fd, buffer, options, common.mustSucceed((bytesWritten, bufferWritten) => {
|
||||
const writeBufCopy = Uint8Array.prototype.slice.call(bufferWritten);
|
||||
|
||||
fs.read(fd, buffer, options, common.mustSucceed((bytesRead, bufferRead) => {
|
||||
const readBufCopy = Uint8Array.prototype.slice.call(bufferRead);
|
||||
|
||||
assert.ok(bytesWritten >= bytesRead);
|
||||
if (options.length !== undefined && options.length !== null) {
|
||||
assert.strictEqual(bytesWritten, options.length);
|
||||
}
|
||||
if (options.offset === undefined || options.offset === 0) {
|
||||
assert.deepStrictEqual(writeBufCopy, readBufCopy);
|
||||
}
|
||||
assert.deepStrictEqual(bufferWritten, bufferRead);
|
||||
fs.close(fd, common.mustSucceed(callback));
|
||||
}));
|
||||
}));
|
||||
}));
|
||||
}
|
||||
|
||||
// Promisify to reduce flakiness
|
||||
const testInvalid = util.promisify(testInvalidCb);
|
||||
const testValid = util.promisify(testValidCb);
|
||||
|
||||
async function runTests(fd) {
|
||||
// Test if first argument is not wrongly interpreted as ArrayBufferView|string
|
||||
for (const badBuffer of [
|
||||
undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {},
|
||||
Promise.resolve(new Uint8Array(1)),
|
||||
{},
|
||||
{ buffer: 'amNotParam' },
|
||||
{ string: 'amNotParam' },
|
||||
{ buffer: new Uint8Array(1).buffer },
|
||||
new Date(),
|
||||
new String('notPrimitive'),
|
||||
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
|
||||
|
||||
// TODO(LiviaMedeiros): add the following after DEP0162 EOL
|
||||
// { toString() { return 'amObject'; } },
|
||||
]) {
|
||||
await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', badBuffer, {});
|
||||
}
|
||||
|
||||
// First argument (buffer or string) is mandatory
|
||||
await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', undefined, undefined);
|
||||
|
||||
// Various invalid options
|
||||
await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: 5 });
|
||||
await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 });
|
||||
await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 });
|
||||
await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: -1 });
|
||||
await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 });
|
||||
await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false });
|
||||
await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true });
|
||||
|
||||
// Test compatibility with fs.read counterpart
|
||||
for (const [ index, options ] of [
|
||||
{},
|
||||
{ length: 1 },
|
||||
{ position: 5 },
|
||||
{ length: 1, position: 5 },
|
||||
{ length: 1, position: -1, offset: 2 },
|
||||
{ length: null },
|
||||
{ position: null },
|
||||
{ offset: 1 },
|
||||
].entries()) {
|
||||
await testValid(buffer, options, index);
|
||||
}
|
||||
}
|
||||
|
||||
fs.open(destInvalid, 'w+', common.mustSucceed(async (fd) => {
|
||||
runTests(fd).then(common.mustCall(() => fs.close(fd, common.mustSucceed())));
|
||||
}));
|
|
@ -0,0 +1,89 @@
|
|||
'use strict';
|
||||
|
||||
require('../common');
|
||||
|
||||
// This test ensures that fs.writeSync accepts "named parameters" object
|
||||
// and doesn't interpret objects as strings
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const tmpdir = require('../common/tmpdir');
|
||||
|
||||
tmpdir.refresh();
|
||||
|
||||
const dest = path.resolve(tmpdir.path, 'tmp.txt');
|
||||
const buffer = Buffer.from('zyx');
|
||||
|
||||
function testInvalid(dest, expectedCode, ...bufferAndOptions) {
|
||||
let fd;
|
||||
try {
|
||||
fd = fs.openSync(dest, 'w+');
|
||||
assert.throws(
|
||||
() => fs.writeSync(fd, ...bufferAndOptions),
|
||||
{ code: expectedCode });
|
||||
} finally {
|
||||
if (fd != null) fs.closeSync(fd);
|
||||
}
|
||||
}
|
||||
|
||||
function testValid(dest, buffer, options) {
|
||||
let fd;
|
||||
try {
|
||||
fd = fs.openSync(dest, 'w+');
|
||||
const bytesWritten = fs.writeSync(fd, buffer, options);
|
||||
const bytesRead = fs.readSync(fd, buffer, options);
|
||||
|
||||
assert.ok(bytesWritten >= bytesRead);
|
||||
if (options.length !== undefined && options.length !== null) {
|
||||
assert.strictEqual(bytesWritten, options.length);
|
||||
}
|
||||
} finally {
|
||||
if (fd != null) fs.closeSync(fd);
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
// Test if second argument is not wrongly interpreted as string or options
|
||||
for (const badBuffer of [
|
||||
undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {},
|
||||
{},
|
||||
{ buffer: 'amNotParam' },
|
||||
{ string: 'amNotParam' },
|
||||
{ buffer: new Uint8Array(1) },
|
||||
{ buffer: new Uint8Array(1).buffer },
|
||||
Promise.resolve(new Uint8Array(1)),
|
||||
new Date(),
|
||||
new String('notPrimitive'),
|
||||
{ toString() { return 'amObject'; } },
|
||||
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
|
||||
]) {
|
||||
testInvalid(dest, 'ERR_INVALID_ARG_TYPE', badBuffer);
|
||||
}
|
||||
|
||||
// First argument (buffer or string) is mandatory
|
||||
testInvalid(dest, 'ERR_INVALID_ARG_TYPE');
|
||||
|
||||
// Various invalid options
|
||||
testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 5 });
|
||||
testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 });
|
||||
testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 });
|
||||
testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: -1 });
|
||||
testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 });
|
||||
testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false });
|
||||
testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true });
|
||||
|
||||
// Test compatibility with fs.readSync counterpart with reused options
|
||||
for (const options of [
|
||||
{},
|
||||
{ length: 1 },
|
||||
{ position: 5 },
|
||||
{ length: 1, position: 5 },
|
||||
{ length: 1, position: -1, offset: 2 },
|
||||
{ length: null },
|
||||
{ position: null },
|
||||
{ offset: 1 },
|
||||
]) {
|
||||
testValid(dest, buffer, options);
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue