fix: preserve writeHead headers when statusMessage is undefined

HISTORY.md

@@ -1,3 +1,8 @@
+unreleased
+==========
+
+  * Fix `res.writeHead(status, undefined, headers)` dropping headers
+
 1.8.2
 =====
 

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.
  *

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