From 301482b4db9eed784d42f1e11f514c8b56380432 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Fri, 28 Sep 2018 03:48:42 +0530 Subject: [PATCH] fs: make sure that file is truncated in writeFile 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: https://github.com/nodejs/node/issues/22554 --- lib/fs.js | 22 +++++- lib/internal/fs/promises.js | 10 ++- test/parallel/test-fs-writefile-truncating.js | 76 +++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-fs-writefile-truncating.js diff --git a/lib/fs.js b/lib/fs.js index 3302cfe0bf756a..424c8f976ba4d3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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); + } } } @@ -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); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 6cae7d1a21ff8d..3bab7e1fc83c4e 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -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)); } diff --git a/test/parallel/test-fs-writefile-truncating.js b/test/parallel/test-fs-writefile-truncating.js new file mode 100644 index 00000000000000..1907b176a04e79 --- /dev/null +++ b/test/parallel/test-fs-writefile-truncating.js @@ -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); + })); + })); + })); +}