Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: make writeFile consistent with readFile wrt fd #23709

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,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;
thefourtheye marked this conversation as resolved.
Show resolved Hide resolved
const position = (/a/.test(flag) || isUserFd) ? null : 0;

writeAll(fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
}
Expand All @@ -1209,7 +1209,7 @@ function writeFileSync(path, data, options) {
}
let offset = 0;
let length = data.byteLength;
let position = /a/.test(flag) ? null : 0;
thefourtheye marked this conversation as resolved.
Show resolved Hide resolved
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');
}));
}));
}));
}