From b4a670a49290ea8fb38a722e21ca650a0f25562d Mon Sep 17 00:00:00 2001 From: Ian Kronquist Date: Mon, 15 Dec 2014 18:14:33 -0800 Subject: [PATCH 1/2] Add test for issue 3992 Test a getAllHeaders function. It should return an object representing the headers in a request. --- test/simple/test-http-header-get-all.js | 71 +++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 test/simple/test-http-header-get-all.js diff --git a/test/simple/test-http-header-get-all.js b/test/simple/test-http-header-get-all.js new file mode 100644 index 00000000000000..d1c7e9a683c4a4 --- /dev/null +++ b/test/simple/test-http-header-get-all.js @@ -0,0 +1,71 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var assert = require('assert'); +var http = require('http'); + +// Verify that ServerResponse.getHeader() works correctly even after +// the response header has been sent. Issue 752 on github. + +var s = http.createServer(function(req, res) { + var contentType = 'Content-Type'; + var plain = 'text/plain'; + var userAgent = 'User-Agent'; + var browser = 'Mozilla'; + var expected = { + 'content-type': plain, + 'user-agent': browser + } + res.setHeader(contentType, plain); + res.setHeader(userAgent, browser); + assert.ok(!res.headersSent); + res.writeHead(200); + assert.ok(res.headersSent); + res.end('hello world\n'); + var headers; + // This checks that after the headers have been sent, getAllHeaders works + // and does not throw an exception + assert.doesNotThrow( + function() { + headers = res.getAllHeaders(); + } + ); + // Check that all fields in headers have the same value as expected, and + // vice-versa. This ensures that there no fields which *don't* exist in one + // or the other. + for (var field in headers) { + assert.equal(headers[field], expected[field]); + } + for (var field in expected) { + assert.equal(headers[field], expected[field]); + } +}); + +s.listen(process.env.PORT, runTest); + +function runTest() { + http.get({ port: process.env.PORT }, function(response) { + response.on('end', function() { + s.close(); + }); + response.resume(); + }); +} From 427fd003f75bcf035e6b29fae40689080bc2b7da Mon Sep 17 00:00:00 2001 From: Ian Kronquist Date: Mon, 15 Dec 2014 18:16:21 -0800 Subject: [PATCH 2/2] Add a getAllHeaders function to the http module Users should not have to access private attributes like res._headers in order to get all the headers in a request. Define a method getAllHeaders to return an object which is a copy of re._headers Fixes issue #3992 --- doc/api/http.markdown | 9 +++ lib/_http_outgoing.js | 8 +++ test/parallel/test-http-header-get-all.js | 23 ++++++++ test/simple/test-http-header-get-all.js | 71 ----------------------- 4 files changed, 40 insertions(+), 71 deletions(-) create mode 100644 test/parallel/test-http-header-get-all.js delete mode 100644 test/simple/test-http-header-get-all.js diff --git a/doc/api/http.markdown b/doc/api/http.markdown index 3d482b441c80d3..f37a6b2ce7cb24 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -380,6 +380,15 @@ Example: var contentType = response.getHeader('content-type'); +### response.getAllHeaders() + +Reads out all headers that are already been queued but not yet sent to the +client. This can only be called before headers get implicitly flushed. + +Example: + + var headers = response.getAllHeaders(); + ### response.removeHeader(name) Removes a header that's queued for implicit sending. diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index cef135c39927f1..d6bfbdd636a676 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -360,6 +360,14 @@ OutgoingMessage.prototype.getHeader = function(name) { }; +OutgoingMessage.prototype.getAllHeaders = function() { + if (!this._headers) + return; + else + return util._extend({}, this._headers); +}; + + OutgoingMessage.prototype.removeHeader = function(name) { if (arguments.length < 1) { throw new Error('`name` is required for removeHeader().'); diff --git a/test/parallel/test-http-header-get-all.js b/test/parallel/test-http-header-get-all.js new file mode 100644 index 00000000000000..57ad44dca21311 --- /dev/null +++ b/test/parallel/test-http-header-get-all.js @@ -0,0 +1,23 @@ +var common = require('../common'); +var assert = require('assert'); +var http = require('http'); + +// Verify that ServerResponse.getHeader() works correctly even after +// the response header has been sent. Issue 752 on github. + +var rando = Math.random(); +var expected = util._extend({}, { + 'X-Random-Thing': rando, +}); +var server = http.createServer(function(req, res) { + res.setHeader('X-Random-Thing', rando); + headers = res.getAllHeaders(); + res.end('hello'); + assert.strictEqual(res.getAllHeaders(), null); +}); +server.listen(common.PORT, function() { + http.get({port: common.PORT}, function(resp) { + assert.deepEqual(response.headers, expected); + server.close(); + }); +}); diff --git a/test/simple/test-http-header-get-all.js b/test/simple/test-http-header-get-all.js deleted file mode 100644 index d1c7e9a683c4a4..00000000000000 --- a/test/simple/test-http-header-get-all.js +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -var assert = require('assert'); -var http = require('http'); - -// Verify that ServerResponse.getHeader() works correctly even after -// the response header has been sent. Issue 752 on github. - -var s = http.createServer(function(req, res) { - var contentType = 'Content-Type'; - var plain = 'text/plain'; - var userAgent = 'User-Agent'; - var browser = 'Mozilla'; - var expected = { - 'content-type': plain, - 'user-agent': browser - } - res.setHeader(contentType, plain); - res.setHeader(userAgent, browser); - assert.ok(!res.headersSent); - res.writeHead(200); - assert.ok(res.headersSent); - res.end('hello world\n'); - var headers; - // This checks that after the headers have been sent, getAllHeaders works - // and does not throw an exception - assert.doesNotThrow( - function() { - headers = res.getAllHeaders(); - } - ); - // Check that all fields in headers have the same value as expected, and - // vice-versa. This ensures that there no fields which *don't* exist in one - // or the other. - for (var field in headers) { - assert.equal(headers[field], expected[field]); - } - for (var field in expected) { - assert.equal(headers[field], expected[field]); - } -}); - -s.listen(process.env.PORT, runTest); - -function runTest() { - http.get({ port: process.env.PORT }, function(response) { - response.on('end', function() { - s.close(); - }); - response.resume(); - }); -}