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: make params in writing methods optional #42601

Merged
merged 1 commit into from
May 17, 2022
Merged
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
67 changes: 63 additions & 4 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
37 changes: 29 additions & 8 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down Expand Up @@ -705,7 +705,7 @@ function readSync(fd, buffer, offset, length, position) {
({
offset = 0,
length = buffer.byteLength - offset,
position = null
position = null,
} = options);
}

Expand Down Expand Up @@ -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 {(
Expand All @@ -811,16 +811,26 @@ 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);
}

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 {
Expand Down Expand Up @@ -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
LiviaMedeiros marked this conversation as resolved.
Show resolved Hide resolved
} = offsetOrOptions ?? ObjectCreate(null));
}
if (position === undefined)
position = null;
if (offset == null) {
Expand Down
11 changes: 10 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
LiviaMedeiros marked this conversation as resolved.
Show resolved Hide resolved
} = offsetOrOptions ?? ObjectCreate(null));
}

if (offset == null) {
offset = 0;
} else {
Expand Down
95 changes: 95 additions & 0 deletions test/parallel/test-fs-promises-write-optional-params.js
Original file line number Diff line number Diff line change
@@ -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());
Loading