fs: support Uint8Array input to methods

Allow `fs.read`, `fs.write` and `fs.writeFile` to take `Uint8Array` arguments. PR-URL: nodejs#10382 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
addaleax · Jan 3, 2017 · abde764 · abde764
1 parent d1843ec
commit abde764
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 28 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1281,7 +1281,7 @@ added: v0.0.2
 -->
 
 * `fd` {Integer}
-* `buffer` {String | Buffer}
+* `buffer` {String | Buffer | Uint8Array}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}
@@ -1427,7 +1427,7 @@ added: v0.1.21
 -->
 
 * `fd` {Integer}
-* `buffer` {String | Buffer}
+* `buffer` {String | Buffer | Uint8Array}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}
@@ -1824,7 +1824,7 @@ added: v0.0.2
 -->
 
 * `fd` {Integer}
-* `buffer` {Buffer}
+* `buffer` {Buffer | Uint8Array}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}
@@ -1891,7 +1891,7 @@ added: v0.1.29
 -->
 
 * `file` {String | Buffer | Integer} filename or file descriptor
-* `data` {String | Buffer}
+* `data` {String | Buffer | Uint8Array}
 * `options` {Object | String}
   * `encoding` {String | Null} default = `'utf8'`
   * `mode` {Integer} default = `0o666`
@@ -1934,7 +1934,7 @@ added: v0.1.29
 -->
 
 * `file` {String | Buffer | Integer} filename or file descriptor
-* `data` {String | Buffer}
+* `data` {String | Buffer | Uint8Array}
 * `options` {Object | String}
   * `encoding` {String | Null} default = `'utf8'`
   * `mode` {Integer} default = `0o666`
@@ -1948,7 +1948,7 @@ added: v0.1.21
 -->
 
 * `fd` {Integer}
-* `buffer` {Buffer}
+* `buffer` {Buffer | Uint8Array}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}

diff --git a/lib/fs.js b/lib/fs.js
@@ -6,6 +6,7 @@
 const constants = process.binding('constants').fs;
 const util = require('util');
 const pathModule = require('path');
+const { isUint8Array } = process.binding('util');
 
 const binding = process.binding('fs');
 const fs = exports;
