From fade068245ac47847e446f80ab565ec56de70f8f Mon Sep 17 00:00:00 2001 From: Manuel Ohlendorf Date: Tue, 2 Aug 2016 13:59:17 +0200 Subject: [PATCH 1/5] Use the configured proxy host and port for the download --- lib/HttpsProxyAgent.js | 123 +++++++++++++++++++++++++++++++++++++++++ lib/Local.js | 7 ++- lib/LocalBinary.js | 18 ++++-- 3 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 lib/HttpsProxyAgent.js diff --git a/lib/HttpsProxyAgent.js b/lib/HttpsProxyAgent.js new file mode 100644 index 0000000..cd461aa --- /dev/null +++ b/lib/HttpsProxyAgent.js @@ -0,0 +1,123 @@ +var util = require('util'), + https = require('https'), + http = require('http'), + lls = require('tls'); + +function HttpsProxyAgent(options){ + https.Agent.call(this, options); + + this.proxyHost = options.proxyHost; + this.proxyPort = options.proxyPort; + + this.createConnection = function (opts, callback){ + // do a CONNECT request + var req = http.request({ + host: options.proxyHost, + port: options.proxyPort, + method: 'CONNECT', + path: opts.host + ':' + opts.port, + headers: { + host: opts.host + } + }); + + req.on('connect', function (res, socket){ + var cts = lls.connect({ + host: opts.host, + socket: socket + }, function () { + callback(false, cts); + }); + }); + + req.on('error', function (err) { + callback(err, null); + }); + + req.end(); + }; +} + +util.inherits(HttpsProxyAgent, https.Agent); + +// Almost verbatim copy of http.Agent.addRequest +HttpsProxyAgent.prototype.addRequest = function (req, options){ + var name = options.host + ':' + options.port; + if(options.path) name += ':' + options.path; + + if(!this.sockets[name]) this.sockets[name] = []; + + if(this.sockets[name].length < this.maxSockets){ + // if we are under maxSockets create a new one. + this.createSocket(name, options.host, options.port, options.path, req, function (socket){ + req.onSocket(socket); + }); + }else{ + // we are over limit so we'll add it to the queue. + if(!this.requests[name]) + this.requests[name] = []; + this.requests[name].push(req); + } +}; + +// Almost verbatim copy of http.Agent.createSocket +HttpsProxyAgent.prototype.createSocket = function (name, host, port, localAddress, req, callback){ + var self = this; + var options = util._extend({}, self.options); + options.port = port; + options.host = host; + options.localAddress = localAddress; + + options.servername = host; + if(req){ + var hostHeader = req.getHeader('host'); + if(hostHeader) + options.servername = hostHeader.replace(/:.*$/, ''); + } + + self.createConnection(options, function (err, s) { + if(err) { + err.message += ' while connecting to HTTP(S) proxy server ' + self.proxyHost + ':' + self.proxyPort; + + if (req) + req.emit('error', err); + else + throw err; + + return; + } + + if(!self.sockets[name]) self.sockets[name] = []; + + self.sockets[name].push(s); + + var onFree = function (){ + self.emit('free', s, host, port, localAddress); + }; + + var onClose = function (){ + // this is the only place where sockets get removed from the Agent. + // if you want to remove a socket from the pool, just close it. + // all socket errors end in a close event anyway. + self.removeSocket(s, name, host, port, localAddress); + }; + + var onRemove = function (){ + // we need this function for cases like HTTP 'upgrade' + // (defined by WebSockets) where we need to remove a socket from the pool + // because it'll be locked up indefinitely + self.removeSocket(s, name, host, port, localAddress); + s.removeListener('close', onClose); + s.removeListener('free', onFree); + s.removeListener('agentRemove', onRemove); + }; + + s.on('free', onFree); + s.on('close', onClose); + s.on('agentRemove', onRemove); + + callback(s); + }); +}; + +module.exports = HttpsProxyAgent; \ No newline at end of file diff --git a/lib/Local.js b/lib/Local.js index 1f00ac4..f91948b 100644 --- a/lib/Local.js +++ b/lib/Local.js @@ -199,7 +199,12 @@ function Local(){ this.getBinaryPath = function(callback){ if(typeof(this.binaryPath) == 'undefined'){ this.binary = new LocalBinary(); - this.binary.binaryPath(callback); + var conf = {}; + if(this.proxyHost && this.proxyPort){ + conf.proxyHost = this.proxyHost; + conf.proxyPort = this.proxyPort; + } + this.binary.binaryPath(conf, callback); } else { callback(this.binaryPath); } diff --git a/lib/LocalBinary.js b/lib/LocalBinary.js index c81260c..82458ca 100644 --- a/lib/LocalBinary.js +++ b/lib/LocalBinary.js @@ -1,7 +1,9 @@ var https = require('https'), + url = require('url'), fs = require('fs'), path = require('path'), os = require('os'), + HttpsProxyAgent = require('./HttpsProxyAgent'), LocalError = require('./LocalError'); function LocalBinary(){ @@ -20,7 +22,7 @@ function LocalBinary(){ this.httpPath = 'https://s3.amazonaws.com/browserStack/browserstack-local/BrowserStackLocal-linux-ia32'; } - this.download = function(destParentDir, callback){ + this.download = function(conf, destParentDir, callback){ if(!this.checkPath(destParentDir)) fs.mkdirSync(destParentDir); @@ -28,7 +30,15 @@ function LocalBinary(){ var binaryPath = path.join(destParentDir, destBinaryName); var file = fs.createWriteStream(binaryPath); - https.get(this.httpPath, function (response) { + var options = url.parse(this.httpPath); + if(conf.proxyHost && conf.proxyPort){ + options.agent = new HttpsProxyAgent({ + proxyHost: conf.proxyHost, + proxyPort: conf.proxyPort + }); + } + + https.get(options, function (response) { response.on('end', function () { fs.chmod(binaryPath, '0755', function() { callback(binaryPath); @@ -38,14 +48,14 @@ function LocalBinary(){ }); }; - this.binaryPath = function(callback){ + this.binaryPath = function(conf, callback){ var destParentDir = this.getAvailableDirs(); var destBinaryName = (this.windows) ? 'BrowserStackLocal.exe' : 'BrowserStackLocal'; var binaryPath = path.join(destParentDir, destBinaryName); if(this.checkPath(binaryPath, fs.X_OK)){ callback(binaryPath); } else { - this.download(destParentDir, callback); + this.download(conf, destParentDir, callback); } }; From 8349722ac283e66ea8cc233238df19775539ff89 Mon Sep 17 00:00:00 2001 From: Manuel Ohlendorf Date: Tue, 2 Aug 2016 14:39:39 +0200 Subject: [PATCH 2/5] Instead of taking the message object, just take message The message field is again an object that contains a message. In order to receive the text, take that. --- lib/Local.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Local.js b/lib/Local.js index f91948b..d976320 100644 --- a/lib/Local.js +++ b/lib/Local.js @@ -40,7 +40,7 @@ function Local(){ callback(new LocalError('No output received')); if(data['state'] != 'connected'){ - callback(new LocalError(data['message'])); + callback(new LocalError(data['message']['message'])); } else { that.pid = data['pid']; callback(); From 99dbe22b24261532657d738c5f9cc4eabd228505 Mon Sep 17 00:00:00 2001 From: Manuel Ohlendorf Date: Tue, 2 Aug 2016 16:14:40 +0200 Subject: [PATCH 3/5] Add tests for LocalBinary download Testing download without and with a proxy --- package.json | 3 ++- test/local.js | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 50d0d6e..d21f60d 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,8 @@ "eslint": "1.10.3", "expect.js": "0.3.1", "mocha": "2.4.5", - "mocks": "0.0.15" + "mocks": "0.0.15", + "proxy": "^0.2.4" }, "bugs": "https://github.com/browserstack/browserstack-local-nodejs/issues", "homepage": "https://github.com/browserstack/browserstack-local-nodejs", diff --git a/test/local.js b/test/local.js index f73a260..0ab40cb 100644 --- a/test/local.js +++ b/test/local.js @@ -2,7 +2,9 @@ var expect = require('expect.js'), mocks = require('mocks'), path = require('path'), fs = require('fs'), - browserstack = require('../index'); + Proxy = require('proxy'), + browserstack = require('../index'), + LocalBinary = require('../lib/LocalBinary'); describe('Local', function () { var bsLocal; @@ -152,3 +154,57 @@ describe('Local', function () { }); }); + +describe('LocalBinary', function () { + var proxy; + var proxyPort; + var binary; + var tempDownloadPath; + + before(function (done) { + // setup HTTP proxy server + proxy = new Proxy(); + proxy.listen(function () { + proxyPort = proxy.address().port; + done(); + }); + }); + + after(function (done) { + proxy.once('close', function () { done(); }); + proxy.close(); + }); + + beforeEach(function () { + binary = new LocalBinary(); + tempDownloadPath = path.join(process.cwd(), 'download'); + }); + + afterEach(function () { + fs.rmdirSync(tempDownloadPath); + }); + + it('should download binaries without proxy', function (done) { + this.timeout(600000); + var conf = {}; + binary.download(conf, tempDownloadPath, function (result) { + expect(fs.existsSync(result)).to.equal(true); + fs.unlinkSync(result); + done(); + }); + }); + + it('should download binaries with proxy', function (done) { + this.timeout(600000); + var conf = { + proxyHost: '127.0.0.1', + proxyPort: proxyPort + }; + binary.download(conf, tempDownloadPath, function (result) { + // test for file existence + expect(fs.existsSync(result)).to.equal(true); + fs.unlinkSync(result); + done(); + }); + }); +}); From e14efc3b4b6a0e80d66bac86a45c2a15843ea0da Mon Sep 17 00:00:00 2001 From: Manuel Ohlendorf Date: Tue, 2 Aug 2016 16:48:30 +0200 Subject: [PATCH 4/5] Always delete result files after tests Use rimraf for recursive delete --- package.json | 3 ++- test/local.js | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index d21f60d..996219d 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "expect.js": "0.3.1", "mocha": "2.4.5", "mocks": "0.0.15", - "proxy": "^0.2.4" + "proxy": "^0.2.4", + "rimraf": "^2.5.4" }, "bugs": "https://github.com/browserstack/browserstack-local-nodejs/issues", "homepage": "https://github.com/browserstack/browserstack-local-nodejs", diff --git a/test/local.js b/test/local.js index 0ab40cb..5fd442d 100644 --- a/test/local.js +++ b/test/local.js @@ -2,6 +2,7 @@ var expect = require('expect.js'), mocks = require('mocks'), path = require('path'), fs = require('fs'), + rimraf = require('rimraf'), Proxy = require('proxy'), browserstack = require('../index'), LocalBinary = require('../lib/LocalBinary'); @@ -181,7 +182,7 @@ describe('LocalBinary', function () { }); afterEach(function () { - fs.rmdirSync(tempDownloadPath); + rimraf.sync(tempDownloadPath); }); it('should download binaries without proxy', function (done) { @@ -189,7 +190,6 @@ describe('LocalBinary', function () { var conf = {}; binary.download(conf, tempDownloadPath, function (result) { expect(fs.existsSync(result)).to.equal(true); - fs.unlinkSync(result); done(); }); }); @@ -203,7 +203,6 @@ describe('LocalBinary', function () { binary.download(conf, tempDownloadPath, function (result) { // test for file existence expect(fs.existsSync(result)).to.equal(true); - fs.unlinkSync(result); done(); }); }); From f5c58d0c9e8e2a1bd51aaa766eac0a8f15c12811 Mon Sep 17 00:00:00 2001 From: Manuel Ohlendorf Date: Tue, 2 Aug 2016 16:49:58 +0200 Subject: [PATCH 5/5] Use library for https proxy agent instead of local one The library version also runs with node v.0.10.x --- lib/HttpsProxyAgent.js | 123 ----------------------------------------- lib/LocalBinary.js | 6 +- package.json | 1 + 3 files changed, 4 insertions(+), 126 deletions(-) delete mode 100644 lib/HttpsProxyAgent.js diff --git a/lib/HttpsProxyAgent.js b/lib/HttpsProxyAgent.js deleted file mode 100644 index cd461aa..0000000 --- a/lib/HttpsProxyAgent.js +++ /dev/null @@ -1,123 +0,0 @@ -var util = require('util'), - https = require('https'), - http = require('http'), - lls = require('tls'); - -function HttpsProxyAgent(options){ - https.Agent.call(this, options); - - this.proxyHost = options.proxyHost; - this.proxyPort = options.proxyPort; - - this.createConnection = function (opts, callback){ - // do a CONNECT request - var req = http.request({ - host: options.proxyHost, - port: options.proxyPort, - method: 'CONNECT', - path: opts.host + ':' + opts.port, - headers: { - host: opts.host - } - }); - - req.on('connect', function (res, socket){ - var cts = lls.connect({ - host: opts.host, - socket: socket - }, function () { - callback(false, cts); - }); - }); - - req.on('error', function (err) { - callback(err, null); - }); - - req.end(); - }; -} - -util.inherits(HttpsProxyAgent, https.Agent); - -// Almost verbatim copy of http.Agent.addRequest -HttpsProxyAgent.prototype.addRequest = function (req, options){ - var name = options.host + ':' + options.port; - if(options.path) name += ':' + options.path; - - if(!this.sockets[name]) this.sockets[name] = []; - - if(this.sockets[name].length < this.maxSockets){ - // if we are under maxSockets create a new one. - this.createSocket(name, options.host, options.port, options.path, req, function (socket){ - req.onSocket(socket); - }); - }else{ - // we are over limit so we'll add it to the queue. - if(!this.requests[name]) - this.requests[name] = []; - this.requests[name].push(req); - } -}; - -// Almost verbatim copy of http.Agent.createSocket -HttpsProxyAgent.prototype.createSocket = function (name, host, port, localAddress, req, callback){ - var self = this; - var options = util._extend({}, self.options); - options.port = port; - options.host = host; - options.localAddress = localAddress; - - options.servername = host; - if(req){ - var hostHeader = req.getHeader('host'); - if(hostHeader) - options.servername = hostHeader.replace(/:.*$/, ''); - } - - self.createConnection(options, function (err, s) { - if(err) { - err.message += ' while connecting to HTTP(S) proxy server ' + self.proxyHost + ':' + self.proxyPort; - - if (req) - req.emit('error', err); - else - throw err; - - return; - } - - if(!self.sockets[name]) self.sockets[name] = []; - - self.sockets[name].push(s); - - var onFree = function (){ - self.emit('free', s, host, port, localAddress); - }; - - var onClose = function (){ - // this is the only place where sockets get removed from the Agent. - // if you want to remove a socket from the pool, just close it. - // all socket errors end in a close event anyway. - self.removeSocket(s, name, host, port, localAddress); - }; - - var onRemove = function (){ - // we need this function for cases like HTTP 'upgrade' - // (defined by WebSockets) where we need to remove a socket from the pool - // because it'll be locked up indefinitely - self.removeSocket(s, name, host, port, localAddress); - s.removeListener('close', onClose); - s.removeListener('free', onFree); - s.removeListener('agentRemove', onRemove); - }; - - s.on('free', onFree); - s.on('close', onClose); - s.on('agentRemove', onRemove); - - callback(s); - }); -}; - -module.exports = HttpsProxyAgent; \ No newline at end of file diff --git a/lib/LocalBinary.js b/lib/LocalBinary.js index 82458ca..aa6a503 100644 --- a/lib/LocalBinary.js +++ b/lib/LocalBinary.js @@ -3,7 +3,7 @@ var https = require('https'), fs = require('fs'), path = require('path'), os = require('os'), - HttpsProxyAgent = require('./HttpsProxyAgent'), + HttpsProxyAgent = require('https-proxy-agent'), LocalError = require('./LocalError'); function LocalBinary(){ @@ -33,8 +33,8 @@ function LocalBinary(){ var options = url.parse(this.httpPath); if(conf.proxyHost && conf.proxyPort){ options.agent = new HttpsProxyAgent({ - proxyHost: conf.proxyHost, - proxyPort: conf.proxyPort + host: conf.proxyHost, + port: conf.proxyPort }); } diff --git a/package.json b/package.json index 996219d..fa3e062 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "author": "BrowserStack", "license": "MIT", "dependencies": { + "https-proxy-agent": "^1.0.0", "is-running": "^2.0.0" }, "devDependencies": {