fs: update read to work with any TypedArray/DataView

PR-URL: #22150 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Sep 6, 2018 · baf7dc5 · baf7dc5
1 parent dfbbfb7
commit baf7dc5
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 32 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -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`.
@@ -2354,7 +2358,7 @@ changes:
 -->
 
 * `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.
 <!-- 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}
@@ -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
@@ -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`.
 
@@ -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
@@ -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`
@@ -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.
 <!-- 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`.
@@ -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`
@@ -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`.
@@ -3544,7 +3569,7 @@ changes:
 -->
 
 * `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
@@ -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
@@ -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
@@ -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/...parallel/test-fs-write-file-uint8array.js → ...arallel/test-fs-write-file-typedarrays.js b/...parallel/test-fs-write-file-uint8array.js → ...arallel/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'));
+    }));
+  }));
+}