fs: make writeFile consistent with readFile wrt fd

As it is, `readFile` always reads from the current position of the file,
if a file descriptor is used. But `writeFile` always writes from the
beginning of the file.

This patch fixes this inconsistency by making `writeFile` also to write
from the current position of the file when used with a file descriptor.

PR-URL: https://github.com/nodejs/node/pull/23709
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

lib/fs.js

@@ -1229,7 +1229,7 @@ function writeFile(path, data, options, callback) {
   function writeFd(fd, isUserFd) {
     const buffer = isArrayBufferView(data) ?
       data : Buffer.from('' + data, options.encoding || 'utf8');
-    const position = /a/.test(flag) ? null : 0;
+    const position = (/a/.test(flag) || isUserFd) ? null : 0;
 
     writeAll(fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
   }
@@ -1247,7 +1247,7 @@ function writeFileSync(path, data, options) {
   }
   let offset = 0;
   let length = data.byteLength;
-  let position = /a/.test(flag) ? null : 0;
+  let position = (/a/.test(flag) || isUserFd) ? null : 0;
   try {
     while (length > 0) {
       const written = fs.writeSync(fd, data, offset, length, position);

test/parallel/test-fs-promises-readfile-with-fd.js

@@ -0,0 +1,35 @@
+'use strict';
+
+/*
+ * This test makes sure that `readFile()` always reads from the current
+ * position of the file, instead of reading from the beginning of the file.
+ */
+
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+const { writeFileSync } = require('fs');
+const { open } = require('fs').promises;
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const fn = path.join(tmpdir.path, 'test.txt');
+writeFileSync(fn, 'Hello World');
+
+async function readFileTest() {
+  const handle = await open(fn, 'r');
+
+  /* Read only five bytes, so that the position moves to five. */
+  const buf = Buffer.alloc(5);
+  const { bytesRead } = await handle.read(buf, 0, 5, null);
+  assert.strictEqual(bytesRead, 5);
+  assert.deepStrictEqual(buf.toString(), 'Hello');
+
+  /* readFile() should read from position five, instead of zero. */
+  assert.deepStrictEqual((await handle.readFile()).toString(), ' World');
+}
+
+
+readFileTest()
+  .then(common.mustCall());

test/parallel/test-fs-promises-writefile-with-fd.js

@@ -0,0 +1,36 @@
+'use strict';
+
+/*
+ * This test makes sure that `writeFile()` always writes from the current
+ * position of the file, instead of truncating the file.
+ */
+
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+const { readFileSync } = require('fs');
+const { open } = require('fs').promises;
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const fn = path.join(tmpdir.path, 'test.txt');
+
+async function writeFileTest() {
+  const handle = await open(fn, 'w');
+
+  /* Write only five bytes, so that the position moves to five. */
+  const buf = Buffer.from('Hello');
+  const { bytesWritten } = await handle.write(buf, 0, 5, null);
+  assert.strictEqual(bytesWritten, 5);
+
+  /* Write some more with writeFile(). */
+  await handle.writeFile('World');
+
+  /* New content should be written at position five, instead of zero. */
+  assert.deepStrictEqual(readFileSync(fn).toString(), 'HelloWorld');
+}
+
+
+writeFileTest()
+  .then(common.mustCall());

test/parallel/test-fs-readfile-fd.js

@@ -1,5 +1,5 @@
 'use strict';
-require('../common');
+const common = require('../common');
 
 // Test fs.readFile using a file descriptor.
 
@@ -7,6 +7,9 @@ const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const fs = require('fs');
 const fn = fixtures.path('empty.txt');
+const join = require('path').join;
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
 
 tempFd(function(fd, close) {
   fs.readFile(fd, function(err, data) {
@@ -46,3 +49,48 @@ function tempFdSync(callback) {
   callback(fd);
   fs.closeSync(fd);
 }
+
+{
+  /*
+   * This test makes sure that `readFile()` always reads from the current
+   * position of the file, instead of reading from the beginning of the file,
+   * when used with file descriptors.
+   */
+
+  const filename = join(tmpdir.path, 'test.txt');
+  fs.writeFileSync(filename, 'Hello World');
+
+  {
+    /* Tests the fs.readFileSync(). */
+    const fd = fs.openSync(filename, 'r');
+
+    /* Read only five bytes, so that the position moves to five. */
+    const buf = Buffer.alloc(5);
+    assert.deepStrictEqual(fs.readSync(fd, buf, 0, 5), 5);
+    assert.deepStrictEqual(buf.toString(), 'Hello');
+
+    /* readFileSync() should read from position five, instead of zero. */
+    assert.deepStrictEqual(fs.readFileSync(fd).toString(), ' World');
+  }
+
+  {
+    /* Tests the fs.readFile(). */
+    fs.open(filename, 'r', common.mustCall((err, fd) => {
+      assert.ifError(err);
+      const buf = Buffer.alloc(5);
+
+      /* Read only five bytes, so that the position moves to five. */
+      fs.read(fd, buf, 0, 5, null, common.mustCall((err, bytes) => {
+        assert.ifError(err);
+        assert.strictEqual(bytes, 5);
+        assert.deepStrictEqual(buf.toString(), 'Hello');
+
+        fs.readFile(fd, common.mustCall((err, data) => {
+          assert.ifError(err);
+          /* readFile() should read from position five, instead of zero. */
+          assert.deepStrictEqual(data.toString(), ' World');
+        }));
+      }));
+    }));
+  }
+}

test/parallel/test-fs-writefile-with-fd.js

@@ -0,0 +1,58 @@
+'use strict';
+
+/*
+ * This test makes sure that `writeFile()` always writes from the current
+ * position of the file, instead of truncating the file, when used with file
+ * descriptors.
+ */
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const join = require('path').join;
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+{
+  /* writeFileSync() test. */
+  const filename = join(tmpdir.path, 'test.txt');
+
+  /* Open the file descriptor. */
+  const fd = fs.openSync(filename, 'w');
+
+  /* Write only five characters, so that the position moves to five. */
+  assert.deepStrictEqual(fs.writeSync(fd, 'Hello'), 5);
+  assert.deepStrictEqual(fs.readFileSync(filename).toString(), 'Hello');
+
+  /* Write some more with writeFileSync(). */
+  fs.writeFileSync(fd, 'World');
+
+  /* New content should be written at position five, instead of zero. */
+  assert.deepStrictEqual(fs.readFileSync(filename).toString(), 'HelloWorld');
+}
+
+{
+  /* writeFile() test. */
+  const file = join(tmpdir.path, 'test1.txt');
+
+  /* Open the file descriptor. */
+  fs.open(file, 'w', common.mustCall((err, fd) => {
+    assert.ifError(err);
+
+    /* Write only five characters, so that the position moves to five. */
+    fs.write(fd, 'Hello', common.mustCall((err, bytes) => {
+      assert.ifError(err);
+      assert.strictEqual(bytes, 5);
+      assert.deepStrictEqual(fs.readFileSync(file).toString(), 'Hello');
+
+      /* Write some more with writeFile(). */
+      fs.writeFile(fd, 'World', common.mustCall((err) => {
+        assert.ifError(err);
+
+        /* New content should be written at position five, instead of zero. */
+        assert.deepStrictEqual(fs.readFileSync(file).toString(), 'HelloWorld');
+      }));
+    }));
+  }));
+}