Skip to content
This repository has been archived by the owner on Jul 24, 2019. It is now read-only.

Default to user agent string in npm config if it is provided #298

Merged
merged 2 commits into from
Feb 18, 2015

Conversation

sshen
Copy link
Contributor

@sshen sshen commented Feb 12, 2015

In some workplaces, the network http proxy is very restrictive and restricts the traffic to very specific user agent strings. This patch should make it so that the phantomjs install script respects the user agent string specified in the npm config.

@@ -232,7 +232,8 @@ function getRequestOptions(conf) {
options.proxy = proxyUrl

// If going through proxy, spoof the User-Agent, since may commerical proxies block blank or unknown agents in headers
options.headers['User-Agent'] = 'curl/7.21.4 (universal-apple-darwin11.0) libcurl/7.21.4 OpenSSL/0.9.8r zlib/1.2.5'
var userAgent = conf.get('user-agent') || 'curl/7.21.4 (universal-apple-darwin11.0) libcurl/7.21.4 OpenSSL/0.9.8r zlib/1.2.5'
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think this will do what you expect, because user-agent is never empty. you can just remove the ||

This user agent string can be overriden through the 'npm config' utility
@sshen
Copy link
Contributor Author

sshen commented Feb 12, 2015

Good point! I removed the unnecessary default settings in the latest commit. I think you might hear some grumbling about needing to run "npm config set user-agent" with this commit, but I think it's the right way to solve the proprietary http proxy issue.

Please take a look at your earliest convenience.

@nicks
Copy link
Contributor

nicks commented Feb 18, 2015

thanks!

nicks added a commit that referenced this pull request Feb 18, 2015
Default to user agent string in npm config if it is provided
@nicks nicks merged commit ee50305 into Medium:master Feb 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants