fs: fix edge case in readFileSync utf8 fast path

richardlau · marco-ippolito · commit 0a9910c2c1c8 · 2024-05-03T08:30:15.000+02:00
Fix a file permissions regression when `fs.readFileSync()` is called in append mode on a file that does not already exist introduced by the fast path for utf8 encoding. PR-URL: #52101 Fixes: #52079 Refs: #49691 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -2499,7 +2499,7 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
     if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
 
     FS_SYNC_TRACE_BEGIN(open);
-    file = uv_fs_open(nullptr, &req, *path, flags, O_RDONLY, nullptr);
+    file = uv_fs_open(nullptr, &req, *path, flags, 0666, nullptr);
     FS_SYNC_TRACE_END(open);
     if (req.result < 0) {
       uv_fs_req_cleanup(&req);
diff --git a/test/parallel/test-fs-read-file-sync.js b/test/parallel/test-fs-read-file-sync.js
@@ -24,11 +24,37 @@ require('../common');
 const assert = require('assert');
 const fs = require('fs');
 const fixtures = require('../common/fixtures');
+const tmpdir = require('../common/tmpdir');
 
 const fn = fixtures.path('elipses.txt');
+tmpdir.refresh();
 
 const s = fs.readFileSync(fn, 'utf8');
 for (let i = 0; i < s.length; i++) {
   assert.strictEqual(s[i], '\u2026');
 }
 assert.strictEqual(s.length, 10000);
+
+// Test file permissions set for readFileSync() in append mode.
+{
+  const expectedMode = 0o666 & ~process.umask();
+
+  for (const test of [
+    { },
+    { encoding: 'ascii' },
+    { encoding: 'base64' },
+    { encoding: 'hex' },
+    { encoding: 'latin1' },
+    { encoding: 'uTf8' }, // case variation
+    { encoding: 'utf16le' },
+    { encoding: 'utf8' },
+  ]) {
+    const opts = { ...test, flag: 'a+' };
+    const file = tmpdir.resolve(`testReadFileSyncAppend${opts.encoding ?? ''}.txt`);
+    const variant = `for '${file}'`;
+
+    const content = fs.readFileSync(file, opts);
+    assert.strictEqual(opts.encoding ? content : content.toString(), '', `file contents ${variant}`);
+    assert.strictEqual(fs.statSync(file).mode & 0o777, expectedMode, `file permissions ${variant}`);
+  }
+}
