Skip to content

Commit

Permalink
fs: make sure that file is truncated in writeFile
Browse files Browse the repository at this point in the history
Simple Reproduction of the Bug

```js
'use strict';

const assert = require('assert');
const fs = require('fs');

const fileName = 'test.txt';
fs.writeFileSync(fileName, '123');
fs.writeFileSync(fileName, '0');
const result1 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

const fd = fs.openSync(fileName, 'w');
fs.writeFileSync(fd, '123');
fs.writeFileSync(fd, '0');
fs.closeSync(fd);
const result2 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

assert.deepStrictEqual(result2, result1);
// + expected - actual
//
//- '023'
//+ '0'

```

Fixes: nodejs#22554
  • Loading branch information
thefourtheye committed Sep 30, 2018
1 parent aba7677 commit 301482b
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 4 deletions.
22 changes: 19 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1191,9 +1191,20 @@ 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 isAppendMode = /a/.test(flag);
const position = isAppendMode ? null : 0;

writeAll(fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
if (isUserFd && !isAppendMode) {
ftruncate(fd, (err) => {
if (err) {
return callback(err);
}
writeAll(
fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
});
} else {
writeAll(fd, isUserFd, buffer, 0, buffer.byteLength, position, callback);
}
}
}

Expand All @@ -1203,13 +1214,18 @@ function writeFileSync(path, data, options) {

const isUserFd = isFd(path); // file descriptor ownership
const fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
const isAppendMode = /a/.test(flag);

if (isUserFd && !isAppendMode) {
ftruncateSync(fd);
}

if (!isArrayBufferView(data)) {
data = Buffer.from('' + data, options.encoding || 'utf8');
}
let offset = 0;
let length = data.byteLength;
let position = /a/.test(flag) ? null : 0;
let position = isAppendMode ? null : 0;
try {
while (length > 0) {
const written = fs.writeSync(fd, data, offset, length, position);
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,11 +464,19 @@ async function mkdtemp(prefix, options) {
async function writeFile(path, data, options) {
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
const flag = options.flag || 'w';
const isAppendMode = /a/.test(flag);

if (path instanceof FileHandle)
if (path instanceof FileHandle) {
if (!isAppendMode) {
await path.truncate();
}
return writeFileHandle(path, data, options);
}

const fd = await open(path, flag, options.mode);
if (!isAppendMode) {
await fd.truncate();
}
return writeFileHandle(fd, data, options).finally(fd.close.bind(fd));
}

Expand Down
76 changes: 76 additions & 0 deletions test/parallel/test-fs-writefile-truncating.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const fsPromises = require('fs').promises;
const tmpdir = require('../common/tmpdir');
const tmpDir = tmpdir.path;
tmpdir.refresh();

// Length of First should be greater than the length of Second. When
// the file is written the second time, it should clear out all the
// contents of the file instead of starting at the end of the file or
// at the beginning of the file replacing characters in the existing
// content.
const First = 'Callbacks';
const Second = 'Promises';

{
const fileName = path.resolve(tmpDir, 'first.txt');
fs.writeFileSync(fileName, First);
fs.writeFileSync(fileName, Second);
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
}

{
const fileName = path.resolve(tmpDir, 'second.txt');
const fd = fs.openSync(fileName, 'w');
fs.writeFileSync(fd, First);
fs.writeFileSync(fd, Second);
fs.closeSync(fd);
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
}

(async function() {
const fileName = path.resolve(tmpDir, 'third.txt');
await fsPromises.writeFile(fileName, First);
await fsPromises.writeFile(fileName, Second);
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
})().then(common.mustCall());

(async function() {
const fileName = path.resolve(tmpDir, 'fourth.txt');
const fileHandle = await fsPromises.open(fileName, 'w');
await fileHandle.writeFile(First);
await fileHandle.writeFile(Second);
await fileHandle.close();
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
})().then(common.mustCall());

{
const fileName = path.resolve(tmpDir, 'fifth.txt');
fs.writeFile(fileName, First, common.mustCall(function(err) {
assert.ifError(err);
fs.writeFile(fileName, Second, common.mustCall(function(err) {
assert.ifError(err);
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
}));
}));
}

{
const fileName = path.resolve(tmpDir, 'sixth.txt');
fs.open(fileName, 'w', common.mustCall(function(err, handle) {
assert.ifError(err);
fs.writeFile(handle, First, common.mustCall(function(err) {
assert.ifError(err);
fs.writeFile(handle, Second, common.mustCall(function(err) {
assert.ifError(err);
fs.closeSync(handle);
assert.deepStrictEqual(fs.readFileSync(fileName, 'utf8'), Second);
}));
}));
}));
}

0 comments on commit 301482b

Please sign in to comment.