@@ -559,7 +560,7 @@ fs.openSync = function(path, flags, mode) {
 
 var readWarned = false;
 fs.read = function(fd, buffer, offset, length, position, callback) {
-  if (!(buffer instanceof Buffer)) {
+  if (!isUint8Array(buffer)) {
     // legacy string interface (fd, length, position, encoding, callback)
     if (!readWarned) {
       readWarned = true;
@@ -623,7 +624,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {
   var legacy = false;
   var encoding;
 
-  if (!(buffer instanceof Buffer)) {
+  if (!isUint8Array(buffer)) {
     // legacy string interface (fd, length, position, encoding, callback)
     if (!readSyncWarned) {
       readSyncWarned = true;
@@ -674,7 +675,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
   var req = new FSReqWrap();
   req.oncomplete = wrapper;
 
-  if (buffer instanceof Buffer) {
+  if (isUint8Array(buffer)) {
     callback = maybeCallback(callback || position || length || offset);
     if (typeof offset !== 'number') {
       offset = 0;
@@ -708,7 +709,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
 // OR
 //  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
-  if (buffer instanceof Buffer) {
+  if (isUint8Array(buffer)) {
     if (position === undefined)
       position = null;
     if (typeof offset !== 'number')
@@ -1206,7 +1207,7 @@ fs.writeFile = function(path, data, options, callback) {
   });
 
   function writeFd(fd, isUserFd) {
-    var buffer = (data instanceof Buffer) ?
+    var buffer = isUint8Array(data) ?
         data : Buffer.from('' + data, options.encoding || 'utf8');
     var position = /a/.test(flag) ? null : 0;
 
@@ -1221,7 +1222,7 @@ fs.writeFileSync = function(path, data, options) {
   var isUserFd = isFd(path); // file descriptor ownership
   var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
 
-  if (!(data instanceof Buffer)) {
+  if (!isUint8Array(data)) {
     data = Buffer.from('' + data, options.encoding || 'utf8');
   }
   var offset = 0;

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -29,7 +29,8 @@ using v8::Value;
   V(isSet, IsSet)                                                             \
   V(isSetIterator, IsSetIterator)                                             \
   V(isSharedArrayBuffer, IsSharedArrayBuffer)                                 \
-  V(isTypedArray, IsTypedArray)
+  V(isTypedArray, IsTypedArray)                                               \
+  V(isUint8Array, IsUint8Array)
 
 
 #define V(_, ucname) \

diff --git a/test/parallel/test-fs-read-buffer.js b/test/parallel/test-fs-read-buffer.js
@@ -6,20 +6,29 @@ const Buffer = require('buffer').Buffer;
 const fs = require('fs');
 const filepath = path.join(common.fixturesDir, 'x.txt');
 const fd = fs.openSync(filepath, 'r');
-const expected = 'xyz\n';
-const bufferAsync = Buffer.allocUnsafe(expected.length);
-const bufferSync = Buffer.allocUnsafe(expected.length);
 
-fs.read(fd,
-        bufferAsync,
-        0,
-        expected.length,
-        0,
-        common.mustCall(function(err, bytesRead) {
-          assert.equal(bytesRead, expected.length);
-          assert.deepStrictEqual(bufferAsync, Buffer.from(expected));
-        }));
+const expected = Buffer.from('xyz\n');
 
-var r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
-assert.deepStrictEqual(bufferSync, Buffer.from(expected));
-assert.equal(r, expected.length);
+function test(bufferAsync, bufferSync, expected) {
+  fs.read(fd,
+          bufferAsync,
+          0,
+          expected.length,
+          0,
+          common.mustCall((err, bytesRead) => {
+            assert.strictEqual(bytesRead, expected.length);
+            assert.deepStrictEqual(bufferAsync, Buffer.from(expected));
+          }));
+
+  const r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
+  assert.deepStrictEqual(bufferSync, Buffer.from(expected));
+  assert.strictEqual(r, expected.length);
+}
+
+test(Buffer.allocUnsafe(expected.length),
+     Buffer.allocUnsafe(expected.length),
+     expected);
+
+test(new Uint8Array(expected.length),
+     new Uint8Array(expected.length),
+     Uint8Array.from(expected));
diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js
@@ -106,3 +106,23 @@ common.refreshTmpDir();
     fs.write(fd, expected, undefined, undefined, cb);
   }));
 }
+
+// fs.write with a Uint8Array, without the offset and length parameters:
+{
+  const filename = path.join(common.tmpDir, 'write6.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(expected.length, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual(expected.toString(), found);
+    });
+
+    fs.write(fd, Uint8Array.from(expected), cb);
+  }));
+}
diff --git a/test/parallel/test-fs-write-file-uint8array.js b/test/parallel/test-fs-write-file-uint8array.js
@@ -0,0 +1,28 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const join = require('path').join;
+
+common.refreshTmpDir();
+
+const filename = join(common.tmpDir, 'test.txt');
+
+const s = '南越国是前203年至前111年存在于岭南地区的一个国家，国都位于番禺，疆域包括今天中国的广东、' +
+          '广西两省区的大部份地区，福建省、湖南、贵州、云南的一小部份地区和越南的北部。' +
+          '南越国是秦朝灭亡后，由南海郡尉赵佗于前203年起兵兼并桂林郡和象郡后建立。' +
+          '前196年和前179年，南越国曾先后两次名义上臣属于西汉，成为西汉的“外臣”。前112年，' +
+          '南越国末代君主赵建德与西汉发生战争，被汉武帝于前111年所灭。南越国共存在93年，' +
+          '历经五代君主。南越国是岭南地区的第一个有记载的政权国家，采用封建制和郡县制并存的制度，' +
+          '它的建立保证了秦末乱世岭南地区社会秩序的稳定，有效的改善了岭南地区落后的政治、##济现状。\n';
+
+const input = Uint8Array.from(Buffer.from(s, 'utf8'));
+
+fs.writeFileSync(filename, input);
+assert.strictEqual(fs.readFileSync(filename, 'utf8'), s);
+
+fs.writeFile(filename, input, common.mustCall((e) => {
+  assert.ifError(e);
+
+  assert.strictEqual(fs.readFileSync(filename, 'utf8'), s);
+}));