From 4a7a98fd0ab3d39685888f15bb3a844e863be255 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 23 Jan 2013 13:47:29 +0100 Subject: [PATCH] http: close connection on 204 and chunked encoding This is similar to commit 2cbf458 but this time for 204 No Content instead of 304 Not Modified responses. When the user sends a 204 response with a Transfer-Encoding: chunked header, suppress sending the zero chunk and force the connection to close. --- lib/http.js | 18 ++++++---- test/simple/test-http-chunked-304.js | 54 ++++++++++++++++------------ 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/lib/http.js b/lib/http.js index 6b774c3c680..28601d061c4 100644 --- a/lib/http.js +++ b/lib/http.js @@ -564,18 +564,22 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) { state.messageHeader += 'Date: ' + utcDate() + CRLF; } - // Force the connection to close when the response is a 304 Not Modified - // and the user has set a "Transfer-Encoding: chunked" header. + // Force the connection to close when the response is a 204 No Content or + // a 304 Not Modified and the user has set a "Transfer-Encoding: chunked" + // header. // - // RFC 2616 mandates that 304 responses MUST NOT have a body but node.js - // used to send out a zero chunk anyway to accommodate clients that don't - // have special handling for 304 responses. + // RFC 2616 mandates that 204 and 304 responses MUST NOT have a body but + // node.js used to send out a zero chunk anyway to accommodate clients + // that don't have special handling for those responses. // // It was pointed out that this might confuse reverse proxies to the point // of creating security liabilities, so suppress the zero chunk and force // the connection to close. - if (this.statusCode === 304 && this.chunkedEncoding === true) { - debug('304 response should not use chunked encoding, closing connection.'); + var statusCode = this.statusCode; + if ((statusCode == 204 || statusCode === 304) && + this.chunkedEncoding === true) { + debug(statusCode + ' response should not use chunked encoding,' + + ' closing connection.'); this.chunkedEncoding = false; this.shouldKeepAlive = false; } diff --git a/test/simple/test-http-chunked-304.js b/test/simple/test-http-chunked-304.js index 0f8351192b3..24c5fa15109 100644 --- a/test/simple/test-http-chunked-304.js +++ b/test/simple/test-http-chunked-304.js @@ -24,32 +24,40 @@ var assert = require('assert'); var http = require('http'); var net = require('net'); -// RFC 2616, section 10.3.5: +// RFC 2616, section 10.2.5: // -// The 304 response MUST NOT contain a message-body, and thus is always +// The 204 response MUST NOT contain a message-body, and thus is always // terminated by the first empty line after the header fields. // -// Verify that no empty chunk is sent when the user explicitly sets -// a Transfer-Encoding header. -var server = http.createServer(function(req, res) { - res.writeHead(304, { 'Transfer-Encoding': 'chunked' }); - res.end(); - server.close(); +// Likewise for 304 responses. Verify that no empty chunk is sent when +// the user explicitly sets a Transfer-Encoding header. + +test(204, function() { + test(304); }); -server.listen(common.PORT, function() { - var conn = net.createConnection(common.PORT, function() { - conn.write('GET / HTTP/1.1\r\n\r\n'); - - var resp = ''; - conn.setEncoding('utf8'); - conn.on('data', function(data) { - resp += data; - }); - - conn.on('end', common.mustCall(function() { - assert.equal(/^Connection: close\r\n$/m.test(resp), true); - assert.equal(/^0\r\n$/m.test(resp), false); - })); +function test(statusCode, next) { + var server = http.createServer(function(req, res) { + res.writeHead(statusCode, { 'Transfer-Encoding': 'chunked' }); + res.end(); + server.close(); }); -}); + + server.listen(common.PORT, function() { + var conn = net.createConnection(common.PORT, function() { + conn.write('GET / HTTP/1.1\r\n\r\n'); + + var resp = ''; + conn.setEncoding('utf8'); + conn.on('data', function(data) { + resp += data; + }); + + conn.on('end', common.mustCall(function() { + assert.equal(/^Connection: close\r\n$/m.test(resp), true); + assert.equal(/^0\r\n$/m.test(resp), false); + if (next) process.nextTick(next); + })); + }); + }); +}