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

Support custom binary hosting mirror #170

Merged
merged 1 commit into from
Nov 4, 2015

Conversation

fengmk2
Copy link
Contributor

@fengmk2 fengmk2 commented Jul 10, 2015

Like we setup a mirror site for node-inspector at
https://npm.taobao.org/mirrors/node-inspector/

These mirrors try to help people download binary files from China.

$ npm install node-inspector \
  --profiler_binary_host_mirror=https://npm.taobao.org/mirrors/node-inspector/

@springmeyer
Copy link
Contributor

I'm hesitant to accept this due to security concerns. So let's work together to brainstorm here if there is a safe way to allow this. My concern is that a malicious application could set this ENV variable (perhaps by writing to ~/.bash_profile) and then a user would unknowingly be installing binaries from a different location.

Possible solutions:

  • add support for shasum validation - this would not completely protect the user but would make it much harder to sneak in malicious binaries
  • require a custom flag be passed to support mirrors like npm install --allow-binary-mirror. Only when that flag is passed would the ENV option be respected.
  • use the npm config infrastructure instead of an arbitrary ENV option like discussed at Overriding "host" to point to alternative binary repository #74

@fengmk2
Copy link
Contributor Author

fengmk2 commented Jul 10, 2015

How about follow npm install config options?

$ npm install node-inspector --profiler_binary_host_mirror=http://npm.taobao.org/mirrors/node-inspector/

And the process.env.npm_config_profiler_binary_host_mirror env it's set by npm.

@springmeyer
Copy link
Contributor

Latest fix looks good. Can you think of a good way to test this on travis?

@fengmk2
Copy link
Contributor Author

fengmk2 commented Jul 21, 2015

OK, I try to add test for this change.

@fengmk2
Copy link
Contributor Author

fengmk2 commented Jul 21, 2015

@springmeyer unittest added.

@fengmk2
Copy link
Contributor Author

fengmk2 commented Jul 25, 2015

@springmeyer travis pass now

@afc163
Copy link

afc163 commented Oct 27, 2015

+1

@fengmk2
Copy link
Contributor Author

fengmk2 commented Oct 27, 2015

@springmeyer review again?

@yiminghe
Copy link

We need this in china 👍

@sorrycc
Copy link

sorrycc commented Oct 27, 2015

+1

2 similar comments
@antife-yinyue
Copy link

👍

@nightink
Copy link

👍

@fengmk2
Copy link
Contributor Author

fengmk2 commented Nov 2, 2015

@springmeyer can you review this again?

@springmeyer
Copy link
Contributor

Great, @fengmk2 will merge once you can:

  • Rebase against latest master and ensure the tests pass
  • Add docs to the README.md

Sound good? Thanks for this.

@fengmk2
Copy link
Contributor Author

fengmk2 commented Nov 3, 2015

Sure, I will do it today.

Like we setup a mirror site for node-inspector at
https://npm.taobao.org/mirrors/node-inspector/

These mirrors try to help people download binary files from China.
@fengmk2
Copy link
Contributor Author

fengmk2 commented Nov 3, 2015

@springmeyer rebase and docs done!

@springmeyer
Copy link
Contributor

Thanks @fengmk2!

springmeyer pushed a commit that referenced this pull request Nov 4, 2015
Support custom binary hosting mirror
@springmeyer springmeyer merged commit 0e20f79 into mapbox:master Nov 4, 2015
@fengmk2
Copy link
Contributor Author

fengmk2 commented Nov 5, 2015

Great!

@fengmk2 fengmk2 deleted the support-host-mirror branch November 5, 2015 00:38
fengmk2 added a commit to fengmk2/node-sqlite3 that referenced this pull request Dec 21, 2015
@springmeyer springmeyer mentioned this pull request Mar 31, 2016
@springmeyer springmeyer mentioned this pull request Nov 7, 2016
mstade added a commit to zambezi/jsbin that referenced this pull request Nov 7, 2016
This updates the sqlite3 dependency to the latest available version.
Among other things, this should enable people interested in hosting
their own copy of jsbin to specify binary mirrors of the sqlite3
dependency. The reason this requires an update is that the current
version being used depends on a version of node-pre-gyp that doesn't
support mirroring – see mapbox/node-pre-gyp#170 for more info on
this feature.
mstade added a commit to zambezi/jsbin that referenced this pull request Jan 23, 2017
This updates the sqlite3 dependency to the latest available version.
Among other things, this should enable people interested in hosting
their own copy of jsbin to specify binary mirrors of the sqlite3
dependency. The reason this requires an update is that the current
version being used depends on a version of node-pre-gyp that doesn't
support mirroring – see mapbox/node-pre-gyp#170 for more info on
this feature.
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.

7 participants