Skip to content

fix: ERR_PNPM_TARBALL_EXTRACT when the URL's hash contains a slash#8761

Merged
zkochan merged 4 commits intopnpm:mainfrom
vudsen:slash-support
Nov 18, 2024
Merged

fix: ERR_PNPM_TARBALL_EXTRACT when the URL's hash contains a slash#8761
zkochan merged 4 commits intopnpm:mainfrom
vudsen:slash-support

Conversation

@vudsen
Copy link
Copy Markdown
Contributor

@vudsen vudsen commented Nov 15, 2024

Before determine the URL type, encode the hash if it contains a slash.

Fix: #7697

@vudsen vudsen requested a review from zkochan as a code owner November 15, 2024 07:29
@welcome
Copy link
Copy Markdown

welcome bot commented Nov 15, 2024

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

wantedDependency: { pref: string }
): Promise<ResolveResult | null> {
if (!wantedDependency.pref.startsWith('http:') && !wantedDependency.pref.startsWith('https:')) {
if (!(wantedDependency.pref.startsWith('http:') || wantedDependency.pref.startsWith('https:'))) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this changed? it is unrelated to the fix

Copy link
Copy Markdown
Contributor Author

@vudsen vudsen Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this is my own hobby. This writing is better for my understanding...

@zkochan zkochan requested a review from Copilot November 17, 2024 16:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (2)

pnpm/test/install/misc.ts:574

  • The test should verify that the package was correctly installed. Add an assertion to check the presence of the installed package.
expect(result.status).toBe(0)

resolving/tarball-resolver/src/index.ts:32

  • Reassigning 'pref' might lead to unintended side effects. Consider using a new variable instead.
pref = url.href

vudsen and others added 2 commits November 18, 2024 08:58
Co-authored-by: Zoltan Kochan <z@kochan.io>
@vudsen vudsen requested a review from zkochan November 18, 2024 01:28
@zkochan zkochan merged commit 3be45b7 into pnpm:main Nov 18, 2024
@welcome
Copy link
Copy Markdown

welcome bot commented Nov 18, 2024

Congrats on merging your first pull request! 🎉🎉🎉

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.

TARBALL_EXTRACT error while installing a dependency from GitHub having a slash in branch name

3 participants