Skip to content

Commit

Permalink
Fix bug in symlink traversal with indirect targets (#1084)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #1084

Fixes a bug where symlinks beginning with indirections (`../`) may fail to traverse in some `projectRoot` / `watchFolders` configurations.

This occurs due to incomplete "normalisation" of a symlink target in `metro-file-map`, relative to the project root. Given a project root of `/project` and a symlink `/project/src/link-to-foo  -> ../../project/foo.js`, we would normalise the target to `../project/foo.js` rather than simply `foo.js` - we were not taking into account that further normalisation could be possible after joining with `projectRoot`.

"Normal" paths are defined as being *fully* normalised relative to the project root. The unnecessary indirection out and back into the project root fails because `TreeFS` is a DAG, and internally there is no entry for `rootNode.get('..').get('project')` - this would be a cycle back to `rootNode`.

Changelog:
```
 * **[Fix]:** Symlinks with indirections may not be resolvable
```

Reviewed By: yungsters

Differential Revision: D49323614

fbshipit-source-id: e01a10a0455b0af22ebc3cdd7c34ca8fd888d0f4
  • Loading branch information
robhogan authored and facebook-github-bot committed Sep 16, 2023
1 parent 75f3927 commit 6f3f7e6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
14 changes: 7 additions & 7 deletions packages/metro-file-map/src/lib/TreeFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -559,13 +559,13 @@ export default class TreeFS implements MutableFileSystem {
typeof literalSymlinkTarget === 'string',
'Expected symlink target to be populated.',
);
if (path.isAbsolute(literalSymlinkTarget)) {
normalSymlinkTarget = path.relative(this.#rootDir, literalSymlinkTarget);
} else {
normalSymlinkTarget = path.normalize(
path.join(path.dirname(canonicalPathOfSymlink), literalSymlinkTarget),
);
}
const absoluteSymlinkTarget = path.resolve(
this.#rootDir,
canonicalPathOfSymlink,
'..', // Symlink target is relative to its containing directory.
literalSymlinkTarget, // May be absolute, in which case the above are ignored
);
normalSymlinkTarget = path.relative(this.#rootDir, absoluteSymlinkTarget);
this.#cachedNormalSymlinkTargets.set(symlinkNode, normalSymlinkTarget);
return normalSymlinkTarget;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
[p('foo/link-to-another.js'), ['', 0, 0, 0, '', '', p('another.js')]],
[p('../outside/external.js'), ['', 0, 0, 0, '', '', 0]],
[p('bar.js'), ['bar', 234, 0, 0, '', '', 0]],
[p('link-to-foo'), ['', 456, 0, 0, '', '', p('././abnormal/../foo')]],
[p('link-to-foo'), ['', 456, 0, 0, '', '', p('./../project/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')]],
Expand Down

0 comments on commit 6f3f7e6

Please sign in to comment.