From e4abe871679a85ea1d1c08d3613f7a4d98630341 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 14:03:00 +0000 Subject: [PATCH] fix: preserve writeHead headers when statusMessage is undefined on-headers treats a non-string 2nd writeHead argument as the headers object, so writeHead(code, undefined, headers) dropped the headers. Normalize that 3-arg form the same way Node does (obj ??= reason). Fixes expressjs/compression#254 Co-authored-by: David --- HISTORY.md | 5 + index.js | 30 ++++++ test/compression.js | 217 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 252 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 607e973..62e0060 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,8 @@ +unreleased +========== + + * Fix `res.writeHead(status, undefined, headers)` dropping headers + 1.8.2 ===== diff --git a/index.js b/index.js index 91e992c..d77da9e 100644 --- a/index.js +++ b/index.js @@ -262,6 +262,11 @@ function compression (options) { }) }) + // on-headers must wrap writeHead first; this wrapper sits outside it so + // writeHead(code, undefined, headers) is rewritten before on-headers + // mis-reads a non-string 2nd argument as the headers object (#254). + patchWriteHead(res) + next() } } @@ -332,6 +337,31 @@ function toBuffer (chunk, encoding) { : Buffer.from(chunk, encoding) } +/** + * Adapt writeHead() so a 3-argument call with a non-string statusMessage + * matches Node.js `http.ServerResponse` (`obj ??= reason`). + * + * on-headers@1.1.x chooses the headers argument only by + * `typeof arguments[1] === 'string'`. When the statusMessage is `undefined` + * or `null`, it reads arguments[1] as headers and never sees arguments[2], + * so writeHead(200, undefined, headers) silently drops the headers. + * + * @param {object} res + * @private + */ + +function patchWriteHead (res) { + var writeHead = res.writeHead + + res.writeHead = function patchedWriteHead (statusCode, reason, headers) { + if (arguments.length > 2 && typeof reason !== 'string') { + return writeHead.call(this, statusCode, headers != null ? headers : reason) + } + + return writeHead.apply(this, arguments) + } +} + /** * Determine if the response headers have been sent. * diff --git a/test/compression.js b/test/compression.js index cf6b516..3fe8bfd 100644 --- a/test/compression.js +++ b/test/compression.js @@ -1058,6 +1058,185 @@ describe('compression()', function () { .expect(200, done) }) }) + + describe('res.writeHead', function () { + it('should set headers when statusMessage is undefined', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, undefined, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should set headers when statusMessage is null', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, null, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should keep headers when the 3rd argument is undefined', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }, undefined) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should keep headers when the 3rd argument is null', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }, null) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should set headers with a string statusMessage', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, 'OK', { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should set headers when filter skips compression', function (done) { + var server = createServer({ filter: function () { return false } }, function (req, res) { + res.writeHead(200, undefined, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }) + res.end('hello, world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect(shouldNotHaveHeader('Content-Encoding')) + .expect(200, 'hello, world', done) + }) + + it('should set headers when write() follows writeHead', function (done) { + var server = createServer({ threshold: 0 }, function (req, res) { + res.writeHead(200, undefined, { + 'Content-Type': 'text/plain', + 'X-Custom': 'header' + }) + res.write('hello, ') + res.end('world') + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('X-Custom', 'header') + .expect('Content-Encoding', 'gzip') + .expect(200, 'hello, world', done) + }) + + it('should match native statusMessage when statusMessage is undefined', function (done) { + assertWriteHeadParity(201, undefined, { 'X-Custom': 'header' }, function (err, nativeRes, compressedRes) { + if (err) return done(err) + assert.strictEqual(compressedRes.statusCode, nativeRes.statusCode) + assert.strictEqual(compressedRes.statusMessage, nativeRes.statusMessage) + if (nativeRes.statusMessage != null) { + assert.strictEqual(nativeRes.statusMessage, 'Created') + } + assert.strictEqual(compressedRes.headers['x-custom'], 'header') + done() + }) + }) + + it('should match native statusMessage when statusMessage is null', function (done) { + assertWriteHeadParity(201, null, { 'X-Custom': 'header' }, function (err, nativeRes, compressedRes) { + if (err) return done(err) + assert.strictEqual(compressedRes.statusCode, nativeRes.statusCode) + assert.strictEqual(compressedRes.statusMessage, nativeRes.statusMessage) + if (nativeRes.statusMessage != null) { + assert.strictEqual(nativeRes.statusMessage, 'Created') + } + assert.strictEqual(compressedRes.headers['x-custom'], 'header') + done() + }) + }) + + it('should preserve a custom string statusMessage', function (done) { + assertWriteHeadParity(200, 'Custom', { 'X-Custom': 'header' }, function (err, nativeRes, compressedRes) { + if (err) return done(err) + assert.strictEqual(compressedRes.statusMessage, nativeRes.statusMessage) + if (compressedRes.statusMessage != null) { + assert.strictEqual(compressedRes.statusMessage, 'Custom') + } + assert.strictEqual(compressedRes.headers['x-custom'], 'header') + done() + }) + }) + + it('should not stringify an undefined statusMessage', function (done) { + var server = createServer({ filter: function () { return false } }, function (req, res) { + res.writeHead(200, undefined, { 'Content-Type': 'text/plain' }) + res.end('hello, world') + }) + + request(server) + .get('/') + .expect(200) + .end(function (err, res) { + if (err) return done(err) + if (res.res.statusMessage != null) { + assert.strictEqual(res.res.statusMessage, 'OK') + assert.notStrictEqual(res.res.statusMessage, 'undefined') + } + done() + }) + }) + }) }) function createServer (opts, fn) { @@ -1075,6 +1254,44 @@ function createServer (opts, fn) { }) } +function writeHeadHandler (statusCode, statusMessage, headers) { + return function (req, res) { + res.writeHead(statusCode, statusMessage, headers) + res.end('hello, world') + } +} + +function requestOnce (server, callback) { + server.listen(0, '127.0.0.1', function () { + http.get({ host: '127.0.0.1', port: server.address().port, path: '/' }, function (res) { + res.resume() + res.on('end', function () { + server.close(function () { + callback(null, res) + }) + }) + }).on('error', function (err) { + server.close(function () { + callback(err) + }) + }) + }) +} + +function assertWriteHeadParity (statusCode, statusMessage, headers, callback) { + var handler = writeHeadHandler(statusCode, statusMessage, headers) + var native = http.createServer(handler) + var compressed = createServer({ filter: function () { return false } }, handler) + + requestOnce(native, function (err, nativeRes) { + if (err) return callback(err) + requestOnce(compressed, function (err, compressedRes) { + if (err) return callback(err) + callback(null, nativeRes, compressedRes) + }) + }) +} + // Assert that the compression stream is destroyed when the response closes // prematurely (client disconnects mid-response), so the native zlib resources // are released. The stream is captured by wrapping the zlib factory, as its