Skip to content

Commit

Permalink
fs: make writeFile consistent with readFile wrt fd
Browse files Browse the repository at this point in the history
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: #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>
  • Loading branch information
thefourtheye authored and Trott committed Dec 15, 2018
1 parent 2c5dae5 commit 8f4b924
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 3 deletions.
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
35 changes: 35 additions & 0 deletions test/parallel/test-fs-promises-readfile-with-fd.js
Original file line number Diff line number Diff line change
@@ -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());
36 changes: 36 additions & 0 deletions test/parallel/test-fs-promises-writefile-with-fd.js
Original file line number Diff line number Diff line change
@@ -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());
50 changes: 49 additions & 1 deletion test/parallel/test-fs-readfile-fd.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
'use strict';
require('../common');
const common = require('../common');

// Test fs.readFile using a file descriptor.

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) {
Expand Down Expand Up @@ -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');
}));
}));
}));
}
}
58 changes: 58 additions & 0 deletions test/parallel/test-fs-writefile-with-fd.js
Original file line number Diff line number Diff line change
@@ -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');
}));
}));
}));
}

0 comments on commit 8f4b924

Please sign in to comment.