fs: Fix default params for fs.write(Sync)

papandreou · MylesBorins · commit 968e5649a3a7 · 2017-10-16T17:02:35.000-04:00
Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb) as documented at https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback and equivalently for fs.writeSync Update docs and code comments to reflect the implementation. Backport-PR-URL: #13179 PR-URL: #7856 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1876,13 +1876,13 @@ you need to compare `curr.mtime` and `prev.mtime`.
 `fs.unwatchFile`. `fs.watch` should be used instead of `fs.watchFile` and
 `fs.unwatchFile` when possible.
 
-## fs.write(fd, buffer, offset, length[, position], callback)
+## fs.write(fd, buffer[, offset[, length[, position]]], callback)
 <!-- YAML
 added: v0.0.2
 -->
 
 * `fd` {integer}
-* `buffer` {string | Buffer}
+* `buffer` {Buffer}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -1908,19 +1908,19 @@ On Linux, positional writes don't work when the file is opened in append mode.
 The kernel ignores the position argument and always appends the data to
 the end of the file.
 
-## fs.write(fd, data[, position[, encoding]], callback)
+## fs.write(fd, string[, position[, encoding]], callback)
 <!-- YAML
 added: v0.11.5
 -->
 
 * `fd` {integer}
-* `data` {string | Buffer}
+* `string` {string}
 * `position` {integer}
 * `encoding` {string}
 * `callback` {Function}
 
-Write `data` to the file specified by `fd`.  If `data` is not a Buffer instance
-then the value will be coerced to a string.
+Write `string` to the file specified by `fd`.  If `string` is not a string, then
+the value will be coerced to one.
 
 `position` refers to the offset from the beginning of the file where this data
 should be written. If `typeof position !== 'number'` the data will be written at
@@ -2001,24 +2001,24 @@ added: v0.1.29
 
 The synchronous version of [`fs.writeFile()`][]. Returns `undefined`.
 
-## fs.writeSync(fd, buffer, offset, length[, position])
+## fs.writeSync(fd, buffer[, offset[, length[, position]]])
 <!-- YAML
 added: v0.1.21
 -->
 
 * `fd` {integer}
-* `buffer` {string | Buffer}
+* `buffer` {Buffer}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
 
-## fs.writeSync(fd, data[, position[, encoding]])
+## fs.writeSync(fd, string[, position[, encoding]])
 <!-- YAML
 added: v0.11.5
 -->
 
 * `fd` {integer}
-* `data` {string | Buffer}
+* `string` {string}
 * `position` {integer}
 * `encoding` {string}
 
diff --git a/lib/fs.js b/lib/fs.js
@@ -740,7 +740,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {
 };
 
 // usage:
