fs: update read to work with any TypedArray/DataView

Sarat Addepalli · targos · commit 1006e6ba05cb · 2018-08-28T22:30:35.000+02:00
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>
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -2360,6 +2360,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`.
@@ -2369,7 +2373,7 @@ changes:
 -->
 
 * `fd` {integer}
-* `buffer` {Buffer|Uint8Array}
+* `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -2639,13 +2643,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}
@@ -3369,6 +3377,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
@@ -3386,14 +3398,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`.
 
@@ -3468,6 +3480,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
@@ -3485,7 +3501,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`
@@ -3501,7 +3517,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!');
 });
@@ -3526,6 +3543,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`.
@@ -3535,7 +3556,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`
@@ -3550,6 +3571,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`.
@@ -3559,7 +3584,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);
@@ -1171,11 +1171,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);
   }
 }
 
@@ -1186,11 +1186,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/test/parallel/test-fs-write-file-typedarrays.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'));
+    }));
+  }));
+}

Original file line number	Diff line number	Diff line change
`@@ -15,8 +15,8 @@ assert.throws(`
`15`	`15`	`{`
`16`	`16`	`code: 'ERR_INVALID_ARG_TYPE',`
`17`	`17`	`name: 'TypeError [ERR_INVALID_ARG_TYPE]',`
`18`		`- message: 'The "buffer" argument must be one of type Buffer or Uint8Array.' +`
`19`		`- ' Received type number'`
	`18`	`+ message: 'The "buffer" argument must be one of type Buffer, TypedArray, ' +`
	`19`	`+ 'or DataView. Received type number'`
`20`	`20`	`}`
`21`	`21`	`);`
`22`	`22`
`@@ -70,8 +70,8 @@ assert.throws(`
`70`	`70`	`{`
`71`	`71`	`code: 'ERR_INVALID_ARG_TYPE',`
`72`	`72`	`name: 'TypeError [ERR_INVALID_ARG_TYPE]',`
`73`		`- message: 'The "buffer" argument must be one of type Buffer or Uint8Array.' +`
`74`		`- ' Received type number'`
	`73`	`+ message: 'The "buffer" argument must be one of type Buffer, TypedArray, ' +`
	`74`	`+ 'or DataView. Received type number'`
`75`	`75`	`}`
`76`	`76`	`);`
`77`	`77`