Allow header mod with streaming

README.md

@@ -1,4 +1,4 @@
-# express-http-proxy [![NPM version](https://badge.fury.io/js/express-http-proxy.svg)](http://badge.fury.io/js/express-http-proxy) [![Build Status](https://travis-ci.org/villadora/express-http-proxy.svg?branch=master)](https://travis-ci.org/villadora/express-http-proxy) 
+# express-http-proxy [![NPM version](https://badge.fury.io/js/express-http-proxy.svg)](http://badge.fury.io/js/express-http-proxy) [![Build Status](https://travis-ci.org/villadora/express-http-proxy.svg?branch=master)](https://travis-ci.org/villadora/express-http-proxy)
 
 
 Express middleware to proxy request to another host and pass response back to original caller.
@@ -28,9 +28,11 @@ app.use('/proxy', proxy('www.google.com'));
 
 Proxy requests and user responses are piped/streamed/chunked by default.
 
-If you define a response modifier (userResDecorator, userResHeaderDecorator),
-or need to inspect the response before continuing (maybeSkipToNext), streaming
-is disabled, and the request and response are buffered.
+Under certain scenarios, streaming is automatically disabled and the request and response are buffered into memory:
+- a `userResDecorator` response modifier is defined
+- a `userResHeaderDecorator` response modifier is defined with greater than 1 argument (ie: `userResHeaderDecorator: function (headers, userReq, userRes, proxyReq, proxyRes)` vs `userResHeaderDecorator: function (headers)`)
+- we need to inspect the response before continuing (`maybeSkipToNext`)
+
 This can cause performance issues with large payloads.
 
 ### Promises
@@ -155,17 +157,17 @@ Promise form:
 
 ```js
   app.use(proxy('localhost:12346', {
-    filter: function (req, res) { 
-      return new Promise(function (resolve) { 
+    filter: function (req, res) {
+      return new Promise(function (resolve) {
         resolve(req.method === 'GET');
-      }); 
+      });
     }
   }));
 ```
 
 Note that in the previous example, `resolve(false)` will execute the happy path
 for filter here (skipping the rest of the proxy, and calling `next()`).
-`reject()` will also skip the rest of proxy and call `next()`. 
+`reject()` will also skip the rest of proxy and call `next()`.
 
 #### userResDecorator (was: intercept) (supports Promise)
 
@@ -262,12 +264,23 @@ When a `userResHeaderDecorator` is defined, the return of this method will repla
 ```js
 app.use('/proxy', proxy('www.google.com', {
   userResHeaderDecorator(headers, userReq, userRes, proxyReq, proxyRes) {
-    // recieves an Object of headers, returns an Object of headers.
+    // receives an Object of headers, returns an Object of headers.
     return headers;
   }
 }));
 ```
 
+If this method is defined with only a single argument, it allows for response streaming (see streaming) since we don't require access to the response body.
+
+```js
+app.use('/proxy', proxy('www.google.com', {
+  userResHeaderDecorator(headers) {
+    // receives an Object of headers, returns an Object of headers.
+    // we don't access the response body so streaming is allowed.
+    return headers;
+  }
+}));
+```
 
 #### decorateRequest
 
