Skip to content

Commit

Permalink
Avoid overriding headers set in onresponse
Browse files Browse the repository at this point in the history
Currently, overriding a header from the target response in the source
response requires doing it in both ondata_response (when receiving the first
chunk of data) and in onend_response (for when the response doesn't contain
any data), after verifying that res.headersSent is false. That's clumsy at
best.

By filtering the copied headers with those already present in the source
response, it's possible to set them in onresponse once and for all.
  • Loading branch information
fpavageau committed Jul 24, 2019
1 parent e54361a commit 5a00eaf
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/plugins-middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,9 @@ function handleTargetResponse(targetRequest, targetResponse, options, cb) {
Object.keys(targetResponse.headers).forEach(function(header) {
// skip setting the 'connection: keep-alive' header
// setting it causes gateway to not accept any more connections
if (header !== 'connection') {
// Headers that have been set in onresponse are also skipped to let the plugins override those from the
// target response.
if (header !== 'connection' && !sourceResponse.hasHeader(header)) {
sourceResponse.setHeader(header, targetResponse.headers[header]);
}
});
Expand Down
15 changes: 15 additions & 0 deletions tests/plugin-lifecycle-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var assert = require('assert');
var gatewayService = require('../index');
var serverFactory = require('./hello_rest/index');
var TestPlugin = require('./testPlugin');
var TestResponseHeaderPlugin = require('./testResponseHeaderPlugin');
var request = require('request');
var should = require('should')

Expand Down Expand Up @@ -301,6 +302,20 @@ describe('test lifecycle events', function() {
});
});
});

it('should keep the header set on the source response', function(done) {
this.timeout(20000);
var testPlugin = TestResponseHeaderPlugin();
var handler = testPlugin.init();
gateway.addPlugin('test', function test() { return handler });
gateway.start(function(err) {
assert(!err, err);
request({ method: "GET", url: 'http://localhost:' + gatewayPort + '/v1/echo/get' }, (err, r) => {
assert.equal(r.headers['content-type'], 'application/octet-stream');
done();
});
});
});
});

function _findHeaders(headers, expectedHeaders) {
Expand Down
17 changes: 17 additions & 0 deletions tests/testResponseHeaderPlugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';

const TestResponseHeaderPlugin = function () {
};

module.exports = function () {
return new TestResponseHeaderPlugin();
};

TestResponseHeaderPlugin.prototype.init = function myPlugin1() {
return {
onresponse: function (sourceReq, sourceRes, targetRes, data, next) {
sourceRes.setHeader('content-type', 'application/octet-stream');
next();
}
};
};

0 comments on commit 5a00eaf

Please sign in to comment.