Skip to content

Commit

Permalink
fs: validate the input data before opening file
Browse files Browse the repository at this point in the history
PR-URL: #31731
Refs: #31030
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
  • Loading branch information
ZYSzys committed Feb 13, 2020
1 parent a751389 commit 3e9302b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
9 changes: 5 additions & 4 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,6 @@ function validateFileHandle(handle) {
}

async function writeFileHandle(filehandle, data, options) {
if (!isArrayBufferView(data)) {
validateStringAfterArrayBufferView(data, 'data');
data = Buffer.from(data, options.encoding || 'utf8');
}
let remaining = data.length;
if (remaining === 0) return;
do {
Expand Down Expand Up @@ -496,6 +492,11 @@ async function writeFile(path, data, options) {
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
const flag = options.flag || 'w';

if (!isArrayBufferView(data)) {
validateStringAfterArrayBufferView(data, 'data');
data = Buffer.from(data, options.encoding || 'utf8');
}

if (path instanceof FileHandle)
return writeFileHandle(path, data, options);

Expand Down
28 changes: 23 additions & 5 deletions test/parallel/test-fs-append-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,18 +129,36 @@ const throwNextTick = (e) => { process.nextTick(() => { throw e; }); };
.catch(throwNextTick);
}

// Test that appendFile does not accept numbers (callback API).
[false, 5, {}, [], null, undefined].forEach((data) => {
// Test that appendFile does not accept invalid data type (callback API).
[false, 5, {}, [], null, undefined].forEach(async (data) => {
const errObj = {
code: 'ERR_INVALID_ARG_TYPE',
message: /"data"|"buffer"/
};
const filename = join(tmpdir.path, 'append-invalid-data.txt');

assert.throws(
() => fs.appendFile(filename, data, common.mustNotCall()),
errObj
);

assert.throws(
() => fs.appendFile('foobar', data, common.mustNotCall()),
() => fs.appendFileSync(filename, data),
errObj
);
assert.throws(() => fs.appendFileSync('foobar', data), errObj);
assert.rejects(fs.promises.appendFile('foobar', data), errObj);

await assert.rejects(
fs.promises.appendFile(filename, data),
errObj
);
// The filename shouldn't exist if throwing error.
assert.throws(
() => fs.statSync(filename),
{
code: 'ENOENT',
message: /no such file or directory/
}
);
});

// Test that appendFile accepts file descriptors (callback API).
Expand Down

0 comments on commit 3e9302b

Please sign in to comment.