Merge pull request hapijs#906 from wpreul/master

Proxy requests are closed when server response already sent
jmonster · May 31, 2013 · 2625e63 · 2625e63
2 parents 98e5f2a + bcc9359
commit 2625e63
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 5 deletions.
diff --git a/lib/client.js b/lib/client.js
@@ -22,6 +22,7 @@ exports.request = function (method, url, options, callback, _trace) {
     // Setup request
 
     var uri = Url.parse(url);
+    var timeoutId;
     uri.method = method.toUpperCase();
     uri.headers = options.headers;
 
@@ -49,6 +50,7 @@ exports.request = function (method, url, options, callback, _trace) {
 
             req.removeAllListeners();
             req.on('error', Utils.ignore);
+            clearTimeout(timeoutId);
 
             return callback(err, res);
         }
@@ -122,10 +124,18 @@ exports.request = function (method, url, options, callback, _trace) {
     }
 
     if (options.timeout) {
-        req.setTimeout(options.timeout, function () {
+        timeoutId = setTimeout(function () {
 
-            req.destroy();
+            req.abort();
             return finish(Boom.internal('Client request timeout'));
+        }, options.timeout);
+    }
+
+    if (options.downstreamRes) {
+        options.downstreamRes.once('finish', function () {
+
+            req.abort();
+            return finish(Boom.internal('Server response finished before client response'));
         });
     }
 

diff --git a/lib/proxy.js b/lib/proxy.js
@@ -56,7 +56,8 @@ internals.Proxy.prototype.handler = function () {
                 payload: null,
                 redirects: self.settings.redirects,
                 timeout: self.settings.timeout,
-                rejectUnauthorized: self.settings.rejectUnauthorized
+                rejectUnauthorized: self.settings.rejectUnauthorized,
+                downstreamRes: request.raw.res
             };
 
             if (self.settings.passThrough) {                        // Never set with cache

diff --git a/package.json b/package.json
@@ -2,7 +2,7 @@
   "name": "hapi",
   "description": "HTTP Server framework",
   "homepage": "http://hapijs.com",
-  "version": "1.6.1",
+  "version": "1.6.2",
   "author": "Eran Hammer <eran@hueniverse.com> (http://hueniverse.com)",
   "contributors": [
     {

diff --git a/test/integration/proxy.js b/test/integration/proxy.js
@@ -1,5 +1,6 @@
 // Load modules
 
+var Async = require('async');
 var Lab = require('lab');
 var Fs = require('fs');
 var Zlib = require('zlib');
@@ -25,6 +26,7 @@ describe('Proxy', function () {
 
     var server = null;
     var sslServer = null;
+    var timeoutServer = null;
 
     before(function (done) {
 
@@ -218,10 +220,20 @@ describe('Proxy', function () {
                     { method: 'GET', path: '/sslDefault', handler: { proxy: { mapUri: mapSslUri } } }
                 ]);
 
+                timeoutServer = new Hapi.Server(0, { timeout: { server: 5 }});
+                timeoutServer.route([
+                    { method: 'GET', path: '/timeout1', handler: { proxy: { host: 'localhost', port: backendPort, timeout: 15 } } },
+                    { method: 'GET', path: '/timeout2', handler: { proxy: { host: 'localhost', port: backendPort, timeout: 2 } } },
+                    { method: 'GET', path: '/item', handler: { proxy: { host: 'localhost', port: backendPort } } }
+                ]);
+
                 server.state('auto', { autoValue: 'xyz' });
                 server.start(function () {
 
-                    sslServer.start(done);
+                    sslServer.start(function () {
+
+                        timeoutServer.start(done);
+                    });
                 });
             });
         });
@@ -545,5 +557,48 @@ describe('Proxy', function () {
             done();
         });
     });
+
+    it('doesn\'t consume all sockets when server times out before proxy', function (done) {
+
+        var wrappedReq = function (next) {
+
+            timeoutServer.inject('/timeout1', next);
+        };
+
+        Async.series([
+            wrappedReq,
+            wrappedReq,
+            wrappedReq,
+            wrappedReq,
+            wrappedReq,
+            wrappedReq,
+            wrappedReq
+        ], function () {
+
+            timeoutServer.inject('/item', function (res) {
+
+                expect(res.statusCode).to.equal(200);
+                done();
+            });
+        });
+    });
+
+    it('times out when proxy timeout is less than server', function (done) {
+
+        timeoutServer.inject('/timeout2', function (res) {
+
+            expect(res.statusCode).to.equal(500);
+            done();
+        });
+    });
+
+    it('times out when server timeout is less than proxy', function (done) {
+
+        timeoutServer.inject('/timeout1', function (res) {
+
+            expect(res.statusCode).to.equal(503);
+            done();
+        });
+    });
 });