Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: protect fs.watch from monkeypatches #45265

Merged
merged 2 commits into from
Nov 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2299,7 +2299,7 @@ function watch(filename, options, listener) {

let watcher;

// TODO(anonrig): Remove this when/if libuv supports it.
// TODO(anonrig): Remove non-native watcher when/if libuv supports recursive.
// As of November 2022, libuv does not support recursive file watch on all platforms,
// e.g. Linux due to the limitations of inotify.
if (options.recursive && !isOSX && !isWindows) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ async function* _watch(filename, options = kEmptyObject) {
if (options.recursive != null) {
validateBoolean(options.recursive, 'options.recursive');

// TODO(anonrig): Remove this when/if libuv supports it.
// TODO(anonrig): Remove non-native watcher when/if libuv supports recursive.
// As of November 2022, libuv does not support recursive file watch on all platforms,
// e.g. Linux due to the limitations of inotify.
if (options.recursive && !isOSX && !isWindows) {
Expand Down
23 changes: 14 additions & 9 deletions lib/internal/fs/recursive_watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ const { getValidatedPath } = require('internal/fs/utils');
const { kFSWatchStart, StatWatcher } = require('internal/fs/watchers');
const { kEmptyObject } = require('internal/util');
const { validateBoolean, validateAbortSignal } = require('internal/validators');
const path = require('path');
const {
basename: pathBasename,
join: pathJoin,
relative: pathRelative,
resolve: pathResolve,
} = require('path');

let internalSync;
let internalPromises;
Expand All @@ -45,7 +50,7 @@ async function traverse(dir, files = new SafeMap(), symbolicLinks = new SafeSet(
const subdirectories = [];

for await (const file of filenames) {
const f = path.join(dir, file.name);
const f = pathJoin(dir, file.name);

files.set(f, file);

Expand All @@ -67,7 +72,7 @@ class FSWatcher extends EventEmitter {
#closed = false;
#files = new SafeMap();
#symbolicFiles = new SafeSet();
#rootPath = path.resolve();
#rootPath = pathResolve();
#watchingFile = false;

constructor(options = kEmptyObject) {
Expand Down Expand Up @@ -140,10 +145,10 @@ class FSWatcher extends EventEmitter {
break;
}

const f = path.join(folder, file.name);
const f = pathJoin(folder, file.name);

if (!this.#files.has(f)) {
this.emit('change', 'rename', path.relative(this.#rootPath, f));
this.emit('change', 'rename', pathRelative(this.#rootPath, f));

if (file.isSymbolicLink()) {
this.#symbolicFiles.add(f);
Expand Down Expand Up @@ -186,23 +191,23 @@ class FSWatcher extends EventEmitter {
if (currentStats.birthtimeMs === 0 && previousStats.birthtimeMs !== 0) {
// The file is now deleted
this.#files.delete(file);
this.emit('change', 'rename', path.relative(this.#rootPath, file));
this.emit('change', 'rename', pathRelative(this.#rootPath, file));
this.#unwatchFiles(file);
} else if (file === this.#rootPath && this.#watchingFile) {
// This case will only be triggered when watching a file with fs.watch
this.emit('change', 'change', path.basename(file));
this.emit('change', 'change', pathBasename(file));
} else if (this.#symbolicFiles.has(file)) {
// Stats from watchFile does not return correct value for currentStats.isSymbolicLink()
// Since it is only valid when using fs.lstat(). Therefore, check the existing symbolic files.
this.emit('change', 'rename', path.relative(this.#rootPath, file));
this.emit('change', 'rename', pathRelative(this.#rootPath, file));
} else if (currentStats.isDirectory()) {
this.#watchFolder(file);
}
});
}

[kFSWatchStart](filename) {
filename = path.resolve(getValidatedPath(filename));
filename = pathResolve(getValidatedPath(filename));

try {
const file = lazyLoadFsSync().statSync(filename);
Expand Down