@@ -581,9 +594,9 @@ app.use('/', proxy('internalhost.example.com', {
 | 1.6.0 | Do gzip and gunzip aysyncronously.   Test and documentation improvements, dependency updates. |
 | 1.5.1 | Fixes bug in stringifying debug messages. |
 | 1.5.0 | Fixes bug in `filter` signature.  Fix bug in skipToNextHandler, add expressHttpProxy value to user res when skipped.  Add tests for host as ip address. |
-| 1.4.0 | DEPRECATED. Critical bug in the `filter` api.| 
+| 1.4.0 | DEPRECATED. Critical bug in the `filter` api.|
 | 1.3.0 | DEPRECATED. Critical bug in the `filter` api. `filter` now supports Promises.  Update linter to eslint.  |
-| 1.2.0 | Auto-stream when no decorations are made to req/res. Improved docs, fixes issues in maybeSkipToNexthandler,  allow authors to manage error handling. | 
+| 1.2.0 | Auto-stream when no decorations are made to req/res. Improved docs, fixes issues in maybeSkipToNexthandler,  allow authors to manage error handling. |
 | 1.1.0 | Add step to allow response headers to be modified.
 | 1.0.7 | Update dependencies.  Improve docs on promise rejection.   Fix promise rejection on body limit.   Improve debug output. |
 | 1.0.6 | Fixes preserveHostHdr not working, skip userResDecorator on 304, add maybeSkipToNext, test improvements and cleanup. |

app/steps/decorateUserResHeaders.js

@@ -16,7 +16,11 @@ function decorateUserResHeaders(container) {
   }
 
   return Promise
-    .resolve(resolverFn(headers, container.user.req, container.user.res, container.proxy.req, container.proxy.res))
+    .resolve(
+      resolverFn.length === 1
+        ? resolverFn(headers)
+        : resolverFn(headers, container.user.req, container.user.res, container.proxy.req, container.proxy.res)
+    )
     .then(function(headers) {
       return new Promise(function(resolve) {
         clearAllHeaders(container.user.res);

lib/resolveOptions.js

@@ -66,11 +66,18 @@ function resolveOptions(options) {
     timeout: options.timeout
   };
 
+
+  // allow streaming if we don't require access to the response body in userResHeaderDecorator
+
+  var userResHeaderDecoratorCanBeStreamed =
+    !resolved.userResHeaderDecorator || resolved.userResHeaderDecorator.length === 1;
+
+
   // automatically opt into stream mode if no response modifiers are specified
 
   resolved.stream = !resolved.skipToNextHandlerFilter &&
                     !resolved.userResDecorator &&
-                    !resolved.userResHeaderDecorator;
+                    userResHeaderDecoratorCanBeStreamed;
 
   debug(resolved);
   return resolved;

test/streaming.js

@@ -27,7 +27,7 @@ function simulateUserRequest() {
     var req = http.request({ hostname: 'localhost', port: 8308, path: '/stream' }, function (res) {
       var chunks = [];
       res.on('data', function (chunk) { chunks.push(chunk.toString()); });
-      res.on('end', function () { resolve(chunks); });
+      res.on('end', function () { resolve([chunks, res]); });
     });
 
     req.on('error', function (e) {
@@ -74,18 +74,35 @@ describe('streams / piped requests', function () {
 
       name: 'proxyReqOptDecorator is a Promise',
       options: { proxyReqOptDecorator: function (reqBuilder) { return Promise.resolve(reqBuilder); } }
+    }, {
+      name: 'userResHeaderDecorator is defined with a single argument',
+      options: {
+        userResHeaderDecorator: function (headers) {
+          return Object.assign({}, headers, { 'x-my-new-header': 'special-header' });
+        }
+      },
+      expectedHeaders: {
+        'x-my-new-header': 'special-header'
+      }
     }];
 
     TEST_CASES.forEach(function (testCase) {
       describe(testCase.name, function () {
         it('chunks are received without any buffering, e.g. before request end', function (done) {
           server = startLocalServer(testCase.options);
           simulateUserRequest()
-            .then(function (res) {
+            .then(function ([chunks, res]) {
               // Assume that if I'm getting a chunked response, it will be an array of length > 1;
 
-              assert(res instanceof Array, 'res is an Array');
-              assert.equal(res.length, 4);
+              assert(chunks instanceof Array, 'res is an Array');
+              assert.equal(chunks.length, 4);
+
+              if (testCase.expectedHeaders) {
+                Object.keys(testCase.expectedHeaders).forEach((header) => {
+                  assert.equal(res.headers[header], testCase.expectedHeaders[header]);
+                });
+              }
+
               done();
             })
             .catch(done);
@@ -98,6 +115,25 @@ describe('streams / piped requests', function () {
     var TEST_CASES = [{
       name: 'skipToNextHandler is defined',
       options: { skipToNextHandlerFilter: function () { return false; } }
+    }, {
+      name: 'userResDecorator is defined',
+      options: {
+        // eslint-disable-next-line
+        userResDecorator: async (proxyRes, proxyResData, _userReq, _userRes) => {
+          return proxyResData;
+        }
+      }
+    }, {
+      name: 'userResHeaderDecorator is defined',
+      options: {
+        // eslint-disable-next-line
+        userResHeaderDecorator: function (headers, userReq, userRes, proxyReq, proxyRes) {
+          return Object.assign({}, headers, { 'x-my-new-header': 'special-header' });
+        }
+      },
+      expectedHeaders: {
+        'x-my-new-header': 'special-header'
+      }
     }];
 
     TEST_CASES.forEach(function (testCase) {
@@ -106,11 +142,18 @@ describe('streams / piped requests', function () {
           server = startLocalServer(testCase.options);
 
           simulateUserRequest()
-            .then(function (res) {
-              // Assume that if I'm getting a un-chunked response, it will be an array of length = 1;
+            .then(function ([chunks, res]) {
+              // Assume that if I'm getting a unbuffered response, it will be an array of length = 1;
+
+              assert(chunks instanceof Array);
+              assert.equal(chunks.length, 1);
+
+              if (testCase.expectedHeaders) {
+                Object.keys(testCase.expectedHeaders).forEach((header) => {
+                  assert.equal(res.headers[header], testCase.expectedHeaders[header]);
+                });
+              }
 
-              assert(res instanceof Array);
-              assert.equal(res.length, 1);
               done();
             })
             .catch(done);