-//  fs.write(fd, buffer, offset, length[, position], callback);
+//  fs.write(fd, buffer[, offset[, length[, position]]], callback);
 // OR
 //  fs.write(fd, string[, position[, encoding]], callback);
 fs.write = function(fd, buffer, offset, length, position, callback) {
@@ -753,12 +753,16 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
   req.oncomplete = wrapper;
 
   if (buffer instanceof Buffer) {
-    // if no position is passed then assume null
-    if (typeof position === 'function') {
-      callback = position;
+    callback = maybeCallback(callback || position || length || offset);
+    if (typeof offset !== 'number') {
+      offset = 0;
+    }
+    if (typeof length !== 'number') {
+      length = buffer.length - offset;
+    }
+    if (typeof position !== 'number') {
       position = null;
     }
-    callback = maybeCallback(callback);
     return binding.writeBuffer(fd, buffer, offset, length, position, req);
   }
 
@@ -778,13 +782,17 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
 };
 
 // usage:
-//  fs.writeSync(fd, buffer, offset, length[, position]);
+//  fs.writeSync(fd, buffer[, offset[, length[, position]]]);
 // OR
 //  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
   if (buffer instanceof Buffer) {
     if (position === undefined)
       position = null;
+    if (typeof offset !== 'number')
+      offset = 0;
+    if (typeof length !== 'number')
+      length = buffer.length - offset;
     return binding.writeBuffer(fd, buffer, offset, length, position);
   }
   if (typeof buffer !== 'string')
diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js
@@ -2,29 +2,107 @@
 const common = require('../common');
 const assert = require('assert');
 const path = require('path');
-const Buffer = require('buffer').Buffer;
 const fs = require('fs');
-const filename = path.join(common.tmpDir, 'write.txt');
 const expected = Buffer.from('hello');
 
 common.refreshTmpDir();
 
-fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
-  if (err) throw err;
-
-  fs.write(fd,
-           expected,
-           0,
-           expected.length,
-           null,
-           common.mustCall(function(err, written) {
-             if (err) throw err;
-
-             assert.strictEqual(expected.length, written);
-             fs.closeSync(fd);
-
-             const found = fs.readFileSync(filename, 'utf8');
-             assert.deepStrictEqual(expected.toString(), found);
-             fs.unlinkSync(filename);
-           }));
-}));
+// fs.write with all parameters provided:
+{
+  const filename = path.join(common.tmpDir, 'write1.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, expected, 0, expected.length, null, cb);
+  }));
+}
+
+// fs.write with a buffer, without the length parameter:
+{
+  const filename = path.join(common.tmpDir, 'write2.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(2, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual('lo', found);
+    });
+
+    fs.write(fd, Buffer.from('hello'), 3, cb);
+  }));
+}
+
+// fs.write with a buffer, without the offset and length parameters:
+{
+  const filename = path.join(common.tmpDir, 'write3.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, expected, cb);
+  }));
+}
+
+// fs.write with the offset passed as undefined followed by the callback:
+{
+  const filename = path.join(common.tmpDir, 'write4.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, expected, undefined, cb);
+  }));
+}
+
+// fs.write with offset and length passed as undefined followed by the callback:
+{
+  const filename = path.join(common.tmpDir, 'write5.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, expected, undefined, undefined, cb);
+  }));
+}
diff --git a/test/parallel/test-fs-write-sync.js b/test/parallel/test-fs-write-sync.js
@@ -3,21 +3,54 @@ const common = require('../common');
 const assert = require('assert');
 const path = require('path');
 const fs = require('fs');
-const fn = path.join(common.tmpDir, 'write.txt');
+const filename = path.join(common.tmpDir, 'write.txt');
 
 common.refreshTmpDir();
 
-const foo = 'foo';
-const fd = fs.openSync(fn, 'w');
+// fs.writeSync with all parameters provided:
+{
+  const fd = fs.openSync(filename, 'w');
 
-let written = fs.writeSync(fd, '');
-assert.strictEqual(0, written);
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
 
-fs.writeSync(fd, foo);
+  fs.writeSync(fd, 'foo');
 
-const bar = 'bár';
-written = fs.writeSync(fd, Buffer.from(bar), 0, Buffer.byteLength(bar));
-assert.ok(written > 3);
-fs.closeSync(fd);
+  written = fs.writeSync(fd, Buffer.from('bár'), 0, Buffer.byteLength('bár'));
+  assert.ok(written > 3);
+  fs.closeSync(fd);
 
-assert.strictEqual(fs.readFileSync(fn, 'utf8'), 'foobár');
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}
+
+// fs.writeSync with a buffer, without the length parameter:
+{
+  const fd = fs.openSync(filename, 'w');
+
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
+
+  fs.writeSync(fd, 'foo');
+
+  written = fs.writeSync(fd, Buffer.from('bár'), 0);
+  assert.ok(written > 3);
+  fs.closeSync(fd);
+
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}
+
+// fs.writeSync with a buffer, without the offset and length parameters:
+{
+  const fd = fs.openSync(filename, 'w');
+
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
+
+  fs.writeSync(fd, 'foo');
+
+  written = fs.writeSync(fd, Buffer.from('bár'));
+  assert.ok(written > 3);
+  fs.closeSync(fd);
+
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}
