Skip to content

Commit 14d652f

Browse files
robhoganfacebook-github-bot
authored andcommitted
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
1 parent 183aba4 commit 14d652f

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

packages/metro-file-map/src/lib/TreeFS.js

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ export default class TreeFS implements MutableFileSystem {
458458
subtreeOnly: boolean,
459459
}>,
460460
pathPrefix: string = '',
461+
followedLinks: $ReadOnlySet<FileMetaData> = new Set(),
461462
): Iterable<Path> {
462463
const pathSep = opts.alwaysYieldPosix ? '/' : path.sep;
463464
const prefixWithSep = pathPrefix === '' ? pathPrefix : pathPrefix + pathSep;
@@ -498,17 +499,27 @@ export default class TreeFS implements MutableFileSystem {
498499
// Symlink goes nowhere, nothing to report.
499500
continue;
500501
}
501-
if (!(resolved.node instanceof Map)) {
502+
const target = resolved.node;
503+
if (!(target instanceof Map)) {
502504
// Symlink points to a file, just yield the path of the symlink.
503505
yield nodePath;
504-
} else if (opts.recursive && opts.follow) {
506+
} else if (
507+
opts.recursive &&
508+
opts.follow &&
509+
!followedLinks.has(node)
510+
) {
505511
// Symlink points to a directory - iterate over its contents using
506512
// the path where we found the symlink as a prefix.
507-
yield* this._pathIterator(resolved.node, opts, nodePath);
513+
yield* this._pathIterator(
514+
target,
515+
opts,
516+
nodePath,
517+
new Set([...followedLinks, node]),
518+
);
508519
}
509520
}
510521
} else if (opts.recursive) {
511-
yield* this._pathIterator(node, opts, nodePath);
522+
yield* this._pathIterator(node, opts, nodePath, followedLinks);
512523
}
513524
}
514525
}

packages/metro-file-map/src/lib/__tests__/TreeFS-test.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,13 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
3232
rootDir: p('/project'),
3333
files: new Map([
3434
[p('foo/another.js'), ['', 123, 0, 0, '', '', 0]],
35+
[p('foo/owndir'), ['', 0, 0, 0, '', '', '.']],
3536
[p('foo/link-to-bar.js'), ['', 0, 0, 0, '', '', p('../bar.js')]],
3637
[p('foo/link-to-another.js'), ['', 0, 0, 0, '', '', p('another.js')]],
3738
[p('../outside/external.js'), ['', 0, 0, 0, '', '', 0]],
3839
[p('bar.js'), ['', 234, 0, 0, '', '', 0]],
39-
[p('link-to-foo'), ['', 456, 0, 0, '', '', p('./foo')]],
40+
[p('link-to-foo'), ['', 456, 0, 0, '', '', p('././abnormal/../foo')]],
41+
[p('abs-link-out'), ['', 456, 0, 0, '', '', p('/outside/./baz/..')]],
4042
[p('root'), ['', 0, 0, 0, '', '', '..']],
4143
[p('link-to-nowhere'), ['', 123, 0, 0, '', '', p('./nowhere')]],
4244
[p('link-to-self'), ['', 123, 0, 0, '', '', p('./link-to-self')]],
@@ -129,10 +131,12 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
129131
[p('link-to-self'), ['', 123, 0, 0, '', '', 0]],
130132
]),
131133
removedFiles: new Set([
134+
p('foo/owndir'),
132135
p('foo/link-to-bar.js'),
133136
p('foo/link-to-another.js'),
134137
p('../outside/external.js'),
135138
p('bar.js'),
139+
p('abs-link-out'),
136140
p('root'),
137141
]),
138142
});
@@ -162,6 +166,9 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
162166
}),
163167
).toEqual([
164168
p('/project/foo/another.js'),
169+
p('/project/foo/owndir/another.js'),
170+
p('/project/foo/owndir/link-to-bar.js'),
171+
p('/project/foo/owndir/link-to-another.js'),
165172
p('/project/foo/link-to-bar.js'),
166173
p('/project/foo/link-to-another.js'),
167174
]);
@@ -186,12 +193,19 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
186193
}),
187194
).toEqual([
188195
p('/project/foo/another.js'),
196+
p('/project/foo/owndir/another.js'),
197+
p('/project/foo/owndir/link-to-bar.js'),
198+
p('/project/foo/owndir/link-to-another.js'),
189199
p('/project/foo/link-to-bar.js'),
190200
p('/project/foo/link-to-another.js'),
191201
p('/project/bar.js'),
192202
p('/project/link-to-foo/another.js'),
203+
p('/project/link-to-foo/owndir/another.js'),
204+
p('/project/link-to-foo/owndir/link-to-bar.js'),
205+
p('/project/link-to-foo/owndir/link-to-another.js'),
193206
p('/project/link-to-foo/link-to-bar.js'),
194207
p('/project/link-to-foo/link-to-another.js'),
208+
p('/project/abs-link-out/external.js'),
195209
p('/project/root/outside/external.js'),
196210
]);
197211
});
@@ -220,7 +234,9 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
220234
}),
221235
).toEqual([
222236
p('/project/foo/another.js'),
237+
p('/project/foo/owndir/another.js'),
223238
p('/project/link-to-foo/another.js'),
239+
p('/project/link-to-foo/owndir/another.js'),
224240
]);
225241
});
226242
});

0 commit comments

Comments
 (0)