Skip to content

Commit 65fd022

Browse files
robhoganfacebook-github-bot
authored andcommitted
Fix handling of paths that leave and re-enter the project root via symlinks (#1280)
Summary: Pull Request resolved: #1280 ## Background The internal implementation of `TreeFS` uses a tree of maps representing file path segments, with a "root" node at the *project root*. For paths outside the project root, we traverse through `'..'` nodes, so that `../outside` traverses from the project root through `'..'` and `'outside'`. Basing the representation around the project root (as opposed to a volume root) minimises traversal through irrelevant paths and makes the cache portable. ## Problem However, because this map of maps is a simple (acyclic) tree, a `'..'` node has no entry for one of its logical (=on disk) children - the node closer to the project root. With a project root `/foo/bar`, the project-root-relative path `../bar` cannot be traversed, because `'..'` is a key of `'bar'` and not vice-versa. This mostly isn't a problem because `'../bar'` is not a *normal* path - normalisation collapses this down to `''`: https://github.com/facebook/metro/blob/6856d00cfdddc1dd5ed9ab35249fe7ca1610ca75/packages/metro-file-map/src/lib/RootPathUtils.js#L142-L148 ## Observable bug For lookups, if instead of a literal indirection we have a symlink `'link-to-parent/bar'`, normalisation cannot (and should not) collapse away the symlink - so after dereferencing the symlink during traversal to `'..'` and joining it with the remaining `bar` we are unable to lookup `'../bar'`. For the module resolver this might appear as a failed existence check, and cause a resolution failure. ## This fix When dereferencing a symlink as part of `_lookupByNormalPath`, we now join the symlink target to the remaining subpath using a (mostly pre-exisitng) utility function that is aware of the project root, and can collapse away the project root segments. This leaves a normalised, root-relative target path that's guaranteed not to leave and re-enter the project root. Changelog: ``` - **[Fix]**: Fix some paths being unresolvable when traversing a symlink that points to an ancestor of the project root. ``` Reviewed By: huntie Differential Revision: D57719224 fbshipit-source-id: e6ceee1b4a5f8d4fd1ea5c478c73e20f771b98a9
1 parent e303578 commit 65fd022

3 files changed

Lines changed: 63 additions & 8 deletions

File tree

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ import * as path from 'path';
3434
* back to `node:path` equivalents in those cases.
3535
*/
3636

37-
const UP_FRAGMENT = '..' + path.sep;
38-
const UP_FRAGMENT_LENGTH = UP_FRAGMENT.length;
37+
const UP_FRAGMENT_SEP = '..' + path.sep;
38+
const SEP_UP_FRAGMENT = path.sep + '..';
39+
const UP_FRAGMENT_SEP_LENGTH = UP_FRAGMENT_SEP.length;
3940
const CURRENT_FRAGMENT = '.' + path.sep;
4041

4142
export class RootPathUtils {
@@ -66,6 +67,10 @@ export class RootPathUtils {
6667
}
6768
}
6869

70+
getBasenameOfNthAncestor(n: number): string {
71+
return this.#rootParts[this.#rootParts.length - 1 - n];
72+
}
73+
6974
// absolutePath may be any well-formed absolute path.
7075
absoluteToNormal(absolutePath: string): string {
7176
let endOfMatchingPrefix = 0;
@@ -114,11 +119,11 @@ export class RootPathUtils {
114119
let i = 0;
115120
let pos = 0;
116121
while (
117-
normalPath.startsWith(UP_FRAGMENT, pos) ||
122+
normalPath.startsWith(UP_FRAGMENT_SEP, pos) ||
118123
(normalPath.endsWith('..') && normalPath.length === 2 + pos)
119124
) {
120125
left = this.#rootDirnames[i === this.#rootDepth ? this.#rootDepth : ++i];
121-
pos += UP_FRAGMENT_LENGTH;
126+
pos += UP_FRAGMENT_SEP_LENGTH;
122127
}
123128
const right = pos === 0 ? normalPath : normalPath.slice(pos);
124129
if (right.length === 0) {
@@ -139,6 +144,22 @@ export class RootPathUtils {
139144
);
140145
}
141146

147+
// Takes a normal and relative path, and joins them efficiently into a normal
148+
// path, including collapsing trailing '..' in the first part with leading
149+
// project root segments in the relative part.
150+
joinNormalToRelative(normalPath: string, relativePath: string): string {
151+
if (normalPath === '') {
152+
return relativePath;
153+
}
154+
if (relativePath === '') {
155+
return normalPath;
156+
}
157+
if (normalPath === '..' || normalPath.endsWith(SEP_UP_FRAGMENT)) {
158+
return this.relativeToNormal(normalPath + path.sep + relativePath);
159+
}
160+
return normalPath + path.sep + relativePath;
161+
}
162+
142163
// Internal: Tries to collapse sequences like `../root/foo` for root
143164
// `/project/root` down to the normal 'foo'.
144165
#tryCollapseIndirectionsInSuffix(
@@ -151,7 +172,7 @@ export class RootPathUtils {
151172
// unmatched suffix e.g /project/[../../foo], but bail out to Node's
152173
// path.relative if we find a possible indirection after any later segment,
153174
// or on any "./" that isn't a "../".
154-
for (let pos = startOfRelativePart; ; pos += UP_FRAGMENT_LENGTH) {
175+
for (let pos = startOfRelativePart; ; pos += UP_FRAGMENT_SEP_LENGTH) {
155176
const nextIndirection = fullPath.indexOf(CURRENT_FRAGMENT, pos);
156177
if (nextIndirection === -1) {
157178
// If we have any indirections, they may "collapse" if a subsequent
@@ -185,13 +206,13 @@ export class RootPathUtils {
185206
) {
186207
// If we have no right side (or an indirection that would take us
187208
// below the root), just ensure we don't include a trailing separtor.
188-
return UP_FRAGMENT.repeat(totalUpIndirections).slice(0, -1);
209+
return UP_FRAGMENT_SEP.repeat(totalUpIndirections).slice(0, -1);
189210
}
190211
// Optimisation for the common case, saves a concatenation.
191212
if (totalUpIndirections === 0) {
192213
return right;
193214
}
194-
return UP_FRAGMENT.repeat(totalUpIndirections) + right;
215+
return UP_FRAGMENT_SEP.repeat(totalUpIndirections) + right;
195216
}
196217

197218
// Cap the number of indirections at the total number of root segments.

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,11 @@ export default class TreeFS implements MutableFileSystem {
469469
// with our new target.
470470
targetNormalPath = isLastSegment
471471
? normalSymlinkTarget
472-
: normalSymlinkTarget + path.sep + targetNormalPath.slice(fromIdx);
472+
: this.#pathUtils.joinNormalToRelative(
473+
normalSymlinkTarget,
474+
targetNormalPath.slice(fromIdx),
475+
);
476+
473477
if (seen == null) {
474478
// Optimisation: set this lazily only when we've encountered a symlink
475479
seen = new Set([requestedNormalPath]);

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
118118
[p('/project/root')],
119119
],
120120
[p('/outside/../project/bar.js'), p('/project/bar.js'), []],
121+
[p('root/project/bar.js'), p('/project/bar.js'), [p('/project/root')]],
121122
])(
122123
'%s -> %s through expected symlinks',
123124
(givenPath, expectedRealPath, expectedSymlinks) =>
@@ -197,6 +198,35 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
197198
});
198199
});
199200

201+
describe('symlinks to an ancestor of the project root', () => {
202+
beforeEach(() => {
203+
tfs.addOrModify(p('foo/link-up-2'), ['', 0, 0, 0, '', '', p('../..')]);
204+
});
205+
206+
test.each([
207+
[
208+
p('foo/link-up-2/project/bar.js'),
209+
p('/project/bar.js'),
210+
[p('/project/foo/link-up-2')],
211+
],
212+
[
213+
p('foo/link-up-2/project/foo/link-up-2/outside/external.js'),
214+
p('/outside/external.js'),
215+
[p('/project/foo/link-up-2')],
216+
],
217+
])(
218+
'lookup can find files that go back towards the project root (%s)',
219+
(mixedPath, expectedRealPath, expectedSymlinks) => {
220+
expect(tfs.lookup(mixedPath)).toEqual({
221+
exists: true,
222+
realPath: expectedRealPath,
223+
links: new Set(expectedSymlinks),
224+
type: 'f',
225+
});
226+
},
227+
);
228+
});
229+
200230
describe('getDifference', () => {
201231
test('returns changed (inc. new) and removed files in given FileData', () => {
202232
const newFiles: FileData = new Map([

0 commit comments

Comments
 (0)