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(resolution): Normalise non-HTTPS NPM registry URLs too #6353

Merged
merged 1 commit into from
Sep 3, 2018
Merged

fix(resolution): Normalise non-HTTPS NPM registry URLs too #6353

merged 1 commit into from
Sep 3, 2018

Conversation

edmorley
Copy link
Contributor

@edmorley edmorley commented Sep 3, 2018

Summary

For some packages the NPM registry is incorrectly returning tarball URLs that are not using HTTPS. For example:
http://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz

Previously the registry.npmjs.org -> registry.yarnpkg.com tarball URL normalisation was not occurring for these non-HTTPS URLs, causing unwanted http://registry.npmjs.org references in yarn.lock.

Whilst the real fix needs to be made upstream:
https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285

...this change prevents lockfile churn and insecure package downloads over HTTP in the meantime.

Fixes #6259.

Test plan

The request-cache entry for onetime has been updated to the current NPM registry response, which contains the bogus tarball URLs. Running the newly added test without the corrected regex, confirmed the test failed prior to the fix.

In addition, running yarn add [email protected] with yarn 1.9.4 gives a yarn.lock containing the http://registry.npmjs.org/onetime/... tarball URL, whereas doing the same with a build from this branch gives https://registry.yarnpkg.com/onetime/....

For some packages the NPM registry is incorrectly returning tarball
URLs that are not using HTTPS. For example:
`http://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz`

Previously the `registry.npmjs.org` -> `registry.yarnpkg.com` tarball
URL normalisation was not occurring for these non-HTTPS URLs, causing
unwanted `http://registry.npmjs.org` references in `yarn.lock`.

Whilst the real fix needs to be made upstream:
https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285

...this change prevents lockfile churn and insecure package downloads
over HTTP in the meantime.

The request-cache entry for `onetime` has been updated to the current
NPM registry response, which contains the bogus `tarball` URLs.

Fixes #6259.
@edmorley
Copy link
Contributor Author

edmorley commented Sep 3, 2018

Could someone retrigger the circle-ci test-macos-node10 job? It failed with what looks like an intermittent/unrelated error.

@arcanis
Copy link
Member

arcanis commented Sep 3, 2018

Yeah, we have these errors sometimes. Your PR looks good, thanks! I think that it was supposed to work like that, seeing that we were using http[s]: ... curious to use a range for the s when there's a single character to match.

@arcanis arcanis merged commit 196fa94 into yarnpkg:master Sep 3, 2018
@edmorley
Copy link
Contributor Author

edmorley commented Sep 3, 2018

Yeah agreed - suspect it was the intention to handle both from the beginning.
Thank for you for the review/merge :-)

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.

reinstall of onetime package replaces https with http
2 participants