Skip to content

Commit

Permalink
fs: fix edge case in readFileSync utf8 fast path
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
richardlau authored and marco-ippolito committed May 3, 2024
1 parent b33eff8 commit 0a9910c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-fs-read-file-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
}

0 comments on commit 0a9910c

Please sign in to comment.