Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: handle symlinking when root directory begins with a special char… #53

Closed
wants to merge 1 commit into from

Conversation

MonochromeChameleon
Copy link
Contributor

This arose out of my investigation into npm/cli#2447
The file resolution for local paths beginning with /~something or /.something strips off the leading slash, meaning that the eventual resolved path is incorrect as it is incorrectly treated as not being an absolute path

I've added a couple of test cases to demonstrate the failing situations and altered the regexp replacement to only match ~ and . characters that are followed by a /

References

Related to npm/cli#2447

@isaacs
Copy link
Contributor

isaacs commented Jun 2, 2021

Thanks! This will be in the next npm release.

isaacs added a commit that referenced this pull request Jun 15, 2021
The change made in e8c4301 (#53) was
reasonable, but had the unfortunate side effect of breaking backwards
compatibility for urls like 'file://.'.

Further investigation showed that we really weren't complying with RFC
8909's specifications regarding file: URLs, and even further
investigation than that showed that it'll be a breaking change if we do.

This patch isolates the non-8909 compliance bits into a single block
that can be disabled with an environment variable and easily removed
when we are willing to take on that breaking change.  The bug fixed by
the commit that broke file://. is still fixed, and we obey RFC8909
otherwise.
isaacs added a commit that referenced this pull request Jun 15, 2021
The change made in e8c4301 (#53) was
reasonable, but had the unfortunate side effect of breaking backwards
compatibility for urls like 'file://.'.

Further investigation showed that we really weren't complying with RFC
8909's specifications regarding file: URLs, and even further
investigation than that showed that it'll be a breaking change if we do.

This patch isolates the non-8909 compliance bits into a single block
that can be disabled with an environment variable and easily removed
when we are willing to take on that breaking change.  The bug fixed by
the commit that broke file://. is still fixed, and we obey RFC8909
otherwise.

PR-URL: #60
Credit: @isaacs
Close: #60
Reviewed-by: @ruyadorno
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants