Skip to content

Commit

Permalink
Gracefully handle symlinks to ., .., etc in require.context
Browse files Browse the repository at this point in the history
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
  • Loading branch information
robhogan authored and facebook-github-bot committed Jun 24, 2023
1 parent 183aba4 commit 14d652f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
19 changes: 15 additions & 4 deletions packages/metro-file-map/src/lib/TreeFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ export default class TreeFS implements MutableFileSystem {
subtreeOnly: boolean,
}>,
pathPrefix: string = '',
followedLinks: $ReadOnlySet<FileMetaData> = new Set(),
): Iterable<Path> {
const pathSep = opts.alwaysYieldPosix ? '/' : path.sep;
const prefixWithSep = pathPrefix === '' ? pathPrefix : pathPrefix + pathSep;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
18 changes: 17 additions & 1 deletion packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')]],
Expand Down Expand Up @@ -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'),
]),
});
Expand Down Expand Up @@ -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'),
]);
Expand All @@ -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'),
]);
});
Expand Down Expand Up @@ -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'),
]);
});
});
Expand Down

0 comments on commit 14d652f

Please sign in to comment.