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: work with any TypedArrays instead of only Uint8Arrays #22150

Closed
wants to merge 16 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
41 changes: 33 additions & 8 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,10 @@ this API: [`fs.open()`][].
<!-- YAML
added: v0.0.2
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `buffer` parameter can now be any `TypedArray`, or a
`DataView`.
- version: v7.4.0
pr-url: https://github.com/nodejs/node/pull/10382
description: The `buffer` parameter can now be a `Uint8Array`.
Expand All @@ -2354,7 +2358,7 @@ changes:
-->

* `fd` {integer}
* `buffer` {Buffer|Uint8Array}
* `buffer` {Buffer|TypedArray|DataView}
* `offset` {integer}
* `length` {integer}
* `position` {integer}
Expand Down Expand Up @@ -2624,13 +2628,17 @@ the link path returned will be passed as a `Buffer` object.
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `buffer` parameter can now be any `TypedArray` or a
`DataView`.
- version: v6.0.0
pr-url: https://github.com/nodejs/node/pull/4518
description: The `length` parameter can now be `0`.
-->

* `fd` {integer}
* `buffer` {Buffer|Uint8Array}
* `buffer` {Buffer|TypedArray|DataView}
* `offset` {integer}
* `length` {integer}
* `position` {integer}
Expand Down Expand Up @@ -3354,6 +3362,10 @@ This happens when:
<!-- YAML
added: v0.0.2
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `buffer` parameter can now be any `TypedArray` or a
`DataView`
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/12562
description: The `callback` parameter is no longer optional. Not passing
Expand All @@ -3371,14 +3383,14 @@ changes:
-->

* `fd` {integer}
* `buffer` {Buffer|Uint8Array}
* `buffer` {Buffer|TypedArray|DataView}
* `offset` {integer}
* `length` {integer}
* `position` {integer}
* `callback` {Function}
* `err` {Error}
* `bytesWritten` {integer}
* `buffer` {Buffer|Uint8Array}
* `buffer` {Buffer|TypedArray|DataView}

Write `buffer` to the file specified by `fd`.

Expand Down Expand Up @@ -3453,6 +3465,10 @@ the end of the file.
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `data` parameter can now be any `TypedArray` or a
`DataView`.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/12562
description: The `callback` parameter is no longer optional. Not passing
Expand All @@ -3470,7 +3486,7 @@ changes:
-->

* `file` {string|Buffer|URL|integer} filename or file descriptor
* `data` {string|Buffer|Uint8Array}
* `data` {string|Buffer|TypedArray|DataView}
* `options` {Object|string}
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
Expand All @@ -3486,7 +3502,8 @@ The `encoding` option is ignored if `data` is a buffer.
Example:

