From baf7dc5fc132161908dda6c8c818297c50c8c554 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 6 Aug 2018 15:25:59 +0530 Subject: [PATCH] fs: update read to work with any TypedArray/DataView PR-URL: https://github.com/nodejs/node/pull/22150 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- doc/api/fs.md | 41 +++++++++++++++---- lib/fs.js | 20 ++++----- lib/internal/fs/utils.js | 7 ++-- test/parallel/test-fs-read-type.js | 8 ++-- ...y.js => test-fs-write-file-typedarrays.js} | 28 +++++++++---- 5 files changed, 72 insertions(+), 32 deletions(-) rename test/parallel/{test-fs-write-file-uint8array.js => test-fs-write-file-typedarrays.js} (59%) diff --git a/doc/api/fs.md b/doc/api/fs.md index b481e94c29..62697bdb16 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -2345,6 +2345,10 @@ this API: [`fs.open()`][]. * `fd` {integer} -* `buffer` {Buffer|Uint8Array} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -2624,13 +2628,17 @@ the link path returned will be passed as a `Buffer` object. * `fd` {integer} -* `buffer` {Buffer|Uint8Array} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -3354,6 +3362,10 @@ This happens when: * `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`. @@ -3453,6 +3465,10 @@ the end of the file. * `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` @@ -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!'); }); @@ -3511,6 +3528,10 @@ automatically. * `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` @@ -3535,6 +3556,10 @@ this API: [`fs.writeFile()`][]. * `fd` {integer} -* `buffer` {Buffer|Uint8Array} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} * `position` {integer} diff --git a/lib/fs.js b/lib/fs.js index 4a9476049d..0031a2b085 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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'); @@ -450,7 +450,7 @@ function read(fd, buffer, offset, length, position, callback) { }); } - validateOffsetLengthRead(offset, length, buffer.length); + validateOffsetLengthRead(offset, length, buffer.byteLength); if (!Number.isSafeInteger(position)) position = -1; @@ -480,7 +480,7 @@ function readSync(fd, buffer, offset, length, position) { return 0; } - validateOffsetLengthRead(offset, length, buffer.length); + validateOffsetLengthRead(offset, length, buffer.byteLength); if (!Number.isSafeInteger(position)) position = -1; @@ -507,7 +507,7 @@ function write(fd, buffer, offset, length, position, callback) { const req = new FSReqWrap(); req.oncomplete = wrapper; - if (isUint8Array(buffer)) { + if (isArrayBufferView(buffer)) { callback = maybeCallback(callback || position || length || offset); if (typeof offset !== 'number') offset = 0; @@ -545,13 +545,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); @@ -1152,11 +1152,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); } } @@ -1167,11 +1167,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) { diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index befb060473..c4a426996c 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -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'); @@ -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; } diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js index 2980bca7d2..d75464ce0a 100644 --- a/test/parallel/test-fs-read-type.js +++ b/test/parallel/test-fs-read-type.js @@ -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' } ); @@ -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' } ); diff --git a/test/parallel/test-fs-write-file-uint8array.js b/test/parallel/test-fs-write-file-typedarrays.js similarity index 59% rename from test/parallel/test-fs-write-file-uint8array.js rename to test/parallel/test-fs-write-file-typedarrays.js index 592bdb0581..e9bd9fe9a7 100644 --- a/test/parallel/test-fs-write-file-uint8array.js +++ b/test/parallel/test-fs-write-file-typedarrays.js @@ -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')); + })); + })); +}