From 431476922621aa9b497d5293e41a4c011a1a394f Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Sat, 11 May 2013 23:58:12 -0700 Subject: [PATCH 01/22] Initial request replacement --- docs/Reference.md | 2 - lib/client.js | 110 ++++++++++++++++++++++++++++++++++++++ lib/proxy.js | 70 +++++++++++------------- test/integration/proxy.js | 21 +------- 4 files changed, 144 insertions(+), 59 deletions(-) create mode 100755 lib/client.js diff --git a/docs/Reference.md b/docs/Reference.md index b20721f99..cc739b605 100755 --- a/docs/Reference.md +++ b/docs/Reference.md @@ -375,8 +375,6 @@ The following options are available when adding a route: - `settings` - the proxy handler configuration. - `res` - the node response object received from the upstream service. - `payload` - the response payload. - - `httpClient` - an alternative HTTP client function, compatible with the [**request**](https://npmjs.org/package/request) module `request()` - interface.

- `view` - generates a template-based response. The `view` options is set to the desired template file name. The view context available to the template includes: diff --git a/lib/client.js b/lib/client.js new file mode 100755 index 000000000..2debdb1be --- /dev/null +++ b/lib/client.js @@ -0,0 +1,110 @@ +// Load modules + +var Url = require('url'); +var Http = require('http'); +var Https = require('https'); +var Stream = require('stream'); +var Utils = require('./utils'); +var Boom = require('boom'); + + +// Declare internals + +var internals = {}; + + +// Create and configure server instance + +exports.request = function (method, url, options, callback) { + + var uri = Url.parse(url); + uri.method = method.toUpperCase(); + uri.headers = options.headers; + + var agent = (uri.protocol === 'https:' ? Https : Http); + var req = agent.request(uri); + + var isFinished = false; + var finish = function (err, res) { + + if (!isFinished) { + isFinished = true; + + req.removeAllListeners(); + + return callback(err, res); + } + }; + + req.once('error', finish); + + req.once('response', function (res) { + + // res.statusCode + // res.headers + // res Readable stream + + return finish(null, res); + }); + + if (uri.method !== 'GET' && + uri.method !== 'HEAD' && + options.payload !== null && + options.payload !== undefined) { // Value can be falsey + + if (options.payload instanceof Stream) { + options.payload.pipe(req); + return; + } + + req.write(options.payload); + } + + req.end(); +}; + + +exports.parse = function (res, callback) { + + var Writer = function () { + + Stream.Writable.call(this); + this.buffers = []; + this.length = 0; + + return this; + }; + + Utils.inherits(Writer, Stream.Writable); + + Writer.prototype._write = function (chunk, encoding, next) { + + this.legnth += chunk.length; + this.buffers.push(chunk); + next(); + }; + + var isFinished = false; + var finish = function (err, buffer) { + + if (!isFinished) { + isFinished = true; + + writer.removeAllListeners(); + res.removeAllListeners(); + + return callback(err || (buffer ? null : Boom.internal('Client request closed')), buffer); + } + }; + + var writer = new Writer(); + writer.once('finish', function () { + + return finish(null, Buffer.concat(writer.buffers, writer.length)); + }); + + res.once('error', finish); + res.once('close', finish); + + res.pipe(writer); +}; diff --git a/lib/proxy.js b/lib/proxy.js index 3b034a57a..c9c207363 100755 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -1,8 +1,8 @@ // Load modules -var Request = require('request'); -var Utils = require('./utils'); var Boom = require('boom'); +var Client = require('./client'); +var Utils = require('./utils'); // Declare internals @@ -32,9 +32,6 @@ exports = module.exports = internals.Proxy = function (options, route) { }; -internals.Proxy.prototype.httpClient = Request; - - internals.Proxy.prototype.handler = function () { var self = this; @@ -50,10 +47,8 @@ internals.Proxy.prototype.handler = function () { var req = request.raw.req; var options = { - uri: uri, - method: request.method, headers: {}, - jar: false + payload: null }; if (self.settings.passThrough) { // Never set with cache @@ -71,46 +66,45 @@ internals.Proxy.prototype.handler = function () { options.headers['x-forwarded-proto'] = (options.headers['x-forwarded-proto'] ? options.headers['x-forwarded-proto'] + ',' : '') + self.settings.protocol; } - var isGet = (request.method === 'get' || request.method === 'head'); - - // Parsed payload interface - - if (self.settings.isCustomPostResponse || // Custom response method - (isGet && request.route.cache.mode.server)) { // GET/HEAD with Cache - - delete options.headers['accept-encoding']; // Remove until Request supports unzip/deflate - self.httpClient(options, function (err, res, payload) { - - // Request handles all redirect responses (3xx) and will return an err if redirection fails - - if (err) { - return request.reply(Boom.internal('Proxy error', err)); - } - - return self.settings.postResponse(request, self.settings, res, payload); - }); - - return; + var isParsed = (self.settings.isCustomPostResponse || request.route.cache.mode.server); + if (isParsed) { + delete options.headers['accept-encoding']; } - // Streamed payload interface + // Set payload - if (!isGet && - request.rawPayload && + if (request.rawPayload && request.rawPayload.length) { options.headers['Content-Type'] = req.headers['content-type']; - options.body = request.rawPayload; + options.payload = request.rawPayload; + } + else { + options.payload = request.raw.req; } - var reqStream = self.httpClient(options); - reqStream.once('response', request.reply); // Request._respond will pass-through headers and status code + // Send request - if (!isGet && - !request.rawPayload) { + Client.request(request.method, uri, options, function (err, res) { - request.raw.req.pipe(reqStream); - } + if (err) { + console.log(err); + return request.reply(Boom.internal('Proxy error', err)); + } + + if (!isParsed) { + return request.reply(res); // Request._respond will pass-through headers and status code + } + + Client.parse(res, function (err, buffer) { + + if (err) { + return request.reply(err); + } + + return self.settings.postResponse(request, self.settings, res, buffer); + }); + }); }); }; }; diff --git a/test/integration/proxy.js b/test/integration/proxy.js index cabdf96f0..2c7c7996d 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -154,8 +154,8 @@ describe('Proxy', function () { { method: 'GET', path: '/noHeaders', handler: { proxy: { host: 'localhost', port: backendPort } } }, { method: 'GET', path: '/gzip', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, { method: 'GET', path: '/gzipstream', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, - { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://google.com'); } } } } - ]); + { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } } + ]); server.state('auto', { autoValue: 'xyz' }); server.start(function () { @@ -324,23 +324,6 @@ describe('Proxy', function () { }); }); - it('handles an error from request safely', function (done) { - - var requestStub = function (options, callback) { - - callback(new Error()); - }; - - var route = server._router.route({ method: 'get', path: '/proxyerror', info: {}, raw: { req: { headers: {} } } }); - route.proxy.httpClient = requestStub; - - makeRequest({ path: '/proxyerror', method: 'get' }, function (rawRes) { - - expect(rawRes.statusCode).to.equal(500); - done(); - }); - }); - it('forwards on a POST body', function (done) { makeRequest({ path: '/echo', method: 'post', form: { echo: true } }, function (rawRes) { From 6cf4d28185bd8e97d752f4744fb59a4076ce56c2 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 00:20:39 -0700 Subject: [PATCH 02/22] Redirection --- docs/Reference.md | 7 +- examples/proxy.js | 2 +- lib/client.js | 128 ++++++++++++++++++++++------ lib/proxy.js | 25 ++++-- lib/request.js | 20 ++--- lib/response/stream.js | 1 - package.json | 4 +- test/integration/cache.js | 15 ---- test/integration/proxy.js | 161 +++++++++++++++++++---------------- test/integration/request.js | 3 +- test/integration/response.js | 17 ---- 11 files changed, 226 insertions(+), 157 deletions(-) diff --git a/docs/Reference.md b/docs/Reference.md index cc739b605..75d7f99d0 100755 --- a/docs/Reference.md +++ b/docs/Reference.md @@ -361,6 +361,10 @@ The following options are available when adding a route: - `passThrough` - if `true`, forwards the headers sent from the client to the upstream service being proxied to. Defaults to `false`. - `xforward` - if `true`, sets the 'X-Forwarded-For', 'X-Forwarded-Port', 'X-Forwarded-Proto' headers when making a request to the proxied upstream endpoint. Defaults to `false`. + - `redirects` - the maximum number of HTTP redirections allowed, to be followed automatically by the handler. Set to `false` or `0` to + disable all redirections (the response will contain the redirection received from the upstream service). If redirections are enabled, + no redirections (301, 302, 307, 308) will be passed along to the client, and reaching the maximum allowed redirections will return an + error response. Defaults to `false`. - `mapUri` - a function used to map the request URI to the proxied URI. Cannot be used together with `host`, `port`, or `protocol`. The function signature is `function(request, callback)` where: - `request` - is the incoming `request` object @@ -1403,7 +1407,8 @@ Each request object have the following properties: - `id` - a unique request identifier. - `info` - request information: - `received` - request reception timestamp. - - `address` - remote client IP address. + - `remoteAddress` - remote client IP address. + - `remotePort` - remote client port. - `referrer` - content of the HTTP 'Referrer' (or 'Referer') header. - `host` - content of the HTTP 'Host' header. - `method` - the request method in lower case (e.g. `'get'`, `'post'`). diff --git a/examples/proxy.js b/examples/proxy.js index c1a47dc3c..6fcfed314 100755 --- a/examples/proxy.js +++ b/examples/proxy.js @@ -17,7 +17,7 @@ internals.main = function () { callback(null, 'http://www.google.com/search?q=' + request.params.term); }; - server.route({ method: '*', path: '/{p*}', handler: { proxy: { host: 'google.com', port: 80 } } }); + server.route({ method: '*', path: '/{p*}', handler: { proxy: { host: 'google.com', port: 80, redirects: 5 } } }); server.route({ method: 'GET', path: '/hapi/{term}', handler: { proxy: { mapUri: mapper } } }); server.start(); }; diff --git a/lib/client.js b/lib/client.js index 2debdb1be..987e0aad8 100755 --- a/lib/client.js +++ b/lib/client.js @@ -15,15 +15,25 @@ var internals = {}; // Create and configure server instance -exports.request = function (method, url, options, callback) { +exports.request = function (method, url, options, callback, _trace) { + + // Setup request var uri = Url.parse(url); uri.method = method.toUpperCase(); uri.headers = options.headers; + var redirects = (options.hasOwnProperty('redirects') ? options.redirects : false); // Needed to allow 0 as valid value when passed recursively + + _trace = (_trace || []); + _trace.push({ method: uri.method, url: url }); var agent = (uri.protocol === 'https:' ? Https : Http); var req = agent.request(uri); + var shadow = null; // A copy of the streamed request payload when redirects are enabled + + // Register handlers + var isFinished = false; var finish = function (err, res) { @@ -36,17 +46,45 @@ exports.request = function (method, url, options, callback) { } }; - req.once('error', finish); + req.once('error', function (err) { + + return finish(Boom.internal('Proxy error', err)); + }); req.once('response', function (res) { // res.statusCode // res.headers - // res Readable stream + // res (Readable stream) + + if (redirects !== false && + [301, 302, 307, 308].indexOf(res.statusCode) !== -1) { + + if (redirects === 0) { + return finish(Boom.internal('Maximum redirections reached', _trace)); + } + + var redirectMethod = (res.statusCode === 301 || res.statusCode === 302 ? 'GET' : uri.method); + var location = res.headers.location; + + if (!location) { + return finish(Boom.internal('Received redirection without location', _trace)); + } + + var redirectOptions = { + headers: options.headers, + payload: shadow || options.payload, // shadow must be ready at this point if set + redirects: --redirects + }; + + return exports.request(redirectMethod, location, redirectOptions, finish, _trace); + } return finish(null, res); }); + // Write payload + if (uri.method !== 'GET' && uri.method !== 'HEAD' && options.payload !== null && @@ -54,36 +92,30 @@ exports.request = function (method, url, options, callback) { if (options.payload instanceof Stream) { options.payload.pipe(req); + + if (redirects) { + var collector = new internals.Collector(function () { + + shadow = collector.collect(); + }); + + options.payload.pipe(collector); + } + return; } req.write(options.payload); } + // Finalize request + req.end(); }; exports.parse = function (res, callback) { - var Writer = function () { - - Stream.Writable.call(this); - this.buffers = []; - this.length = 0; - - return this; - }; - - Utils.inherits(Writer, Stream.Writable); - - Writer.prototype._write = function (chunk, encoding, next) { - - this.legnth += chunk.length; - this.buffers.push(chunk); - next(); - }; - var isFinished = false; var finish = function (err, buffer) { @@ -93,18 +125,60 @@ exports.parse = function (res, callback) { writer.removeAllListeners(); res.removeAllListeners(); - return callback(err || (buffer ? null : Boom.internal('Client request closed')), buffer); + return callback(err, buffer); } }; - var writer = new Writer(); - writer.once('finish', function () { + res.once('error', function (err) { - return finish(null, Buffer.concat(writer.buffers, writer.length)); + return finish(Boom.internal('Proxy response error', err)); }); - res.once('error', finish); - res.once('close', finish); + res.once('close', function () { + + return finish(Boom.internal('Client request closed')); + }); + + var writer = new internals.Collector(function () { + + return finish(null, writer.collect()); + }); res.pipe(writer); }; + + +internals.Collector = function (options, callback) { + + var self = this; + + if (!callback) { + callback = options; + options = {} + } + + Stream.Writable.call(this); + this.buffers = []; + this.length = 0; + + this.once('finish', callback); + + return this; +}; + +Utils.inherits(internals.Collector, Stream.Writable); + + +internals.Collector.prototype._write = function (chunk, encoding, next) { + + this.legnth += chunk.length; + this.buffers.push(chunk); + next(); +}; + + +internals.Collector.prototype.collect = function () { + + var buffer = (this.buffers.length === 0 ? new Buffer(0) : (this.buffers.length === 1 ? this.buffers[0] : Buffer.concat(this.buffers, this.length))); + return buffer; +}; diff --git a/lib/proxy.js b/lib/proxy.js index c9c207363..bf437297d 100755 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -20,6 +20,7 @@ exports = module.exports = internals.Proxy = function (options, route) { Utils.assert(!options.mapUri || typeof options.mapUri === 'function', 'options.mapUri must be a function'); Utils.assert(!options.postResponse || typeof options.postResponse === 'function', 'options.postResponse must be a function'); Utils.assert(!options.hasOwnProperty('isCustomPostResponse'), 'Cannot manually set options.isCustomPostResponse'); + Utils.assert(!options.redirects || (typeof options.redirects === 'number' && options.redirects >= 0), 'Proxy redirects must false or a positive number'); this.settings = {}; this.settings.mapUri = options.mapUri || internals.mapUri(options.protocol, options.host, options.port); @@ -27,6 +28,7 @@ exports = module.exports = internals.Proxy = function (options, route) { this.settings.passThrough = options.passThrough || false; this.settings.isCustomPostResponse = !!options.postResponse; this.settings.postResponse = options.postResponse || internals.postResponse; // function (request, settings, response, payload) + this.settings.redirects = options.redirects || false; return this; }; @@ -48,7 +50,8 @@ internals.Proxy.prototype.handler = function () { var options = { headers: {}, - payload: null + payload: null, + redirects: self.settings.redirects }; if (self.settings.passThrough) { // Never set with cache @@ -61,8 +64,8 @@ internals.Proxy.prototype.handler = function () { } if (self.settings.xforward) { - options.headers['x-forwarded-for'] = (options.headers['x-forwarded-for'] ? options.headers['x-forwarded-for'] + ',' : '') + req.connection.remoteAddress || req.socket.remoteAddress; - options.headers['x-forwarded-port'] = (options.headers['x-forwarded-port'] ? options.headers['x-forwarded-port'] + ',' : '') + req.connection.remotePort || req.socket.remotePort; + options.headers['x-forwarded-for'] = (options.headers['x-forwarded-for'] ? options.headers['x-forwarded-for'] + ',' : '') + request.info.remoteAddress; + options.headers['x-forwarded-port'] = (options.headers['x-forwarded-port'] ? options.headers['x-forwarded-port'] + ',' : '') + request.info.remotePort; options.headers['x-forwarded-proto'] = (options.headers['x-forwarded-proto'] ? options.headers['x-forwarded-proto'] + ',' : '') + self.settings.protocol; } @@ -76,7 +79,11 @@ internals.Proxy.prototype.handler = function () { if (request.rawPayload && request.rawPayload.length) { - options.headers['Content-Type'] = req.headers['content-type']; + var contentType = req.headers['content-type']; + if (contentType) { + options.headers['Content-Type'] = contentType; + } + options.payload = request.rawPayload; } else { @@ -88,14 +95,15 @@ internals.Proxy.prototype.handler = function () { Client.request(request.method, uri, options, function (err, res) { if (err) { - console.log(err); - return request.reply(Boom.internal('Proxy error', err)); + return request.reply(err); } if (!isParsed) { return request.reply(res); // Request._respond will pass-through headers and status code } + // Parse payload for caching or post-processing + Client.parse(res, function (err, buffer) { if (err) { @@ -126,10 +134,9 @@ internals.mapUri = function (protocol, host, port) { internals.postResponse = function (request, settings, res, payload) { var contentType = res.headers['content-type']; - var statusCode = res.statusCode; - if (statusCode >= 400) { - return request.reply(Boom.passThrough(statusCode, payload, contentType)); + if (res.statusCode !== 200) { + return request.reply(Boom.passThrough(res.statusCode, payload, contentType)); } var response = request.reply(payload); diff --git a/lib/request.js b/lib/request.js index f46b91db6..6c5612969 100755 --- a/lib/request.js +++ b/lib/request.js @@ -54,11 +54,14 @@ exports = module.exports = internals.Request = function (server, req, res, optio this.pre = {}; this.info = { received: now, - address: (req.connection && req.connection.remoteAddress) || '', + remoteAddress: (req.connection && req.connection.remoteAddress) || '', + remotePort: (req.connection && req.connection.remotePort) || '', referrer: req.headers.referrer || req.headers.referer || '', host: req.headers.host ? req.headers.host.replace(/\s/g, '') : '' }; + this.info.address = this.info.remoteAddress; // Backwards compatibility + // Apply options if (options.credentials) { @@ -457,7 +460,6 @@ internals.Request.prototype._replyInterface = function (next, withProperties) { delete self.reply; - Utils.assert(!self.route.cache.mode.server || result instanceof Stream === false, 'Cannot reply using a stream when caching enabled'); response = Response._generate(result, process); return response; }; @@ -466,15 +468,13 @@ internals.Request.prototype._replyInterface = function (next, withProperties) { return reply; } - if (!this.route.cache.mode.server) { - reply.close = function () { + reply.close = function () { - delete self.reply; + delete self.reply; - response = new Closed(); - process(); - }; - } + response = new Closed(); + process(); + }; // Chain initializers @@ -635,7 +635,7 @@ internals.handler = function (request, next) { } request._log(['handler'], { msec: timer.elapsed() }); - return exit(null, response); + return exit(null, response, !response.varieties.cacheable); }; // Execute handler diff --git a/lib/response/stream.js b/lib/response/stream.js index 80a481e03..b603c67e6 100755 --- a/lib/response/stream.js +++ b/lib/response/stream.js @@ -63,7 +63,6 @@ internals.Stream.prototype._setStream = function (stream) { // Support pre node v0.10 streams API if (stream.pipe === Stream.prototype.pipe) { - var oldStream = stream; oldStream.pause(); stream = new Stream.Readable().wrap(oldStream); diff --git a/package.json b/package.json index 16b363758..3bcf85fb3 100755 --- a/package.json +++ b/package.json @@ -32,13 +32,12 @@ "hoek": "0.8.x", "boom": "0.4.x", "joi": "0.3.x", - "catbox": "0.5.x", + "catbox": "0.6.x", "hawk": "0.13.x", "shot": "0.4.x", "cryptiles": "0.2.x", "iron": "0.3.x", "async": "0.2.x", - "request": "2.21.x", "formidable": "1.0.13", "mime": "1.2.x", "lru-cache": "2.3.x", @@ -48,6 +47,7 @@ }, "devDependencies": { "lab": "0.1.x", + "request": "2.21.x", "handlebars": "1.0.x", "jade": "0.30.x", "complexity-report": "0.x.x" diff --git a/test/integration/cache.js b/test/integration/cache.js index d0eb22096..5eed23941 100755 --- a/test/integration/cache.js +++ b/test/integration/cache.js @@ -47,11 +47,6 @@ describe('Cache', function () { request.reply(cacheable); }; - var badHandler = function (request) { - - request.reply(new Stream()); - }; - var errorHandler = function (request) { var error = new Error('myerror'); @@ -69,7 +64,6 @@ describe('Cache', function () { { method: 'GET', path: '/item', config: { handler: activeItemHandler, cache: { expiresIn: 120000 } } }, { method: 'GET', path: '/item2', config: { handler: activeItemHandler } }, { method: 'GET', path: '/item3', config: { handler: activeItemHandler, cache: { expiresIn: 120000 } } }, - { method: 'GET', path: '/bad', config: { handler: badHandler, cache: { mode: 'client+server', expiresIn: 120000 } } }, { method: 'GET', path: '/cache', config: { handler: cacheItemHandler, cache: { mode: 'client+server', expiresIn: 120000 } } }, { method: 'GET', path: '/error', config: { handler: errorHandler, cache: { mode: 'client+server', expiresIn: 120000 } } }, { method: 'GET', path: '/clientserver', config: { handler: profileHandler, cache: { mode: 'client+server', expiresIn: 120000 } } }, @@ -108,15 +102,6 @@ describe('Cache', function () { }); }); - it('returns 500 when returning a stream in a cached endpoint handler', function (done) { - - server.inject('/bad', function (res) { - - expect(res.statusCode).to.equal(500); - done(); - }); - }); - it('doesn\'t cache error responses', function (done) { server.inject('/error', function () { diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 2c7c7996d..f41e0427c 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -2,8 +2,8 @@ var Lab = require('lab'); var Fs = require('fs'); -var Request = require('request'); var Zlib = require('zlib'); +var Request = require('request'); var Hapi = require('../..'); @@ -27,8 +27,6 @@ describe('Proxy', function () { before(function (done) { - // Define backend handlers - var mapUriWithError = function (request, callback) { return callback(new Error('myerror')); @@ -123,7 +121,10 @@ describe('Proxy', function () { { method: 'GET', path: '/noHeaders', handler: headers }, { method: 'GET', path: '/forward', handler: forward }, { method: 'GET', path: '/gzip', handler: gzipHandler }, - { method: 'GET', path: '/gzipstream', handler: gzipStreamHandler } + { method: 'GET', path: '/gzipstream', handler: gzipStreamHandler }, + { method: 'GET', path: '/redirect', handler: function () { this.reply.redirect(this.query.x ? '/redirect?x=1' : '/profile'); } }, + { method: 'POST', path: '/post1', handler: function () { this.reply.redirect('/post2').rewritable(false); } }, + { method: 'POST', path: '/post2', handler: function () { this.reply(this.payload); } } ]); var mapUri = function (request, callback) { @@ -154,7 +155,9 @@ describe('Proxy', function () { { method: 'GET', path: '/noHeaders', handler: { proxy: { host: 'localhost', port: backendPort } } }, { method: 'GET', path: '/gzip', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, { method: 'GET', path: '/gzipstream', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, - { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } } + { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } }, + { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } }, + { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 10 } }, config: { payload: 'parse' } } ]); server.state('auto', { autoValue: 'xyz' }); @@ -165,38 +168,30 @@ describe('Proxy', function () { }); }); - function makeRequest(options, callback) { - - options = options || {}; - options.path = options.path || '/'; - options.method = options.method || 'get'; + it('can add a proxy route with a http protocol set', function (done) { - var req = { - method: options.method, - url: server.info.uri + options.path, - form: options.form, - headers: options.headers, - jar: false - }; + server.route({ method: 'GET', path: '/httpport', handler: { proxy: { host: 'localhost', protocol: 'http' } } }); + done(); + }); - Request(req, function (err, res) { + it('can add a proxy route with a https protocol set', function (done) { - callback(res); - }); - } + server.route({ method: 'GET', path: '/httpsport', handler: { proxy: { host: 'localhost', protocol: 'https' } } }); + done(); + }); it('forwards on the response when making a GET request', function (done) { - makeRequest({ path: '/profile' }, function (rawRes) { + server.inject('/profile', function (res) { + + expect(res.statusCode).to.equal(200); + expect(res.payload).to.contain('John Doe'); + expect(res.headers['set-cookie']).to.deep.equal(['test=123', 'auto=xyz']); - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.contain('John Doe'); - expect(rawRes.headers['set-cookie']).to.deep.equal(['test=123','auto=xyz']); - - makeRequest({ path: '/profile' }, function (rawRes) { + server.inject('/profile', function (res) { - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.contain('John Doe'); + expect(res.statusCode).to.equal(200); + expect(res.payload).to.contain('John Doe'); done(); }); }); @@ -204,20 +199,20 @@ describe('Proxy', function () { it('forwards on x-forward headers', function (done) { - makeRequest({ path: '/forward', headers: { 'x-forwarded-for': 'xforwardfor', 'x-forwarded-port': '9000', 'x-forwarded-proto': 'xforwardproto' } }, function (rawRes) { + server.inject({ url: '/forward', headers: { 'x-forwarded-for': 'xforwardfor', 'x-forwarded-port': '9000', 'x-forwarded-proto': 'xforwardproto' } }, function (res) { - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.equal('Success'); + expect(res.statusCode).to.equal(200); + expect(res.payload).to.equal('Success'); done(); }); }); it('forwards upstream headers', function (done) { - server.inject({ url: '/headers', method: 'GET' }, function (res) { + server.inject('/headers', function (res) { expect(res.statusCode).to.equal(200); - expect(res.result).to.equal('{\"status\":\"success\"}'); + expect(res.payload).to.equal('{\"status\":\"success\"}'); expect(res.headers.custom1).to.equal('custom header value 1'); expect(res.headers['x-custom2']).to.equal('custom header value 2'); expect(res.headers['access-control-allow-headers']).to.equal('Authorization, Content-Type, If-None-Match'); @@ -230,10 +225,12 @@ describe('Proxy', function () { Zlib.gzip(new Buffer('123456789012345678901234567890123456789012345678901234567890'), function (err, zipped) { - makeRequest({ path: '/gzip', method: 'GET', headers: { 'accept-encoding': 'gzip' } }, function (res) { + expect(err).to.not.exist; + + server.inject({ url: '/gzip', headers: { 'accept-encoding': 'gzip' } }, function (res) { expect(res.statusCode).to.equal(200); - expect(res.body).to.equal(zipped.toString()); + expect(res.payload).to.equal(zipped.toString()); done(); }); }); @@ -241,7 +238,7 @@ describe('Proxy', function () { it('forwards gzipped stream', function (done) { - makeRequest({ path: '/gzipstream', method: 'GET', headers: { 'accept-encoding': 'gzip' } }, function (res) { + server.inject({ url: '/gzipstream', headers: { 'accept-encoding': 'gzip' } }, function (res) { expect(res.statusCode).to.equal(200); @@ -249,7 +246,7 @@ describe('Proxy', function () { Zlib.gzip(file, function (err, zipped) { - expect(zipped.toString()).to.equal(res.body); + expect(zipped.toString()).to.equal(res.payload); done(); }); }); @@ -258,117 +255,135 @@ describe('Proxy', function () { it('does not forward upstream headers without passThrough', function (done) { - makeRequest({ path: '/noHeaders' }, function (rawRes) { + server.inject('/noHeaders', function (res) { - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.equal('{\"status\":\"success\"}'); - expect(rawRes.headers.custom1).to.not.exist; - expect(rawRes.headers['x-custom2']).to.not.exist; + expect(res.statusCode).to.equal(200); + expect(res.payload).to.equal('{\"status\":\"success\"}'); + expect(res.headers.custom1).to.not.exist; + expect(res.headers['x-custom2']).to.not.exist; done(); }); }); it('forwards on the response when making a GET request to a route that also accepts a POST', function (done) { - makeRequest({ path: '/item' }, function (rawRes) { + server.inject('/item', function (res) { - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.contain('Active Item'); + expect(res.statusCode).to.equal(200); + expect(res.payload).to.contain('Active Item'); done(); }); }); it('forwards on the status code when making a POST request', function (done) { - makeRequest({ path: '/item', method: 'post' }, function (rawRes) { + server.inject({ url: '/item', method: 'POST' }, function (res) { - expect(rawRes.statusCode).to.equal(201); - expect(rawRes.body).to.contain('Item'); + expect(res.statusCode).to.equal(201); + expect(res.payload).to.contain('Item'); done(); }); }); it('sends the correct status code with a request is unauthorized', function (done) { - makeRequest({ path: '/unauthorized', method: 'get' }, function (rawRes) { + server.inject('/unauthorized', function (res) { - expect(rawRes.statusCode).to.equal(401); + expect(res.statusCode).to.equal(401); done(); }); }); it('sends a 404 status code with a proxied route doesn\'t exist', function (done) { - makeRequest({ path: '/notfound', method: 'get' }, function (rawRes) { + server.inject('/notfound', function (res) { - expect(rawRes.statusCode).to.equal(404); + expect(res.statusCode).to.equal(404); done(); }); }); it('forwards on the status code when a custom postResponse returns an error', function (done) { - makeRequest({ path: '/postResponseError', method: 'get' }, function (rawRes) { + server.inject('/postResponseError', function (res) { - expect(rawRes.statusCode).to.equal(403); + expect(res.statusCode).to.equal(403); done(); }); }); it('forwards the error message with a custom postResponse and a route error', function (done) { - makeRequest({ path: '/postResponseNotFound', method: 'get' }, function (rawRes) { + server.inject('/postResponseNotFound', function (res) { - expect(rawRes.body).to.contain('error'); + expect(res.payload).to.contain('error'); done(); }); }); it('forwards on a POST body', function (done) { - makeRequest({ path: '/echo', method: 'post', form: { echo: true } }, function (rawRes) { + server.inject({ url: '/echo', method: 'POST', payload: '{"echo":true}' }, function (res) { - expect(rawRes.statusCode).to.equal(200); - expect(rawRes.body).to.equal('true@'); + expect(res.statusCode).to.equal(200); + expect(res.payload).to.equal('true@'); done(); }); }); it('replies with an error when it occurs in mapUri', function (done) { - makeRequest({ path: '/maperror', method: 'get' }, function (rawRes) { + server.inject('/maperror', function (res) { - expect(rawRes.body).to.contain('myerror'); + expect(res.payload).to.contain('myerror'); done(); }); }); it('works with a stream when the proxy response is streamed', function (done) { - Fs.createReadStream(__dirname + '/proxy.js').pipe(Request.post(server.info.uri + '/file', function (err, rawRes, body) { + Fs.createReadStream(__dirname + '/proxy.js').pipe(Request.post(server.info.uri + '/file', function (err, res, body) { - expect(rawRes.statusCode).to.equal(200); + expect(res.statusCode).to.equal(200); done(); })); }); - it('can add a proxy route with a http protocol set', function (done) { + it('proxies to a remote site', function (done) { - server.route({ method: 'GET', path: '/httpport', handler: { proxy: { host: 'localhost', protocol: 'http' } } }); - done(); + server.inject('/google', function (res) { + + expect(res.statusCode).to.equal(200); + done(); + }); }); - it('can add a proxy route with a https protocol set', function (done) { + it('redirects to another endpoint', function (done) { - server.route({ method: 'GET', path: '/httpsport', handler: { proxy: { host: 'localhost', protocol: 'https' } } }); - done(); + server.inject('/redirect', function (res) { + + expect(res.statusCode).to.equal(200); + expect(res.payload).to.contain('John Doe'); + expect(res.headers['set-cookie']).to.deep.equal(['test=123', 'auto=xyz']); + done(); + }); }); - it('proxies to a remote site', function (done) { + it('maxs out redirects to another endpoint', function (done) { - makeRequest({ path: '/google' }, function (rawRes) { + server.inject('/redirect?x=1', function (res) { - expect(rawRes.statusCode).to.equal(200); + expect(res.statusCode).to.equal(500); + done(); + }); + }); + + it('redirects to a post endpoint with stream', function (done) { + + server.inject({ method: 'POST', url: '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { + + expect(res.statusCode).to.equal(200); + expect(res.payload).to.equal('test'); done(); }); }); diff --git a/test/integration/request.js b/test/integration/request.js index 06d46a261..c2ca5c2f2 100755 --- a/test/integration/request.js +++ b/test/integration/request.js @@ -344,7 +344,8 @@ describe('Request', function () { var handler = function (request) { - expect(request.info.address).to.equal('127.0.0.1'); + expect(request.info.remoteAddress).to.equal('127.0.0.1'); + expect(request.info.remoteAddress).to.equal(request.info.address); request.reply('ok'); }; diff --git a/test/integration/response.js b/test/integration/response.js index 50c57d46c..6e6c6a15f 100755 --- a/test/integration/response.js +++ b/test/integration/response.js @@ -1884,23 +1884,6 @@ describe('Response', function () { done(); }); }); - - it('returns 500 when using close() and route is cached', function (done) { - - var handler = function () { - - this.reply.close(); - }; - - var server = new Hapi.Server({ debug: false }); - server.route({ method: 'GET', path: '/', config: { handler: handler, cache: { mode: 'server', expiresIn: 9999 } } }); - - server.inject('/', function (res) { - - expect(res.statusCode).to.equal(500); - done(); - }); - }); }); describe('Extension', function () { From 6fd4a52a84ec8373beb6a952f649bbab7d09e095 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 00:25:06 -0700 Subject: [PATCH 03/22] Remove stream hack for handling Request --- lib/response/stream.js | 14 +++++--------- test/integration/response.js | 12 ++---------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/lib/response/stream.js b/lib/response/stream.js index b603c67e6..3cc49d106 100755 --- a/lib/response/stream.js +++ b/lib/response/stream.js @@ -46,18 +46,14 @@ internals.Stream.prototype._setStream = function (stream) { return; } - // Check if stream is a node HTTP response (stream.*) or a (mikeal's) Request object (stream.response.*) + // Check if stream is a node HTTP response - if (stream.statusCode || - (stream.response && stream.response.statusCode)) { - - this._passThrough.code = stream.statusCode || stream.response.statusCode; + if (stream.statusCode) { + this._passThrough.code = stream.statusCode; } - if (stream.headers || - (stream.response && stream.response.headers)) { - - this._passThrough.headers = stream.headers || stream.response.headers; + if (stream.headers) { + this._passThrough.headers = stream.headers; } // Support pre node v0.10 streams API diff --git a/test/integration/response.js b/test/integration/response.js index 6e6c6a15f..dd33d139b 100755 --- a/test/integration/response.js +++ b/test/integration/response.js @@ -1079,12 +1079,7 @@ describe('Response', function () { var HeadersStream = function () { Stream.Readable.call(this); - this.response = { - headers: { - custom: 'header' - } - }; - + this.headers = { custom: 'header' }; return this; }; @@ -1123,10 +1118,7 @@ describe('Response', function () { var HeadersStream = function () { Stream.Readable.call(this); - this.response = { - statusCode: 201 - }; - + this.statusCode = 201; return this; }; From 78f074b4624bd288627d9cca3ee3ae5076e0e98a Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 11:11:03 -0500 Subject: [PATCH 04/22] Using Request for proxy tests --- lib/client.js | 4 ++-- test/integration/proxy.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/client.js b/lib/client.js index 987e0aad8..ffb96f901 100755 --- a/lib/client.js +++ b/lib/client.js @@ -154,7 +154,7 @@ internals.Collector = function (options, callback) { if (!callback) { callback = options; - options = {} + options = {}; } Stream.Writable.call(this); @@ -171,7 +171,7 @@ Utils.inherits(internals.Collector, Stream.Writable); internals.Collector.prototype._write = function (chunk, encoding, next) { - this.legnth += chunk.length; + this.length += chunk.length; this.buffers.push(chunk); next(); }; diff --git a/test/integration/proxy.js b/test/integration/proxy.js index f41e0427c..c4420b444 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -238,7 +238,7 @@ describe('Proxy', function () { it('forwards gzipped stream', function (done) { - server.inject({ url: '/gzipstream', headers: { 'accept-encoding': 'gzip' } }, function (res) { + Request({ uri: server.info.uri + '/gzipstream', headers: { 'accept-encoding': 'gzip' }}, function (err, res) { expect(res.statusCode).to.equal(200); @@ -246,7 +246,7 @@ describe('Proxy', function () { Zlib.gzip(file, function (err, zipped) { - expect(zipped.toString()).to.equal(res.payload); + expect(zipped.toString()).to.equal(res.body); done(); }); }); @@ -351,7 +351,7 @@ describe('Proxy', function () { it('proxies to a remote site', function (done) { - server.inject('/google', function (res) { + Request(server.info.uri + '/google', function (err, res) { expect(res.statusCode).to.equal(200); done(); @@ -380,7 +380,7 @@ describe('Proxy', function () { it('redirects to a post endpoint with stream', function (done) { - server.inject({ method: 'POST', url: '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { + Request({ method: 'POST', uri: server.info.uri + '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (err, res) { expect(res.statusCode).to.equal(200); expect(res.payload).to.equal('test'); From c9d2f385fca7819dd7784b37964d834a5e158350 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 11:13:33 -0500 Subject: [PATCH 05/22] Reverting changed test --- test/integration/proxy.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index c4420b444..9a8c8192f 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -93,9 +93,9 @@ describe('Proxy', function () { var headers = function () { this.reply({ status: 'success' }) - .header('Custom1', 'custom header value 1') - .header('X-Custom2', 'custom header value 2') - .header('access-control-allow-headers', 'Invalid, List, Of, Values'); + .header('Custom1', 'custom header value 1') + .header('X-Custom2', 'custom header value 2') + .header('access-control-allow-headers', 'Invalid, List, Of, Values'); }; var gzipHandler = function () { @@ -238,7 +238,7 @@ describe('Proxy', function () { it('forwards gzipped stream', function (done) { - Request({ uri: server.info.uri + '/gzipstream', headers: { 'accept-encoding': 'gzip' }}, function (err, res) { + server.inject({ url: '/gzipstream', headers: { 'accept-encoding': 'gzip' } }, function (res) { expect(res.statusCode).to.equal(200); @@ -246,7 +246,7 @@ describe('Proxy', function () { Zlib.gzip(file, function (err, zipped) { - expect(zipped.toString()).to.equal(res.body); + expect(zipped.toString()).to.equal(res.payload); done(); }); }); @@ -351,7 +351,7 @@ describe('Proxy', function () { it('proxies to a remote site', function (done) { - Request(server.info.uri + '/google', function (err, res) { + server.inject('/google', function (res) { expect(res.statusCode).to.equal(200); done(); @@ -380,7 +380,7 @@ describe('Proxy', function () { it('redirects to a post endpoint with stream', function (done) { - Request({ method: 'POST', uri: server.info.uri + '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (err, res) { + server.inject({ method: 'POST', url: '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { expect(res.statusCode).to.equal(200); expect(res.payload).to.equal('test'); From 4e4c71014b0f770e5b011a7dd929248369bed6f4 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 09:30:41 -0700 Subject: [PATCH 06/22] debugging --- lib/response/stream.js | 18 ++++++------------ test/integration/proxy.js | 12 +++++++++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/response/stream.js b/lib/response/stream.js index 3cc49d106..bdf4df559 100755 --- a/lib/response/stream.js +++ b/lib/response/stream.js @@ -140,17 +140,9 @@ internals.Stream.prototype._transmit = function (request, callback) { } }); - if (encoding === 'gzip') { - this._headers['Content-Encoding'] = 'gzip'; - this._headers.Vary = 'Accept-Encoding'; - encoder = Zlib.createGzip(); - } - - if (encoding === 'deflate') { - this._headers['Content-Encoding'] = 'deflate'; - this._headers.Vary = 'Accept-Encoding'; - encoder = Zlib.createDeflate(); - } + this._headers['Content-Encoding'] = encoding; + this._headers.Vary = 'Accept-Encoding'; + encoder = (encoding === 'gzip' ? Zlib.createGzip() : Zlib.createDeflate()); } } @@ -173,7 +165,9 @@ internals.Stream.prototype._transmit = function (request, callback) { self._preview.removeAllListeners(); self._stream.removeAllListeners(); - self._stream.destroy && self._stream.destroy(); + if (self._stream.destroy) { + self._stream.destroy(); + } callback(); } diff --git a/test/integration/proxy.js b/test/integration/proxy.js index f41e0427c..269d9cf05 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -156,6 +156,7 @@ describe('Proxy', function () { { method: 'GET', path: '/gzip', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, { method: 'GET', path: '/gzipstream', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true } } }, { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } }, + { method: 'GET', path: '/googler', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://google.com'); }, redirects: 1 } } }, { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } }, { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 10 } }, config: { payload: 'parse' } } ]); @@ -230,7 +231,7 @@ describe('Proxy', function () { server.inject({ url: '/gzip', headers: { 'accept-encoding': 'gzip' } }, function (res) { expect(res.statusCode).to.equal(200); - expect(res.payload).to.equal(zipped.toString()); + expect(res.rawPayload).to.deep.equal(zipped); done(); }); }); @@ -358,6 +359,15 @@ describe('Proxy', function () { }); }); + it('proxies to a remote site with redirects', function (done) { + + server.inject('/googler', function (res) { + + expect(res.statusCode).to.equal(200); + done(); + }); + }); + it('redirects to another endpoint', function (done) { server.inject('/redirect', function (res) { From 3599ea36e95f81a56f62bd267149df3e59c51c8d Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 11:44:51 -0500 Subject: [PATCH 07/22] Fixing test --- test/integration/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 9a8c8192f..6d105be83 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -380,7 +380,7 @@ describe('Proxy', function () { it('redirects to a post endpoint with stream', function (done) { - server.inject({ method: 'POST', url: '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { + server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { expect(res.statusCode).to.equal(200); expect(res.payload).to.equal('test'); From 1159b757a0c4e6272ad2bcd5039800f1a6d540df Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 11:28:49 -0700 Subject: [PATCH 08/22] fix test --- test/integration/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 91c34ef50..1e125fd83 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -390,7 +390,7 @@ describe('Proxy', function () { it('redirects to a post endpoint with stream', function (done) { - server.inject({ method: 'POST', url: '/post1', body: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { + server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { expect(res.statusCode).to.equal(200); expect(res.payload).to.equal('test'); From 3aa11634be21e68e8dbf147dc1ea9360fc0a438e Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 14:41:32 -0500 Subject: [PATCH 09/22] Increasing allowed sockets to 10 for client --- lib/client.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index ffb96f901..510a3c398 100755 --- a/lib/client.js +++ b/lib/client.js @@ -22,15 +22,17 @@ exports.request = function (method, url, options, callback, _trace) { var uri = Url.parse(url); uri.method = method.toUpperCase(); uri.headers = options.headers; + var redirects = (options.hasOwnProperty('redirects') ? options.redirects : false); // Needed to allow 0 as valid value when passed recursively _trace = (_trace || []); _trace.push({ method: uri.method, url: url }); var agent = (uri.protocol === 'https:' ? Https : Http); + agent.globalAgent.maxSockets = 10; // Support 10 connections in the agent pool var req = agent.request(uri); - var shadow = null; // A copy of the streamed request payload when redirects are enabled + var shadow = null; // A copy of the streamed request payload when redirects are enabled // Register handlers @@ -104,8 +106,11 @@ exports.request = function (method, url, options, callback, _trace) { return; } + if (typeof options.payload === 'string' || + Buffer.isBuffer(options.payload)) { - req.write(options.payload); + req.write(options.payload); + } } // Finalize request From 04bbe17672744d32f7ea492838d2ea6d05c134a9 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 13:25:10 -0700 Subject: [PATCH 10/22] Fix socket pool drain --- lib/client.js | 45 ++++++++++++++++++------------------ test/integration/proxy.js | 48 +++++++++++++++++++-------------------- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/lib/client.js b/lib/client.js index ffb96f901..f52cd3ba0 100755 --- a/lib/client.js +++ b/lib/client.js @@ -53,34 +53,35 @@ exports.request = function (method, url, options, callback, _trace) { req.once('response', function (res) { - // res.statusCode - // res.headers - // res (Readable stream) + // Pass-through response - if (redirects !== false && - [301, 302, 307, 308].indexOf(res.statusCode) !== -1) { + if (redirects === false || + [301, 302, 307, 308].indexOf(res.statusCode) === -1) { - if (redirects === 0) { - return finish(Boom.internal('Maximum redirections reached', _trace)); - } + return finish(null, res); + } - var redirectMethod = (res.statusCode === 301 || res.statusCode === 302 ? 'GET' : uri.method); - var location = res.headers.location; + // Redirection - if (!location) { - return finish(Boom.internal('Received redirection without location', _trace)); - } + if (redirects === 0) { + return finish(Boom.internal('Maximum redirections reached', _trace)); + } - var redirectOptions = { - headers: options.headers, - payload: shadow || options.payload, // shadow must be ready at this point if set - redirects: --redirects - }; + var redirectMethod = (res.statusCode === 301 || res.statusCode === 302 ? 'GET' : uri.method); + var location = res.headers.location; - return exports.request(redirectMethod, location, redirectOptions, finish, _trace); + if (!location) { + return finish(Boom.internal('Received redirection without location', _trace)); } - return finish(null, res); + var redirectOptions = { + headers: options.headers, + payload: shadow || options.payload, // shadow must be ready at this point if set + redirects: --redirects + }; + + res.destroy(); + exports.request(redirectMethod, location, redirectOptions, finish, _trace); }); // Write payload @@ -154,7 +155,7 @@ internals.Collector = function (options, callback) { if (!callback) { callback = options; - options = {}; + options = {} } Stream.Writable.call(this); @@ -171,7 +172,7 @@ Utils.inherits(internals.Collector, Stream.Writable); internals.Collector.prototype._write = function (chunk, encoding, next) { - this.length += chunk.length; + this.legnth += chunk.length; this.buffers.push(chunk); next(); }; diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 1e125fd83..fcb264240 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -181,6 +181,24 @@ describe('Proxy', function () { done(); }); + it('proxies to a remote site', function (done) { + + server.inject('/google', function (res) { + + expect(res.statusCode).to.equal(200); + done(); + }); + }); + + it('proxies to a remote site with redirects', function (done) { + + server.inject('/googler', function (res) { + + expect(res.statusCode).to.equal(200); + done(); + }); + }); + it('forwards on the response when making a GET request', function (done) { server.inject('/profile', function (res) { @@ -350,20 +368,21 @@ describe('Proxy', function () { })); }); - it('proxies to a remote site', function (done) { + it('maxs out redirects to another endpoint', function (done) { - server.inject('/google', function (res) { + server.inject('/redirect?x=1', function (res) { - expect(res.statusCode).to.equal(200); + expect(res.statusCode).to.equal(500); done(); }); }); - it('proxies to a remote site with redirects', function (done) { + it('redirects to a post endpoint with stream', function (done) { - server.inject('/googler', function (res) { + server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { expect(res.statusCode).to.equal(200); + expect(res.payload).to.equal('test'); done(); }); }); @@ -378,23 +397,4 @@ describe('Proxy', function () { done(); }); }); - - it('maxs out redirects to another endpoint', function (done) { - - server.inject('/redirect?x=1', function (res) { - - expect(res.statusCode).to.equal(500); - done(); - }); - }); - - it('redirects to a post endpoint with stream', function (done) { - - server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { - - expect(res.statusCode).to.equal(200); - expect(res.payload).to.equal('test'); - done(); - }); - }); }); \ No newline at end of file From 025c23addf22b73a4df856dd756b5456112ca1f7 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 15:30:07 -0500 Subject: [PATCH 11/22] Adding global maxSockets setting --- lib/client.js | 8 +++----- lib/defaults.js | 11 ++++++----- lib/schema.js | 3 ++- lib/server.js | 15 +++++++++------ test/integration/proxy.js | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/lib/client.js b/lib/client.js index 510a3c398..d73cc1e9b 100755 --- a/lib/client.js +++ b/lib/client.js @@ -17,6 +17,8 @@ var internals = {}; exports.request = function (method, url, options, callback, _trace) { + Utils.assert(typeof options.payload === 'string' || options.payload instanceof Stream || Buffer.isBuffer(options.payload), 'options.payload must be either a string, Buffer, or Stream'); + // Setup request var uri = Url.parse(url); @@ -29,7 +31,6 @@ exports.request = function (method, url, options, callback, _trace) { _trace.push({ method: uri.method, url: url }); var agent = (uri.protocol === 'https:' ? Https : Http); - agent.globalAgent.maxSockets = 10; // Support 10 connections in the agent pool var req = agent.request(uri); var shadow = null; // A copy of the streamed request payload when redirects are enabled @@ -106,11 +107,8 @@ exports.request = function (method, url, options, callback, _trace) { return; } - if (typeof options.payload === 'string' || - Buffer.isBuffer(options.payload)) { - req.write(options.payload); - } + req.write(options.payload); } // Finalize request diff --git a/lib/defaults.js b/lib/defaults.js index d514839d9..ef6a9d7ce 100755 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -37,9 +37,9 @@ exports.server = { strictHeader: true // Require an RFC 6265 compliant header format } }, - + // Location - + location: '', // Base uri used to prefix non-absolute outgoing Location headers ('http://example.com:8080'). Must not contain trailing '/'. // Payload @@ -80,7 +80,8 @@ exports.server = { cors: false, // CORS headers on responses and OPTIONS requests (defaults: exports.cors): false -> null, true -> defaults, {} -> override defaults views: null, // Views engine - auth: {} // Authentication + auth: {}, // Authentication + maxSockets: 5 // Sets http/https globalAgent maxSockets value }; @@ -105,7 +106,7 @@ exports.cors = { ], additionalMethods: [], exposedHeaders: [ - 'WWW-Authenticate', + 'WWW-Authenticate', 'Server-Authorization' ], additionalExposedHeaders: [], @@ -136,7 +137,7 @@ exports.state = { }; -// Views +// Views exports.views = { defaultExtension: '', diff --git a/lib/schema.js b/lib/schema.js index e7893d73b..6143a0677 100755 --- a/lib/schema.js +++ b/lib/schema.js @@ -91,7 +91,8 @@ internals.serverSchema = { allowInsecureAccess: T.Boolean(), partialsPath: T.String(), contentType: T.String() - }).nullOk() + }).nullOk(), + maxSockets: T.Number().nullOk() }; diff --git a/lib/server.js b/lib/server.js index c45fde94e..b9074e895 100755 --- a/lib/server.js +++ b/lib/server.js @@ -68,7 +68,7 @@ module.exports = internals.Server = function (/* host, port, options */) { this._host = args.host ? args.host.toLowerCase() : ''; this._port = typeof args.port !== 'undefined' ? args.port : (this.settings.tls ? 443 : 80); - + Utils.assert(!this.settings.location || this.settings.location.charAt(this.settings.location.length - 1) !== '/', 'Location setting must not contain a trailing \'/\''); var socketTimeout = (this.settings.timeout.socket === undefined ? 2 * 60 * 1000 : this.settings.timeout.socket); @@ -119,23 +119,26 @@ module.exports = internals.Server = function (/* host, port, options */) { this.listener = Http.createServer(this._dispatch()); } + Https.globalAgent.maxSockets = this.settings.maxSockets; + Http.globalAgent.maxSockets = this.settings.maxSockets; + // Authentication if (this.settings.auth) { this._auth.addBatch(this.settings.auth); } - + // Server information - + this.info = { host: this._host || '0.0.0.0', port: this._port || 0, protocol: (this.settings.tls ? 'https' : 'http') }; - + if (this.info.port) { this.info.uri = this.info.protocol + '://' + this.info.host + ':' + this.info.port; - } + } return this; }; @@ -203,7 +206,7 @@ internals.Server.prototype._start = function (callback) { this.listener.once('listening', function () { // Update the host, port, and uri with active values - + var address = self.listener.address(); self.info.host = self._host || address.address || '0.0.0.0'; self.info.port = address.port; diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 1e125fd83..b4bf19af3 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -137,7 +137,7 @@ describe('Proxy', function () { var backendPort = upstream.info.port; var routeCache = { expiresIn: 500, mode: 'server+client' }; - server = new Hapi.Server(0, { cors: true }); + server = new Hapi.Server(0, { cors: true, maxSockets: 10 }); server.route([ { method: 'GET', path: '/profile', handler: { proxy: { host: 'localhost', port: backendPort, xforward: true, passThrough: true } } }, { method: 'GET', path: '/item', handler: { proxy: { host: 'localhost', port: backendPort } }, config: { cache: routeCache } }, From 44bcd99f42bc51471a14561303bca42045930126 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 15:40:48 -0500 Subject: [PATCH 12/22] Adding client unit tests --- test/unit/client.js | 58 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test/unit/client.js diff --git a/test/unit/client.js b/test/unit/client.js new file mode 100644 index 000000000..d024b5af6 --- /dev/null +++ b/test/unit/client.js @@ -0,0 +1,58 @@ +// Load modules + +var Lab = require('lab'); +var Boom = require('boom'); +var Events = require('events'); +var Client = require('../../lib/client'); + + +// Declare internals + +var internals = {}; + + +// Test shortcuts + +var expect = Lab.expect; +var before = Lab.before; +var after = Lab.after; +var describe = Lab.experiment; +var it = Lab.test; + + +describe('Client', function () { + + describe('#parse', function () { + + it('handles errors with a boom response', function (done) { + + var res = new Events.EventEmitter(); + res.pipe = function () { }; + + Client.parse(res, function (err) { + + expect(err).to.be.instanceOf(Boom); + done(); + }); + + res.emit('error', new Error('my error')); + }); + + it('handles responses that close early', function (done) { + + var res = new Events.EventEmitter(); + res.pipe = function () { }; + + Client.parse(res, function (err) { + + expect(err).to.be.instanceOf(Boom); + done(); + }); + + res.emit('close'); + }); + }); +}); + + + From bdc18086d708bd4e038ae29706f1f63f2bebd78c Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 15:46:29 -0500 Subject: [PATCH 13/22] Adding documentation for maxSockets --- docs/Reference.md | 48 ++++++++++++++++++++++++----------------------- lib/client.js | 2 -- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/docs/Reference.md b/docs/Reference.md index 75d7f99d0..fa273ea14 100755 --- a/docs/Reference.md +++ b/docs/Reference.md @@ -218,13 +218,15 @@ When creating a server instance, the following options configure the server's be - `server` - response timeout in milliseconds. Sets the maximum time allowed for the server to respond to an incoming client request before giving up and responding with a Service Unavailable (503) error response. Disabled by default (`false`). - `client` - request timeout in milliseconds. Sets the maximum time allowed for the client to transmit the request payload (body) before giving up - and responding with a Request Timeout (408) error response. Set to `false` to disable. Defaults to `10000` (10 seconds). + and responding with a Request Timeout (408) error response. Set to `false` to disable. Defaults to `10000` (10 seconds). - `socket` - by default, node sockets automatically timeout after 2 minutes. Use this option to override this behavior. Defaults to `undefined` which leaves the node default unchanged. Set to `false` to disable socket timeouts.

- `tls` - used to create an HTTPS server. The `tls` object is passed unchanged as options to the node.js HTTPS server as described in the [node.js HTTPS documentation](http://nodejs.org/api/https.html#https_https_createserver_options_requestlistener).

+- `maxSockets` - used to set the number of sockets available per outgoing host connection. Default is 5. This impacts all servers sharing the process. +

- `views` - enables support for view rendering (using templates to generate responses). Disabled by default. To enable, set to an object with the following options: - `engines` - (required) an object where each key is a file extension (e.g. 'html', 'jade'), mapped to the npm module name (string) used for @@ -326,7 +328,7 @@ The following options are available when adding a route: Matching is done against the hostname part of the header only (excluding the port). Defaults to all hosts.

- `handler` - (required) the function called to generate the response after successful authentication and validation. The handler function is - described in [Route handler](#route-handler). Alternatively, `handler` can be set to the string `'notfound'` to return a Not Found (404) + described in [Route handler](#route-handler). Alternatively, `handler` can be set to the string `'notfound'` to return a Not Found (404) error response, or `handler` can be assigned an object with one of: - `file` - generates a static file endpoint for serving a single file. `file` can be set to: - a relative or absolute file path string (relative paths are resolved based on the server [`files`](#server.config.files) configuration). @@ -563,7 +565,7 @@ var paths = [ Parameterized paths are processed by matching the named parameters to the content of the incoming request path at that path segment. For example, '/book/{id}/cover' will match '/book/123/cover' and `request.params.id` will be set to `'123'`. Each path segment (everything between the opening '/' and the closing '/' unless it is the end of the path) can only include one named parameter. - + An optional '?' suffix following the parameter name indicates an optional parameter (only allowed if the parameter is at the ends of the path). For example, the route '/book/{id?}' matches '/book/'. @@ -838,10 +840,10 @@ subscribe to the `'request'` events and filter on `'error'` and `'state'` tags: ```javascript server.on('request', function (request, event, tags) { - + if (tags.error && tags.state) { console.error(event); - } + } }); ``` @@ -874,7 +876,7 @@ Registers an authentication strategy where: - `implementation` - an object with the **hapi** authentication scheme interface (use the `'hawk'` implementation as template). Cannot be used together with `scheme`. - `defaultMode` - if `true`, the scheme is automatically assigned as a required strategy to any route without an `auth` config. Can only be assigned to a single server strategy. Value must be `true` (which is the same as `'required'`) or a valid authentication mode (`'required'`, `'optional'`, `'try'`). Defaults to `false`. - + ##### Basic authentication Basic authentication requires validating a username and password combination. The `'basic'` scheme takes the following required options: @@ -986,7 +988,7 @@ var login = function () { var account = null; if (this.method === 'post') { - + if (!this.payload.username || !this.payload.password) { @@ -1067,7 +1069,7 @@ var credentials = { } var getCredentials = function (id, callback) { - + return callback(null, credentials[id]); }; @@ -1104,7 +1106,7 @@ var credentials = { } var getCredentials = function (id, callback) { - + return callback(null, credentials[id]); }; @@ -1153,7 +1155,7 @@ var Hapi = require('hapi'); var server = new Hapi.Server(); server.ext('onRequest', function (request, next) { - + // Change all requests to '/test' request.setUrl('/test'); next(); @@ -1177,7 +1179,7 @@ Each incoming request passes through a pre-defined set of steps, along with opti - **`'onRequest'`** extension point - always called - the `request` object passed to the extension functions is decorated with the `request.setUrl(url)` and `request.setMethod(verb)` methods. Calls to these methods - will impact how the request is routed and can be used for rewrite rules. + will impact how the request is routed and can be used for rewrite rules. - Lookup route using request path - Parse cookies - **`'onPreAuth'`** extension point @@ -1446,7 +1448,7 @@ var Hapi = require('hapi'); var server = new Hapi.Server(); server.ext('onRequest', function (request, next) { - + // Change all requests to '/test' request.setUrl('/test'); next(); @@ -1466,7 +1468,7 @@ var Hapi = require('hapi'); var server = new Hapi.Server(); server.ext('onRequest', function (request, next) { - + // Change all requests to 'GET' request.setMethod('GET'); next(); @@ -1666,7 +1668,7 @@ var server = new Hapi.Server({ path: __dirname + '/templates' } }); - + var handler = function () { var context = { @@ -1787,7 +1789,7 @@ var handler = function () { onTimeout(function () { response.send(); - }, 1000); + }, 1000); }; ``` @@ -1802,7 +1804,7 @@ Every response type must include the following properties: #### `Generic` The `Generic` response type is used as the parent prototype for all other response types. It cannot be instantiated directly and is only made available -for deriving other response types. It provides the following methods: +for deriving other response types. It provides the following methods: - `code(statusCode)` - sets the HTTP status code where: - `statusCode` - the HTTP status code. @@ -2198,7 +2200,7 @@ Provides a set of utilities for returning HTTP errors. An alias of the [**boom** if `reformat()` is called. Any content allowed and by default includes the following content: - `code` - the HTTP status code, derived from `error.response.code`. - `error` - the HTTP status message (e.g. 'Bad Request', 'Internal Server Error') derived from `code`. - - `message` - the error message derived from `error.message`. + - `message` - the error message derived from `error.message`. - inherited `Error` properties. It also supports the following method: @@ -2407,7 +2409,7 @@ Each `Pack` object instance has the following properties: #### `pack.server([host], [port], [options])` -Creates a `Server` instance and adds it to the pack, where `host`, `port`, `options` are the same as described in +Creates a `Server` instance and adds it to the pack, where `host`, `port`, `options` are the same as described in [`new Server()`](#new-serverhost-port-options) with the exception that the `cache` option is not allowed and must be configured via the pack `cache` option. @@ -2834,7 +2836,7 @@ exports.register = function (plugin, options, next) { this.reply(Hapi.error.internal('Not implemented yet')); }; - + plugin.route({ method: 'GET', path: '/', handler: handler }); next(); }; @@ -2917,7 +2919,7 @@ exports.register = function (plugin, options, next) { }, path: './templates' }); - + next(); }; ``` @@ -3170,15 +3172,15 @@ var handler = function (request) { var content = request.pre.user; Hapi.state.prepareValue('user', content, cookieOptions, function (err, value) { - + if (err) { return request.reply(err); } - + if (value.length < maxCookieSize) { request.setState('user', value, { encoding: 'none' } ); // Already encoded } - + request.reply('success'); }); }; diff --git a/lib/client.js b/lib/client.js index d73cc1e9b..001ba8b60 100755 --- a/lib/client.js +++ b/lib/client.js @@ -153,8 +153,6 @@ exports.parse = function (res, callback) { internals.Collector = function (options, callback) { - var self = this; - if (!callback) { callback = options; options = {}; From 067c314edd01e8b407c777ceed5d36eeb345f557 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 15:53:44 -0500 Subject: [PATCH 14/22] Defaulting maxSockets to null --- docs/Reference.md | 2 +- lib/defaults.js | 5 +++-- lib/server.js | 6 ++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/Reference.md b/docs/Reference.md index fa273ea14..203b80b7b 100755 --- a/docs/Reference.md +++ b/docs/Reference.md @@ -225,7 +225,7 @@ When creating a server instance, the following options configure the server's be - `tls` - used to create an HTTPS server. The `tls` object is passed unchanged as options to the node.js HTTPS server as described in the [node.js HTTPS documentation](http://nodejs.org/api/https.html#https_https_createserver_options_requestlistener).

-- `maxSockets` - used to set the number of sockets available per outgoing host connection. Default is 5. This impacts all servers sharing the process. +- `maxSockets` - used to set the number of sockets available per outgoing host connection. Default is null. This impacts all servers sharing the process.

- `views` - enables support for view rendering (using templates to generate responses). Disabled by default. To enable, set to an object with the following options: diff --git a/lib/defaults.js b/lib/defaults.js index ef6a9d7ce..dd3f3a086 100755 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -20,6 +20,8 @@ exports.server = { // cert: '' // }, + maxSockets: null, // Sets http/https globalAgent maxSockets value + // Router router: { @@ -80,8 +82,7 @@ exports.server = { cors: false, // CORS headers on responses and OPTIONS requests (defaults: exports.cors): false -> null, true -> defaults, {} -> override defaults views: null, // Views engine - auth: {}, // Authentication - maxSockets: 5 // Sets http/https globalAgent maxSockets value + auth: {} // Authentication }; diff --git a/lib/server.js b/lib/server.js index b9074e895..d5ff02d94 100755 --- a/lib/server.js +++ b/lib/server.js @@ -119,8 +119,10 @@ module.exports = internals.Server = function (/* host, port, options */) { this.listener = Http.createServer(this._dispatch()); } - Https.globalAgent.maxSockets = this.settings.maxSockets; - Http.globalAgent.maxSockets = this.settings.maxSockets; + if (this.settings.maxSockets !== null) { + Https.globalAgent.maxSockets = this.settings.maxSockets; + Http.globalAgent.maxSockets = this.settings.maxSockets; + } // Authentication From 516a74442caaf22fbf5124601d8cdbe8f505a196 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Mon, 13 May 2013 15:56:12 -0500 Subject: [PATCH 15/22] Added missing space --- lib/defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/defaults.js b/lib/defaults.js index dd3f3a086..b23a7f178 100755 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -82,7 +82,7 @@ exports.server = { cors: false, // CORS headers on responses and OPTIONS requests (defaults: exports.cors): false -> null, true -> defaults, {} -> override defaults views: null, // Views engine - auth: {} // Authentication + auth: {} // Authentication }; From e6b181e6c0c34e1fb27d469584de5a31fc33e7c3 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 13:57:20 -0700 Subject: [PATCH 16/22] rework zip test --- test/integration/proxy.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index fcb264240..fb8f6eaa7 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -261,11 +261,12 @@ describe('Proxy', function () { expect(res.statusCode).to.equal(200); - Fs.readFile(__dirname + '/../../package.json', function (err, file) { + Fs.readFile(__dirname + '/../../package.json', { encoding: 'utf-8' }, function (err, file) { - Zlib.gzip(file, function (err, zipped) { + Zlib.unzip(res.rawPayload, function (err, unzipped) { - expect(zipped.toString()).to.equal(res.payload); + expect(err).to.not.exist; + expect(unzipped.toString('utf-8')).to.deep.equal(); done(); }); }); @@ -377,23 +378,23 @@ describe('Proxy', function () { }); }); - it('redirects to a post endpoint with stream', function (done) { + it('redirects to another endpoint', function (done) { - server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { + server.inject('/redirect', function (res) { expect(res.statusCode).to.equal(200); - expect(res.payload).to.equal('test'); + expect(res.payload).to.contain('John Doe'); + expect(res.headers['set-cookie']).to.deep.equal(['test=123', 'auto=xyz']); done(); }); }); - it('redirects to another endpoint', function (done) { + it('redirects to a post endpoint with stream', function (done) { - server.inject('/redirect', function (res) { + server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { expect(res.statusCode).to.equal(200); - expect(res.payload).to.contain('John Doe'); - expect(res.headers['set-cookie']).to.deep.equal(['test=123', 'auto=xyz']); + expect(res.payload).to.equal('test'); done(); }); }); From c10189c4112770c65be0abc200b25f924536f7c9 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 14:51:47 -0700 Subject: [PATCH 17/22] Fix zip test --- test/integration/proxy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index fb137f620..0707c03fa 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -263,10 +263,10 @@ describe('Proxy', function () { Fs.readFile(__dirname + '/../../package.json', { encoding: 'utf-8' }, function (err, file) { - Zlib.unzip(res.rawPayload, function (err, unzipped) { + Zlib.unzip(new Buffer(res.payload, 'binary'), function (err, unzipped) { expect(err).to.not.exist; - expect(unzipped.toString('utf-8')).to.deep.equal(); + expect(unzipped.toString('utf-8')).to.deep.equal(file); done(); }); }); From 7f6cf5d37f0e576f6c896a79e8f897e9d42a58fa Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 15:18:39 -0700 Subject: [PATCH 18/22] redirect to relative uris --- lib/client.js | 6 +++++- test/integration/proxy.js | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index 70c0f1e36..3651b52be 100755 --- a/lib/client.js +++ b/lib/client.js @@ -51,7 +51,7 @@ exports.request = function (method, url, options, callback, _trace) { req.once('error', function (err) { - return finish(Boom.internal('Proxy error', err)); + return finish(Boom.internal('Client request error', { err: err, trace: _trace })); }); req.once('response', function (res) { @@ -77,6 +77,10 @@ exports.request = function (method, url, options, callback, _trace) { return finish(Boom.internal('Received redirection without location', _trace)); } + if (!location.match(/^https?:/i)) { + location = Url.resolve(uri.href, location) + } + var redirectOptions = { headers: options.headers, payload: shadow || options.payload, // shadow must be ready at this point if set diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 0707c03fa..018fa2190 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -108,6 +108,15 @@ describe('Proxy', function () { this.reply(new Hapi.response.File(__dirname + '/../../package.json')); }; + var redirectHandler = function () { + + switch (this.query.x) { + case '1': this.reply.redirect('/redirect?x=1'); break; + case '2': this.reply().header('Location', '//localhost:' + this.server.info.port + '/profile').code(302); break; + default: this.reply.redirect('/profile'); break; + } + }; + var upstream = new Hapi.Server(0); upstream.route([ { method: 'GET', path: '/profile', handler: profile }, @@ -122,7 +131,7 @@ describe('Proxy', function () { { method: 'GET', path: '/forward', handler: forward }, { method: 'GET', path: '/gzip', handler: gzipHandler }, { method: 'GET', path: '/gzipstream', handler: gzipStreamHandler }, - { method: 'GET', path: '/redirect', handler: function () { this.reply.redirect(this.query.x ? '/redirect?x=1' : '/profile'); } }, + { method: 'GET', path: '/redirect', handler: redirectHandler }, { method: 'POST', path: '/post1', handler: function () { this.reply.redirect('/post2').rewritable(false); } }, { method: 'POST', path: '/post2', handler: function () { this.reply(this.payload); } } ]); @@ -389,6 +398,17 @@ describe('Proxy', function () { }); }); + it('redirects to another endpoint with relative location', function (done) { + + server.inject('/redirect?x=2', function (res) { + + expect(res.statusCode).to.equal(200); + expect(res.payload).to.contain('John Doe'); + expect(res.headers['set-cookie']).to.deep.equal(['test=123', 'auto=xyz']); + done(); + }); + }); + it('redirects to a post endpoint with stream', function (done) { server.inject({ method: 'POST', url: '/post1', payload: 'test', headers: { 'content-type': 'text/plain' } }, function (res) { From 629a1bdfd1eb0829a3c095042d0adce1a1f190bc Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 15:25:13 -0700 Subject: [PATCH 19/22] redirect missing location --- test/integration/proxy.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 018fa2190..006f627c7 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -113,6 +113,7 @@ describe('Proxy', function () { switch (this.query.x) { case '1': this.reply.redirect('/redirect?x=1'); break; case '2': this.reply().header('Location', '//localhost:' + this.server.info.port + '/profile').code(302); break; + case '3': this.reply().code(302); break; default: this.reply.redirect('/profile'); break; } }; @@ -387,6 +388,15 @@ describe('Proxy', function () { }); }); + it('errors on redirect missing location header', function (done) { + + server.inject('/redirect?x=3', function (res) { + + expect(res.statusCode).to.equal(500); + done(); + }); + }); + it('redirects to another endpoint', function (done) { server.inject('/redirect', function (res) { From 2340ff3183448cae3722ad16336fb8c5f0396e45 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 15:36:04 -0700 Subject: [PATCH 20/22] Fix post streaming --- lib/proxy.js | 19 ++++++------------- test/integration/proxy.js | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/lib/proxy.js b/lib/proxy.js index bf437297d..c260812cc 100755 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -74,22 +74,15 @@ internals.Proxy.prototype.handler = function () { delete options.headers['accept-encoding']; } - // Set payload + // Pass payload - if (request.rawPayload && - request.rawPayload.length) { - - var contentType = req.headers['content-type']; - if (contentType) { - options.headers['Content-Type'] = contentType; - } - - options.payload = request.rawPayload; - } - else { - options.payload = request.raw.req; + var contentType = req.headers['content-type']; + if (contentType) { + options.headers['Content-Type'] = contentType; } + options.payload = request.rawPayload || request.raw.req; + // Send request Client.request(request.method, uri, options, function (err, res) { diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 006f627c7..bea26c37b 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -168,7 +168,7 @@ describe('Proxy', function () { { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } }, { method: 'GET', path: '/googler', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://google.com'); }, redirects: 1 } } }, { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } }, - { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 10 } }, config: { payload: 'parse' } } + { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 3 } }, config: { payload: 'stream' } } ]); server.state('auto', { autoValue: 'xyz' }); From 759c6fc977cb80c51e4ae08a141fd444f9033b5d Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 15:42:11 -0700 Subject: [PATCH 21/22] bad host proxy --- test/integration/proxy.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/integration/proxy.js b/test/integration/proxy.js index bea26c37b..0ac49f7c5 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -168,7 +168,8 @@ describe('Proxy', function () { { method: 'GET', path: '/google', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://www.google.com'); } } } }, { method: 'GET', path: '/googler', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://google.com'); }, redirects: 1 } } }, { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } }, - { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 3 } }, config: { payload: 'stream' } } + { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 3 } }, config: { payload: 'stream' } }, + { method: 'GET', path: '/nowhere', handler: { proxy: { host: 'no.such.domain.x8' } } } ]); server.state('auto', { autoValue: 'xyz' }); @@ -397,6 +398,15 @@ describe('Proxy', function () { }); }); + it('errors on redirection to bad host', function (done) { + + server.inject('/nowhere', function (res) { + + expect(res.statusCode).to.equal(500); + done(); + }); + }); + it('redirects to another endpoint', function (done) { server.inject('/redirect', function (res) { From bea40f1683d93bebfe4e96e012815472f17dcb74 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 13 May 2013 16:15:16 -0700 Subject: [PATCH 22/22] cache bug and 100% --- lib/client.js | 2 +- lib/proxy.js | 5 ++-- test/integration/proxy.js | 51 ++++++++++++++++++++++++++++++--------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/lib/client.js b/lib/client.js index 3651b52be..0e5e2b8eb 100755 --- a/lib/client.js +++ b/lib/client.js @@ -139,7 +139,7 @@ exports.parse = function (res, callback) { res.once('error', function (err) { - return finish(Boom.internal('Proxy response error', err)); + return finish(Boom.internal('Client response error', err)); }); res.once('close', function () { diff --git a/lib/proxy.js b/lib/proxy.js index c260812cc..539ea7ab6 100755 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -103,7 +103,7 @@ internals.Proxy.prototype.handler = function () { return request.reply(err); } - return self.settings.postResponse(request, self.settings, res, buffer); + return self.settings.postResponse(request, self.settings, res, buffer.toString()); }); }); }); @@ -136,4 +136,5 @@ internals.postResponse = function (request, settings, res, payload) { if (contentType) { response.type(contentType); } -}; \ No newline at end of file +}; + diff --git a/test/integration/proxy.js b/test/integration/proxy.js index 0ac49f7c5..06c78e963 100755 --- a/test/integration/proxy.js +++ b/test/integration/proxy.js @@ -5,6 +5,7 @@ var Fs = require('fs'); var Zlib = require('zlib'); var Request = require('request'); var Hapi = require('../..'); +var Client = require('../../lib/client'); // Declare internals @@ -39,26 +40,28 @@ describe('Proxy', function () { } var profile = { - 'id': 'fa0dbda9b1b', - 'name': 'John Doe' + id: 'fa0dbda9b1b', + name: 'John Doe' }; this.reply(profile).state('test', '123'); }; + var activeCount = 0; var activeItem = function () { this.reply({ - 'id': '55cf687663', - 'name': 'Active Item' + id: '55cf687663', + name: 'Active Item', + count: activeCount++ }); }; var item = function () { this.reply({ - 'id': '55cf687663', - 'name': 'Item' + id: '55cf687663', + name: 'Item' }).created('http://example.com'); }; @@ -134,7 +137,8 @@ describe('Proxy', function () { { method: 'GET', path: '/gzipstream', handler: gzipStreamHandler }, { method: 'GET', path: '/redirect', handler: redirectHandler }, { method: 'POST', path: '/post1', handler: function () { this.reply.redirect('/post2').rewritable(false); } }, - { method: 'POST', path: '/post2', handler: function () { this.reply(this.payload); } } + { method: 'POST', path: '/post2', handler: function () { this.reply(this.payload); } }, + { method: 'GET', path: '/cached', handler: profile } ]); var mapUri = function (request, callback) { @@ -169,7 +173,8 @@ describe('Proxy', function () { { method: 'GET', path: '/googler', handler: { proxy: { mapUri: function (request, callback) { callback(null, 'http://google.com'); }, redirects: 1 } } }, { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } }, { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 3 } }, config: { payload: 'stream' } }, - { method: 'GET', path: '/nowhere', handler: { proxy: { host: 'no.such.domain.x8' } } } + { method: 'GET', path: '/nowhere', handler: { proxy: { host: 'no.such.domain.x8' } } }, + { method: 'GET', path: '/cached', handler: { proxy: { host: 'localhost', port: backendPort } }, config: { cache: routeCache } } ]); server.state('auto', { autoValue: 'xyz' }); @@ -296,13 +301,20 @@ describe('Proxy', function () { }); }); - it('forwards on the response when making a GET request to a route that also accepts a POST', function (done) { + it('request a cached proxy route', function (done) { server.inject('/item', function (res) { expect(res.statusCode).to.equal(200); expect(res.payload).to.contain('Active Item'); - done(); + var counter = res.result.count; + + server.inject('/item', function (res) { + + expect(res.statusCode).to.equal(200); + expect(res.result.count).to.equal(counter); + done(); + }); }); }); @@ -438,4 +450,21 @@ describe('Proxy', function () { done(); }); }); -}); \ No newline at end of file + + it('errors on invalid response stream', function (done) { + + var orig = Client.parse; + Client.parse = function (res, callback) { + + Client.parse = orig; + callback(Hapi.error.internal('Fake error')); + }; + + server.inject('/cached', function (res) { + + expect(res.statusCode).to.equal(500); + done(); + }); + }); +}); +