fs: validate the input data before opening file

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]>
nodejs · Feb 13, 2020 · 3e9302b · 3e9302b
1 parent a751389
commit 3e9302b
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 9 deletions.
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -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 {
@@ -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);
 

diff --git a/test/parallel/test-fs-append-file.js b/test/parallel/test-fs-append-file.js
@@ -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).