Use writeHead instead of setHeader/getHeader. (#813)

* Use writeHead instead of setHeader/getHeader.

* Added 'reply.getHeader' interface

* Implemented fast path for getHeader

* Removed public getHeader

* Removed 'content-length: 0' in all occurrences

* Added comment about why we cannot use writeHead for streams

* Support the fast path in Node 10+

* Ignored clinic stuff

* Set 'content-length: 0' according to spec

.gitignore

@@ -52,8 +52,10 @@ profile-*
 .vscode
 *code-workspace
 
-# flamegraphs
+# clinic
 profile*
+*clinic*
+*flamegraph*
 
 # lock files
 yarn.lock

examples/example.js

@@ -19,7 +19,6 @@ const opts = {
 
 fastify
   .get('/', opts, function (req, reply) {
-    reply.header('Content-Type', 'application/json').code(200)
     reply.send({ hello: 'world' })
   })
   .get('/promise', opts, function (req, reply) {

lib/handleRequest.js

@@ -13,8 +13,10 @@ const inputSchemaError = fastJsonStringify(schemas.inputSchemaError)
 function handleRequest (req, res, params, context) {
   var method = req.method
   var headers = req.headers
-  var request = new context.Request(params, req, urlUtil.parse(req.url, true).query, headers, req.log)
-  var reply = new context.Reply(res, context, request)
+  var Request = context.Request
+  var Reply = context.Reply
+  var request = new Request(params, req, urlUtil.parse(req.url, true).query, headers, req.log)
+  var reply = new Reply(res, context, request)
 
   if (method === 'GET' || method === 'HEAD') {
     handler(reply)

lib/reply.js

@@ -19,6 +19,8 @@ const serializeError = FJS({
   }
 })
 
+var getHeader
+
 function Reply (res, context, request) {
   this.res = res
   this.context = context
@@ -27,6 +29,7 @@ function Reply (res, context, request) {
   this._customError = false
   this._isError = false
   this.request = request
+  this._headers = {}
 }
 
 Reply.prototype.send = function (payload) {
@@ -47,20 +50,20 @@ Reply.prototype.send = function (payload) {
     return
   }
 
-  const contentType = this.res.getHeader('Content-Type')
-  const contentTypeNotSet = contentType === undefined
+  var contentType = getHeader(this, 'content-type')
+  var contentTypeNotSet = contentType === undefined
 
   if (payload !== null) {
     if (Buffer.isBuffer(payload) || typeof payload.pipe === 'function') {
       if (contentTypeNotSet) {
-        this.res.setHeader('Content-Type', 'application/octet-stream')
+        this._headers['content-type'] = 'application/octet-stream'
       }
       onSendHook(this, payload)
       return
     }
 
     if (contentTypeNotSet && typeof payload === 'string') {
-      this.res.setHeader('Content-Type', 'text/plain')
+      this._headers['content-type'] = 'text/plain'
       onSendHook(this, payload)
       return
     }
@@ -69,7 +72,7 @@ Reply.prototype.send = function (payload) {
   if (this._serializer) {
     payload = this._serializer(payload)
   } else if (contentTypeNotSet || contentType === 'application/json') {
-    this.res.setHeader('Content-Type', 'application/json')
+    this._headers['content-type'] = 'application/json'
     payload = serialize(this.context, payload, this.res.statusCode)
     flatstr(payload)
   }
@@ -78,7 +81,7 @@ Reply.prototype.send = function (payload) {
 }
 
 Reply.prototype.header = function (key, value) {
-  this.res.setHeader(key, value)
+  this._headers[key.toLowerCase()] = value
   return this
 }
 
@@ -105,7 +108,7 @@ Reply.prototype.serializer = function (fn) {
 }
 
 Reply.prototype.type = function (type) {
-  this.res.setHeader('Content-Type', type)
+  this._headers['content-type'] = type
   return this
 }
 
@@ -115,7 +118,7 @@ Reply.prototype.redirect = function (code, url) {
     code = 302
   }
 
-  this.header('Location', url).code(code).send()
+  this.header('location', url).code(code).send()
 }
 
 function onSendHook (reply, payload) {
@@ -140,27 +143,44 @@ function wrapOnSendEnd (err, reply, payload) {
 }
 
 function onSendEnd (reply, payload) {
+  var res = reply.res
+  var statusCode = res.statusCode
+
   if (payload === undefined || payload === null) {
     reply.sent = true
-    reply.res.end()
+
+    // according to https://tools.ietf.org/html/rfc7230#section-3.3.2
+    // we cannot send a content-length for 304 and 204, and all status code
+    // < 200.
+    if (statusCode >= 200 && statusCode !== 204 && statusCode !== 304) {
+      reply._headers['content-length'] = '0'
+    }
+
+    res.writeHead(statusCode, reply._headers)
+    // avoid ArgumentsAdaptorTrampoline from V8
+    res.end(null, null, null)
     return
   }
 
   if (typeof payload.pipe === 'function') {
-    sendStream(payload, reply.res, reply)
+    sendStream(payload, res, reply)
     return
   }
 
   if (typeof payload !== 'string' && !Buffer.isBuffer(payload)) {
     throw new TypeError(`Attempted to send payload of invalid type '${typeof payload}'. Expected a string or Buffer.`)
   }
 
-  if (!reply.res.getHeader('Content-Length')) {
-    reply.res.setHeader('Content-Length', '' + Buffer.byteLength(payload))
+  if (!reply._headers['content-length']) {
+    reply._headers['content-length'] = '' + Buffer.byteLength(payload)
   }
 
   reply.sent = true
-  reply.res.end(payload)
+
+  res.writeHead(statusCode, reply._headers)
+
+  // avoid ArgumentsAdaptorTrampoline from V8
+  res.end(payload, null, null)
 }
 
 function sendStream (payload, res, reply) {
@@ -196,11 +216,19 @@ function sendStream (payload, res, reply) {
     }
   })
 
+  // streams will error asynchronously, and we want to handle that error
+  // appropriately, e.g. a 404 for a missing file. So we cannot use
+  // writeHead, and we need to resort to setHeader, which will trigger
+  // a writeHead when there is data to send.
+  for (var key in reply._headers) {
+    res.setHeader(key, reply._headers[key])
+  }
   payload.pipe(res)
 }
 
 function handleError (reply, error, cb) {
-  var statusCode = reply.res.statusCode
+  var res = reply.res
+  var statusCode = res.statusCode
   statusCode = (statusCode >= 400) ? statusCode : 500
   if (error != null) {
     if (error.status >= 400) {
@@ -218,12 +246,12 @@ function handleError (reply, error, cb) {
     }
   }
 
-  reply.res.statusCode = statusCode
+  res.statusCode = statusCode
 
   if (statusCode >= 500) {
-    reply.res.log.error({ req: reply.request.raw, res: reply.res, err: error }, error && error.message)
+    res.log.error({ req: reply.request.raw, res: res, err: error }, error && error.message)
   } else if (statusCode >= 400) {
-    reply.res.log.info({ res: reply.res, err: error }, error && error.message)
+    res.log.info({ res: res, err: error }, error && error.message)
   }
 
   if (error && error.headers) {
@@ -250,16 +278,17 @@ function handleError (reply, error, cb) {
     statusCode: statusCode
   })
   flatstr(payload)
-  reply.res.setHeader('Content-Type', 'application/json')
+  reply._headers['content-type'] = 'application/json'
 
   if (cb) {
     cb(reply, payload)
     return
   }
 
-  reply.res.setHeader('Content-Length', '' + Buffer.byteLength(payload))
+  reply._headers['content-length'] = '' + Buffer.byteLength(payload)
   reply.sent = true
-  reply.res.end(payload)
+  res.writeHead(res.statusCode, reply._headers)
+  res.end(payload)
 }
 
 function buildReply (R) {
@@ -271,6 +300,7 @@ function buildReply (R) {
     this.sent = false
     this._serializer = null
     this.request = request
+    this._headers = {}
   }
   _Reply.prototype = new R()
   return _Reply
@@ -292,5 +322,36 @@ function notFound (reply) {
 
 function noop () {}
 
+function getHeaderProper (reply, key) {
+  key = key.toLowerCase()
+  var res = reply.res
+  var value = reply._headers[key]
+  if (value === undefined && res.hasHeader(key)) {
+    value = res.getHeader(key)
+  }
+  return value
+}
+
+function getHeaderFallback (reply, key) {
+  key = key.toLowerCase()
+  var res = reply.res
+  var value = reply._headers[key]
+  if (value === undefined) {
+    value = res.getHeader(key)
+  }
+  return value
+}
+
+// ponyfill for hasHeader. It has been intoroduced into Node 7.7,
+// so it's ok to use it in 8+
+{
+  const v = process.version.match(/v(\d+)/)[1]
+  if (Number(v) > 7) {
+    getHeader = getHeaderProper
+  } else {
+    getHeader = getHeaderFallback
+  }
+}
+
 module.exports = Reply
 module.exports.buildReply = buildReply

test/hooks.test.js

@@ -697,7 +697,7 @@ test('onSend hook is called after payload is serialized and headers are set', t
 
     instance.addHook('onSend', function (request, reply, payload, next) {
       t.deepEqual(JSON.parse(payload), thePayload)
-      t.strictEqual(reply.res.getHeader('Content-Type'), 'application/json')
+      t.strictEqual(reply._headers['content-type'], 'application/json')
       next()
     })
 
@@ -711,7 +711,7 @@ test('onSend hook is called after payload is serialized and headers are set', t
   fastify.register((instance, opts, next) => {
     instance.addHook('onSend', function (request, reply, payload, next) {
       t.strictEqual(payload, 'some text')
-      t.strictEqual(reply.res.getHeader('Content-Type'), 'text/plain')
+      t.strictEqual(reply._headers['content-type'], 'text/plain')
       next()
     })
 
@@ -727,7 +727,7 @@ test('onSend hook is called after payload is serialized and headers are set', t
 
     instance.addHook('onSend', function (request, reply, payload, next) {
       t.strictEqual(payload, thePayload)
-      t.strictEqual(reply.res.getHeader('Content-Type'), 'application/octet-stream')
+      t.strictEqual(reply._headers['content-type'], 'application/octet-stream')
       next()
     })
 
@@ -749,7 +749,7 @@ test('onSend hook is called after payload is serialized and headers are set', t
 
     instance.addHook('onSend', function (request, reply, payload, next) {
       t.strictEqual(payload, thePayload)
-      t.strictEqual(reply.res.getHeader('Content-Type'), 'application/octet-stream')
+      t.strictEqual(reply._headers['content-type'], 'application/octet-stream')
       next()
     })
 
@@ -765,7 +765,7 @@ test('onSend hook is called after payload is serialized and headers are set', t
 
     instance.addHook('onSend', function (request, reply, payload, next) {
       t.strictEqual(payload, serializedPayload)
-      t.strictEqual(reply.res.getHeader('Content-Type'), 'text/custom')
+      t.strictEqual(reply._headers['content-type'], 'text/custom')
       next()
     })
 
@@ -1675,6 +1675,50 @@ test('onRequest, preHandler, and onResponse hooks that resolve to a value do not
   })
 })
 
+test('If a response header has been set inside an hook it shoulod not be overwritten by the final response handler', t => {
+  t.plan(5)
+  const fastify = Fastify()
+
+  fastify.addHook('onRequest', (req, res, next) => {
+    res.setHeader('X-Custom-Header', 'hello')
+    next()
+  })
+
+  fastify.get('/', (request, reply) => {
+    reply.send('hello')
+  })
+
+  fastify.inject('/', (err, res) => {
+    t.error(err)
+    t.strictEqual(res.headers['x-custom-header'], 'hello')
+    t.strictEqual(res.headers['content-type'], 'text/plain')
+    t.strictEqual(res.statusCode, 200)
+    t.strictEqual(res.payload, 'hello')
+  })
+})
+
+test('If the content type has been set inside an hook it should not be changed', t => {
+  t.plan(5)
+  const fastify = Fastify()
+
+  fastify.addHook('onRequest', (req, res, next) => {
+    res.setHeader('content-type', 'text/html')
+    next()
+  })
+
+  fastify.get('/', (request, reply) => {
+    t.notOk(reply._headers['content-type'])
+    reply.send('hello')
+  })
+
+  fastify.inject('/', (err, res) => {
+    t.error(err)
+    t.strictEqual(res.headers['content-type'], 'text/html')
+    t.strictEqual(res.statusCode, 200)
+    t.strictEqual(res.payload, 'hello')
+  })
+})
+
 if (Number(process.versions.node[0]) >= 8) {
   require('./hooks-async')(t)
 } else {

test/internals/handleRequest.test.js

@@ -44,10 +44,7 @@ test('handler function - invalid schema', t => {
     t.equal(res.statusCode, 400)
     t.pass()
   }
-  res.setHeader = (key, value) => {
-    return
-  }
-  res.getHeader = (key) => {
+  res.writeHead = () => {
     return
   }
   res.log = { error: () => {}, info: () => {} }
@@ -81,10 +78,7 @@ test('handler function - reply', t => {
     t.equal(res.statusCode, 204)
     t.pass()
   }
-  res.getHeader = (key) => {
-    return false
-  }
-  res.setHeader = (key, value) => {
+  res.writeHead = () => {
     return
   }
   const context = {