Skip to content

Commit 5a00eaf

Browse files
committed
Avoid overriding headers set in onresponse
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.
1 parent e54361a commit 5a00eaf

3 files changed

Lines changed: 35 additions & 1 deletion

File tree

lib/plugins-middleware.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,9 @@ function handleTargetResponse(targetRequest, targetResponse, options, cb) {
364364
Object.keys(targetResponse.headers).forEach(function(header) {
365365
// skip setting the 'connection: keep-alive' header
366366
// setting it causes gateway to not accept any more connections
367-
if (header !== 'connection') {
367+
// Headers that have been set in onresponse are also skipped to let the plugins override those from the
368+
// target response.
369+
if (header !== 'connection' && !sourceResponse.hasHeader(header)) {
368370
sourceResponse.setHeader(header, targetResponse.headers[header]);
369371
}
370372
});

tests/plugin-lifecycle-tests.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ var assert = require('assert');
33
var gatewayService = require('../index');
44
var serverFactory = require('./hello_rest/index');
55
var TestPlugin = require('./testPlugin');
6+
var TestResponseHeaderPlugin = require('./testResponseHeaderPlugin');
67
var request = require('request');
78
var should = require('should')
89

@@ -301,6 +302,20 @@ describe('test lifecycle events', function() {
301302
});
302303
});
303304
});
305+
306+
it('should keep the header set on the source response', function(done) {
307+
this.timeout(20000);
308+
var testPlugin = TestResponseHeaderPlugin();
309+
var handler = testPlugin.init();
310+
gateway.addPlugin('test', function test() { return handler });
311+
gateway.start(function(err) {
312+
assert(!err, err);
313+
request({ method: "GET", url: 'http://localhost:' + gatewayPort + '/v1/echo/get' }, (err, r) => {
314+
assert.equal(r.headers['content-type'], 'application/octet-stream');
315+
done();
316+
});
317+
});
318+
});
304319
});
305320

306321
function _findHeaders(headers, expectedHeaders) {

tests/testResponseHeaderPlugin.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
const TestResponseHeaderPlugin = function () {
4+
};
5+
6+
module.exports = function () {
7+
return new TestResponseHeaderPlugin();
8+
};
9+
10+
TestResponseHeaderPlugin.prototype.init = function myPlugin1() {
11+
return {
12+
onresponse: function (sourceReq, sourceRes, targetRes, data, next) {
13+
sourceRes.setHeader('content-type', 'application/octet-stream');
14+
next();
15+
}
16+
};
17+
};

0 commit comments

Comments
 (0)