From 14d652f411fd055c950f820d2de3087e2a791acd Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 24 Jun 2023 13:40:37 -0700 Subject: [PATCH] Gracefully handle symlinks to ., .., etc in `require.context` Summary: `matchFilesWithContext` powers `require.context`, returning all paths under a given root matching a given `regex`. This is originally an established [Webpack feature](https://webpack.js.org/guides/dependency-management/#requirecontext), and we aim to match behaviour as closely as we can. Webpack *does* follow symlinks when enumerating paths prior to matching, so that for example in the following file layout: ``` - link-to-foo -> foo - foo - - bar.js ``` `link-to-foo/bar.js` is enumerated, so that `require.context(myRoot, true, /link-to-foo\/bar.js/)` matches. Webpack's implementation of this uses recursive `fs.readdir` calls, following symlinks, to enumerate all reachable paths. This raises the question of handling a kind of cycle, for example: ``` - foo -> . - bar.js ``` Would theoretically match `./bar.js`, `./foo/bar.js`, `./foo/foo/bar.js` and so on. Webpack actually dies on this, throwing `ELOOP` from `fs`. Similarly, currently, Metro will crash with call stack size exceeded inside `_pathIterator`. This diff makes the (arguably arbitrary, but reasonablish) choice to follow symlinks at most once per path, by tracking "seen" links in a set during enumeration. ``` - foo -> . - baz -> . - bar.js ``` Expands to: ``` bar.js foo/bar.js foo/baz/bar.js baz/bar.js baz/foo/bar.js ``` Changelog: ``` - **[Experimental]**: Fix crash when `require.context` is used on a directory with infinite path expansions. Reviewed By: motiz88 Differential Revision: D46767544 fbshipit-source-id: ed3d17b3afcf27189aa442627a31b6225fd873bf --- packages/metro-file-map/src/lib/TreeFS.js | 19 +++++++++++++++---- .../src/lib/__tests__/TreeFS-test.js | 18 +++++++++++++++++- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js index 2f225532f0..6fdf43c367 100644 --- a/packages/metro-file-map/src/lib/TreeFS.js +++ b/packages/metro-file-map/src/lib/TreeFS.js @@ -458,6 +458,7 @@ export default class TreeFS implements MutableFileSystem { subtreeOnly: boolean, }>, pathPrefix: string = '', + followedLinks: $ReadOnlySet = new Set(), ): Iterable { const pathSep = opts.alwaysYieldPosix ? '/' : path.sep; const prefixWithSep = pathPrefix === '' ? pathPrefix : pathPrefix + pathSep; @@ -498,17 +499,27 @@ export default class TreeFS implements MutableFileSystem { // Symlink goes nowhere, nothing to report. continue; } - if (!(resolved.node instanceof Map)) { + const target = resolved.node; + if (!(target instanceof Map)) { // Symlink points to a file, just yield the path of the symlink. yield nodePath; - } else if (opts.recursive && opts.follow) { + } else if ( + opts.recursive && + opts.follow && + !followedLinks.has(node) + ) { // Symlink points to a directory - iterate over its contents using // the path where we found the symlink as a prefix. - yield* this._pathIterator(resolved.node, opts, nodePath); + yield* this._pathIterator( + target, + opts, + nodePath, + new Set([...followedLinks, node]), + ); } } } else if (opts.recursive) { - yield* this._pathIterator(node, opts, nodePath); + yield* this._pathIterator(node, opts, nodePath, followedLinks); } } } diff --git a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js index 83ca6790cf..3805c3a944 100644 --- a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js +++ b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js @@ -32,11 +32,13 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { rootDir: p('/project'), files: new Map([ [p('foo/another.js'), ['', 123, 0, 0, '', '', 0]], + [p('foo/owndir'), ['', 0, 0, 0, '', '', '.']], [p('foo/link-to-bar.js'), ['', 0, 0, 0, '', '', p('../bar.js')]], [p('foo/link-to-another.js'), ['', 0, 0, 0, '', '', p('another.js')]], [p('../outside/external.js'), ['', 0, 0, 0, '', '', 0]], [p('bar.js'), ['', 234, 0, 0, '', '', 0]], - [p('link-to-foo'), ['', 456, 0, 0, '', '', p('./foo')]], + [p('link-to-foo'), ['', 456, 0, 0, '', '', p('././abnormal/../foo')]], + [p('abs-link-out'), ['', 456, 0, 0, '', '', p('/outside/./baz/..')]], [p('root'), ['', 0, 0, 0, '', '', '..']], [p('link-to-nowhere'), ['', 123, 0, 0, '', '', p('./nowhere')]], [p('link-to-self'), ['', 123, 0, 0, '', '', p('./link-to-self')]], @@ -129,10 +131,12 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { [p('link-to-self'), ['', 123, 0, 0, '', '', 0]], ]), removedFiles: new Set([ + p('foo/owndir'), p('foo/link-to-bar.js'), p('foo/link-to-another.js'), p('../outside/external.js'), p('bar.js'), + p('abs-link-out'), p('root'), ]), }); @@ -162,6 +166,9 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { }), ).toEqual([ p('/project/foo/another.js'), + p('/project/foo/owndir/another.js'), + p('/project/foo/owndir/link-to-bar.js'), + p('/project/foo/owndir/link-to-another.js'), p('/project/foo/link-to-bar.js'), p('/project/foo/link-to-another.js'), ]); @@ -186,12 +193,19 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { }), ).toEqual([ p('/project/foo/another.js'), + p('/project/foo/owndir/another.js'), + p('/project/foo/owndir/link-to-bar.js'), + p('/project/foo/owndir/link-to-another.js'), p('/project/foo/link-to-bar.js'), p('/project/foo/link-to-another.js'), p('/project/bar.js'), p('/project/link-to-foo/another.js'), + p('/project/link-to-foo/owndir/another.js'), + p('/project/link-to-foo/owndir/link-to-bar.js'), + p('/project/link-to-foo/owndir/link-to-another.js'), p('/project/link-to-foo/link-to-bar.js'), p('/project/link-to-foo/link-to-another.js'), + p('/project/abs-link-out/external.js'), p('/project/root/outside/external.js'), ]); }); @@ -220,7 +234,9 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { }), ).toEqual([ p('/project/foo/another.js'), + p('/project/foo/owndir/another.js'), p('/project/link-to-foo/another.js'), + p('/project/link-to-foo/owndir/another.js'), ]); }); });