```js
fs.writeFile('message.txt', 'Hello Node.js', (err) => {
const data = new Uint8Array(Buffer.from('Hello Node.js'));
fs.writeFile('message.txt', data, (err) => {
if (err) throw err;
console.log('The file has been saved!');
});
Expand All @@ -3511,6 +3528,10 @@ automatically.
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `data` parameter can now be any `TypedArray` or a
`DataView`.
- version: v7.4.0
pr-url: https://github.com/nodejs/node/pull/10382
description: The `data` parameter can now be a `Uint8Array`.
Expand All @@ -3520,7 +3541,7 @@ changes:
-->

* `file` {string|Buffer|URL|integer} filename or file descriptor
* `data` {string|Buffer|Uint8Array}
* `data` {string|Buffer|TypedArray|DataView}
* `options` {Object|string}
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
Expand All @@ -3535,6 +3556,10 @@ this API: [`fs.writeFile()`][].
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22150
description: The `buffer` parameter can now be any `TypedArray` or a
`DataView`.
- version: v7.4.0
pr-url: https://github.com/nodejs/node/pull/10382
description: The `buffer` parameter can now be a `Uint8Array`.
Expand All @@ -3544,7 +3569,7 @@ changes:
-->

* `fd` {integer}
* `buffer` {Buffer|Uint8Array}
* `buffer` {Buffer|TypedArray|DataView}
* `offset` {integer}
* `length` {integer}
* `position` {integer}
Expand Down
24 changes: 12 additions & 12 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const {

const { _extend } = require('util');
const pathModule = require('path');
const { isUint8Array } = require('internal/util/types');
const { isArrayBufferView } = require('internal/util/types');
const binding = process.binding('fs');
const { Buffer, kMaxLength } = require('buffer');
const errors = require('internal/errors');
Expand Down Expand Up @@ -458,12 +458,12 @@ function read(fd, buffer, offset, length, position, callback) {
});
}

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 (!Number.isSafeInteger(position))
position = -1;
Expand Down Expand Up @@ -493,12 +493,12 @@ function readSync(fd, buffer, offset, length, position) {
return 0;
}

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 (!Number.isSafeInteger(position))
position = -1;
Expand All @@ -525,7 +525,7 @@ function write(fd, buffer, offset, length, position, callback) {
const req = new FSReqCallback();
req.oncomplete = wrapper;

if (isUint8Array(buffer)) {
if (isArrayBufferView(buffer)) {
callback = maybeCallback(callback || position || length || offset);
if (typeof offset !== 'number')
offset = 0;
Expand Down Expand Up @@ -563,13 +563,13 @@ function writeSync(fd, buffer, offset, length, position) {
validateUint32(fd, 'fd');
const ctx = {};
let result;
if (isUint8Array(buffer)) {
if (isArrayBufferView(buffer)) {
if (position === undefined)
position = null;
if (typeof offset !== 'number')
offset = 0;
if (typeof length !== 'number')
length = buffer.length - offset;
length = buffer.byteLength - offset;
validateOffsetLengthWrite(offset, length, buffer.byteLength);
result = binding.writeBuffer(fd, buffer, offset, length, position,
undefined, ctx);
Expand Down Expand Up @@ -1189,11 +1189,11 @@ function writeFile(path, data, options, callback) {
});

function writeFd(fd, isUserFd) {
const buffer = isUint8Array(data) ?
const buffer = isArrayBufferView(data) ?
data : Buffer.from('' + data, options.encoding || 'utf8');
const position = /a/.test(flag) ? null : 0;

writeAll(fd, isUserFd, buffer, 0, buffer.length, position, callback);
writeAll(fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
}
}

Expand All @@ -1204,11 +1204,11 @@ function writeFileSync(path, data, options) {
const isUserFd = isFd(path); // file descriptor ownership
const fd = isUserFd ? path : fs.openSync(path, flag, options.mode);

if (!isUint8Array(data)) {
if (!isArrayBufferView(data)) {
data = Buffer.from('' + data, options.encoding || 'utf8');
}
let offset = 0;
let length = data.length;
let length = data.byteLength;
let position = /a/.test(flag) ? null : 0;
try {
while (length > 0) {
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const {
ERR_INVALID_OPT_VALUE_ENCODING,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
const { isUint8Array } = require('internal/util/types');
const { isUint8Array, isArrayBufferView } = require('internal/util/types');
const pathModule = require('path');
const util = require('util');
const kType = Symbol('type');
Expand Down Expand Up @@ -394,9 +394,10 @@ function toUnixTimestamp(time, name = 'time') {
}

function validateBuffer(buffer) {
if (!isUint8Array(buffer)) {
if (!isArrayBufferView(buffer)) {
const err = new ERR_INVALID_ARG_TYPE('buffer',
['Buffer', 'Uint8Array'], buffer);
['Buffer', 'TypedArray', 'DataView'],
buffer);
Error.captureStackTrace(err, validateBuffer);
throw err;
}
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-fs-read-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ assert.throws(
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError [ERR_INVALID_ARG_TYPE]',
message: 'The "buffer" argument must be one of type Buffer or Uint8Array.' +
' Received type number'
message: 'The "buffer" argument must be one of type Buffer, TypedArray, ' +
'or DataView. Received type number'
}
);

Expand Down Expand Up @@ -70,8 +70,8 @@ assert.throws(
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError [ERR_INVALID_ARG_TYPE]',
message: 'The "buffer" argument must be one of type Buffer or Uint8Array.' +
' Received type number'
message: 'The "buffer" argument must be one of type Buffer, TypedArray, ' +
'or DataView. Received type number'
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,27 @@ const s = '南越国是前203年至前111年存在于岭南地区的一个国家
'历经五代君主。南越国是岭南地区的第一个有记载的政权国家,采用封建制和郡县制并存的制度,' +
'它的建立保证了秦末乱世岭南地区社会秩序的稳定,有效的改善了岭南地区落后的政治、##济现状。\n';

const input = Uint8Array.from(Buffer.from(s, 'utf8'));
// The length of the buffer should be a multiple of 8
// as required by common.getArrayBufferViews()
const inputBuffer = Buffer.from(s.repeat(8), 'utf8');

fs.writeFileSync(filename, input);
assert.strictEqual(fs.readFileSync(filename, 'utf8'), s);
for (const expectView of common.getArrayBufferViews(inputBuffer)) {
console.log('Sync test for ', expectView[Symbol.toStringTag]);
fs.writeFileSync(filename, expectView);
assert.strictEqual(
fs.readFileSync(filename, 'utf8'),
inputBuffer.toString('utf8')
);
}

fs.writeFile(filename, input, common.mustCall((e) => {
assert.ifError(e);
for (const expectView of common.getArrayBufferViews(inputBuffer)) {
console.log('Async test for ', expectView[Symbol.toStringTag]);
fs.writeFile(filename, expectView, common.mustCall((e) => {
assert.ifError(e);

assert.strictEqual(fs.readFileSync(filename, 'utf8'), s);
}));
fs.readFile(filename, 'utf8', common.mustCall((err, data) => {
assert.ifError(err);
assert.strictEqual(data, inputBuffer.toString('utf8'));
}));
}));
}