diff --git a/.travis.yml b/.travis.yml index c32b0a8..a0ae418 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,9 +2,9 @@ language: node_js script: make prepush node_js: - - "4" - - "6" - - "8" + - "4" # Maintenance LTS release + - "lts/*" # Active LTS release + - "node" # Latest stable release after_success: - make report-coverage - make nsp diff --git a/README.md b/README.md index 829841d..575488f 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ var client = restify.createJsonClient({ |Name | Type | Description | | :--- | :----: | :---- | |accept|String|Accept header to send| +|appendPath|Boolean|Append paths provided at verb time to existing client path| |audit|Boolean|Enable Audit logging| |auditor|Function|Function for Audit logging| |connectTimeout|Number|Amount of time to wait for a socket| diff --git a/lib/HttpClient.js b/lib/HttpClient.js index 3908419..a3f206c 100644 --- a/lib/HttpClient.js +++ b/lib/HttpClient.js @@ -428,6 +428,7 @@ function isProxyForURL(noProxy, address) { function HttpClient(options) { assert.object(options, 'options'); + assert.optionalBool(options.appendPath, 'options.appendPath'); assert.optionalObject(options.headers, 'options.headers'); assert.object(options.log, 'options.log'); assert.optionalObject(options.query, 'options.query'); @@ -443,6 +444,7 @@ function HttpClient(options) { var self = this; this.agent = options.agent; + this.appendPath = options.appendPath || false; this.ca = options.ca; this.checkServerIdentity = options.checkServerIdentity; this.cert = options.cert; @@ -484,8 +486,10 @@ function HttpClient(options) { 'must specify http/https protocol!' ); this.url = parsedUrl; + this.path = parsedUrl.pathname; } else { this.url = {}; + this.path = ''; } // HTTP proxy: `options.proxy` wins, else `https_proxy`/`http_proxy` envvars @@ -742,10 +746,36 @@ HttpClient.prototype.request = function request(opts, cb) { }; +/** + * internal options construction at verb time. variadic args, so the `options` + * object can be a string or a pojo: + * client.get('/foo', cb); + * => method='GET', options='/foo' + * client.get({ path: '/foo' }, cb); + * => method='GET', options={ path: '/foo' } + * @private + * @method _options + * @param {String} method http verb + * @param {String | Object} options string path or options object + * @returns {Object} options object specific to this request +*/ HttpClient.prototype._options = function (method, options) { + // need to assert on all options again here - we're not doing that at verb + // time for some reason which could cause all sorts of weird behavior. + assert.string(method, 'method'); + + // assert on variadic signature based on typeof + if (typeof options === 'object') { + // TODO: missing lots of asserts here + assert.optionalBool(options.appendPath, 'options.appendPath'); + } else { + assert.string(options, 'options'); + } + var self = this; var opts = { + appendPath: options.appendPath || self.appendPath, agent: (typeof options.agent !== 'undefined') ? options.agent : self.agent, @@ -759,9 +789,6 @@ HttpClient.prototype._options = function (method, options) { log: options.log || self.log, method: method, passphrase: options.passphrase || self.passphrase, - path: (typeof options !== 'object') ? - options : - (options.path || self.path), pfx: options.pfx || self.pfx, query: options.query || self.query, rejectUnauthorized: options.rejectUnauthorized || @@ -776,6 +803,17 @@ HttpClient.prototype._options = function (method, options) { opts.checkServerIdentity = checkServerIdentity; } + // if appendPath option is true, append the passed in path to existing base + // path. + if (opts.appendPath === true) { + opts.path = self.path + '/' + ((typeof options !== 'object') ? + options : options.path); + opts.path = opts.path.replace(/(\/)\/+/g, '$1'); + } else { + // fall back on legacy behavior + opts.path = (typeof options !== 'object') ? options : options.path; + } + if (!opts.retry && opts.retry !== false) { opts.retry = self.retry; } diff --git a/test/appendPath.test.js b/test/appendPath.test.js new file mode 100644 index 0000000..b5a5c48 --- /dev/null +++ b/test/appendPath.test.js @@ -0,0 +1,130 @@ +'use strict'; + +// external files +var assert = require('chai').assert; +var bunyan = require('bunyan'); +var restify = require('restify'); + +// local files +var clients = require('../lib'); + + +describe('`appendPath` option', function () { + + var SERVER; + var CLIENT; + var LOG = bunyan.createLogger({ + name: 'clientlog' + }); + + beforeEach(function (done) { + SERVER = restify.createServer({ + name: 'unittest', + log: LOG + }); + SERVER.use(restify.plugins.queryParser()); + SERVER.listen(3000, done); + }); + + afterEach(function (done) { + CLIENT.close(); + SERVER.close(done); + }); + + + describe('constructor time appendPath', function () { + + it('should append to existing constructor time path', function (done) { + SERVER.get('/foo/bar', function (req, res, next) { + assert.deepEqual(req.path(), '/foo/bar'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000/foo', + appendPath: true + }); + CLIENT.get('/bar', done); + }); + + it('should append to bare host', function (done) { + SERVER.get('/foo', function (req, res, next) { + assert.deepEqual(req.path(), '/foo'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000', + appendPath: true + }); + CLIENT.get('/foo', done); + }); + + it('should dedupe url slashes', function (done) { + SERVER.get('/foo', function (req, res, next) { + assert.deepEqual(req.path(), '/foo'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000//', + appendPath: true + }); + CLIENT.get('//foo', done); + }); + }); + + describe('verb time appendPath', function () { + + it('should append to existing constructor time path', function (done) { + SERVER.get('/foo/bar', function (req, res, next) { + assert.deepEqual(req.path(), '/foo/bar'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000/foo' + }); + CLIENT.get({ + path: '/bar', + appendPath: true + }, done); + }); + + it('should append to bare host', function (done) { + SERVER.get('/foo', function (req, res, next) { + assert.deepEqual(req.path(), '/foo'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000' + }); + CLIENT.get({ + path: '/foo', + appendPath: true + }, done); + }); + + it('should dedupe url slashes', function (done) { + SERVER.get('/foo', function (req, res, next) { + assert.deepEqual(req.path(), '/foo'); + res.send(200); + return next(); + }); + + CLIENT = clients.createJsonClient({ + url: 'http://localhost:3000//' + }); + CLIENT.get({ + path: '//foo', + appendPath: true + }, done); + }); + }); +}); diff --git a/test/e2e.js b/test/e2e.js index 90f91cc..36698df 100644 --- a/test/e2e.js +++ b/test/e2e.js @@ -8,8 +8,9 @@ var clients = require('../lib'); describe('restify-client tests against real web server', function () { it('have timings', function (done) { - var client = clients.createJsonClient({ - url: 'https://netflix.com' + this.timeout(10000); + var client = clients.createStringClient({ + url: 'https://www.netflix.com' }); client.get('/', function (err, req, res) {