From b7d5d77f3cef505959ea0cc0afb12a8999f6dd4d Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Wed, 13 Nov 2024 14:02:48 -0800 Subject: [PATCH] Bugfix/use write head instead of implicit header (#170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add failing tests for http2 * Add support for http2 The change just removes the usage of undocumented http API and instead uses the proper writeHead method * Remove arrow function in tests * Conditionally run http2 tests if http2 is available * Fix port in tests to be assigned automatically * Change http2 test usage to describe.skip if no http2 available * Fix closing the http2 connections to prevent possible exceptions * Fix closing the request first, then the client, then the server * Fix closing for v8.x and v9.x * fix tests not draining data for http2 requests, resulting in timeouts in node v10.4 onwards * fix: 🐛 assert.equal error * fix: 🐛 remove console.log's and timeout, let build fail * Apply suggestions from code review Co-authored-by: Lam Wei Li * fixed lint * fix: an issue where test hangs when assertion fails in http2 as http2server is not closed * refactor: use http2.constants instead of hard-coded strings in http2 test * Node.js 0.8 compatible * fix lint issue --------- Co-authored-by: Moritz Peters Co-authored-by: Moritz Peters Co-authored-by: Lam Wei Li Co-authored-by: Sebastian Beltran --- index.js | 4 +-- test/compression.js | 86 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 332e4ab5..01b15bd5 100644 --- a/index.js +++ b/index.js @@ -81,7 +81,7 @@ function compression (options) { } if (!headersSent(res)) { - this._implicitHeader() + this.writeHead(this.statusCode) } return stream @@ -100,7 +100,7 @@ function compression (options) { length = chunkLength(chunk, encoding) } - this._implicitHeader() + this.writeHead(this.statusCode) } if (!stream) { diff --git a/test/compression.js b/test/compression.js index 6975ea0b..b6166eb1 100644 --- a/test/compression.js +++ b/test/compression.js @@ -7,6 +7,16 @@ var http = require('http') var request = require('supertest') var zlib = require('zlib') +var describeHttp2 = describe.skip +try { + var http2 = require('http2') + describeHttp2 = describe +} catch (err) { + if (err) { + console.log('http2 tests disabled.') + } +} + var compression = require('..') describe('compression()', function () { @@ -305,6 +315,41 @@ describe('compression()', function () { .expect(200, done) }) + describeHttp2('http2', function () { + it('should work with http2 server', function (done) { + var server = createHttp2Server({ threshold: 0 }, function (req, res) { + res.setHeader(http2.constants.HTTP2_HEADER_CONTENT_TYPE, 'text/plain') + res.end('hello, world') + }) + server.on('listening', function () { + var client = createHttp2Client(server.address().port) + // using ES5 as Node.js <=4.0.0 does not have Computed Property Names + var reqHeaders = {} + reqHeaders[http2.constants.HTTP2_HEADER_ACCEPT_ENCODING] = 'gzip' + var request = client.request(reqHeaders) + request.on('response', function (headers) { + assert.strictEqual(headers[http2.constants.HTTP2_HEADER_STATUS], 200) + assert.strictEqual(headers[http2.constants.HTTP2_HEADER_CONTENT_TYPE], 'text/plain') + assert.strictEqual(headers[http2.constants.HTTP2_HEADER_CONTENT_ENCODING], 'gzip') + }) + var chunks = [] + request.on('data', function (chunk) { + chunks.push(chunk) + }) + request.on('end', function () { + closeHttp2(client, server, function () { + zlib.gunzip(Buffer.concat(chunks), function (err, data) { + assert.ok(!err) + assert.strictEqual(data.toString(), 'hello, world') + done() + }) + }) + }) + request.end() + }) + }) + }) + describe('threshold', function () { it('should not compress responses below the threshold size', function (done) { var server = createServer({ threshold: '1kb' }, function (req, res) { @@ -674,6 +719,47 @@ function createServer (opts, fn) { }) } +function createHttp2Server (opts, fn) { + var _compression = compression(opts) + var server = http2.createServer(function (req, res) { + _compression(req, res, function (err) { + if (err) { + res.statusCode = err.status || 500 + res.end(err.message) + return + } + + fn(req, res) + }) + }) + server.listen(0, '127.0.0.1') + return server +} + +function createHttp2Client (port) { + return http2.connect('http://127.0.0.1:' + port) +} + +function closeHttp2 (client, server, callback) { + if (typeof client.shutdown === 'function') { + // this is the node v8.x way of closing the connections + client.shutdown({}, function () { + server.close(function () { + callback() + }) + }) + } else { + // this is the node v9.x onwards way of closing the connections + client.close(function () { + // force existing connections to time out after 1ms. + // this is done to force the server to close in some cases where it wouldn't do it otherwise. + server.close(function () { + callback() + }) + }) + } +} + function shouldHaveBodyLength (length) { return function (res) { assert.strictEqual(res.text.length, length, 'should have body length of ' + length)