Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix handling of paths that leave and re-enter the project root via symlinks #1280

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

@robhogan robhogan commented Jun 3, 2024

Summary:

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 '':

// Internal: Tries to collapse sequences like `../root/foo` for root
// `/project/root` down to the normal 'foo'.
#tryCollapseIndirectionsInSuffix(
fullPath: string, // A string ending with the relative path to process
startOfRelativePart: number, // Index of the start of part to process
implicitUpIndirections: number, // 0=root-relative, 1=dirname(root)-relative...
): ?string {

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

…mlinks

Summary:
## 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
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 3, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57719224

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 65fd022.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants