From c2dea6ba858dc520b5553355b8ad9adbfc665caf Mon Sep 17 00:00:00 2001 From: KaKa Date: Sun, 27 Mar 2022 22:41:32 +0800 Subject: [PATCH 1/2] feat: reply trailers support (#3794) --- docs/Reference/Reply.md | 49 +++++++ lib/errors.js | 8 ++ lib/reply.js | 89 +++++++++++- lib/symbols.js | 1 + test/internals/reply.test.js | 3 + test/reply-trailers.test.js | 270 +++++++++++++++++++++++++++++++++++ 6 files changed, 417 insertions(+), 3 deletions(-) create mode 100644 test/reply-trailers.test.js diff --git a/docs/Reference/Reply.md b/docs/Reference/Reply.md index ae08b2f3ec..081185d1e0 100644 --- a/docs/Reference/Reply.md +++ b/docs/Reference/Reply.md @@ -13,6 +13,9 @@ - [.getHeaders()](#getheaders) - [.removeHeader(key)](#removeheaderkey) - [.hasHeader(key)](#hasheaderkey) + - [.trailer(key, function)](#trailerkey-function) + - [.hasTrailer(key)](#hastrailerkey) + - [.removeTrailer(key)](#removetrailerkey) - [.redirect([code,] dest)](#redirectcode--dest) - [.callNotFound()](#callnotfound) - [.getResponseTime()](#getresponsetime) @@ -47,6 +50,9 @@ object that exposes the following functions and properties: - `.getHeaders()` - Gets a shallow copy of all current response headers. - `.removeHeader(key)` - Remove the value of a previously set header. - `.hasHeader(name)` - Determine if a header has been set. +- `.trailer(key, function)` - Sets a response trailer. +- `.hasTrailer(key)` - Determine if a trailer has been set. +- `.removeTrailer(key)` - Remove the value of a previously set trailer. - `.type(value)` - Sets the header `Content-Type`. - `.redirect([code,] dest)` - Redirect to the specified url, the status code is optional (default to `302`). @@ -199,6 +205,49 @@ reply.getHeader('x-foo') // undefined Returns a boolean indicating if the specified header has been set. +### .trailer(key, function) + + +Sets a response trailer. Trailer usually used when you want some header that require heavy resources to be sent after the `data`, for example `Server-Timing`, `Etag`. It can ensure the client get the response data as soon as possible. + +*Note: The header `Transfer-Encoding: chunked` will be added once you use the trailer. It is a hard requipment for using trailer in Node.js.* + +*Note: Currently, the computation function only supports synchronous function. That means `async-await` and `promise` are not supported.* + +```js +reply.trailer('server-timing', function() { + return 'db;dur=53, app;dur=47.2' +}) + +const { createHash } = require('crypto') +// trailer function also recieve two argument +// @param {object} reply fastify reply +// @param {string|Buffer|null} payload payload that already sent, note that it will be null when stream is sent +reply.trailer('content-md5', function(reply, payload) { + const hash = createHash('md5') + hash.update(payload) + return hash.disgest('hex') +}) +``` + +### .hasTrailer(key) + + +Returns a boolean indicating if the specified trailer has been set. + +### .removeTrailer(key) + + +Remove the value of a previously set trailer. +```js +reply.trailer('server-timing', function() { + return 'db;dur=53, app;dur=47.2' +}) +reply.removeTrailer('server-timing') +reply.getTrailer('server-timing') // undefined +``` + + ### .redirect([code ,] dest) diff --git a/lib/errors.js b/lib/errors.js index eec1392cc1..6d64d6a7e0 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -147,6 +147,14 @@ const codes = { 'FST_ERR_BAD_STATUS_CODE', 'Called reply with an invalid status code: %s' ), + FST_ERR_BAD_TRAILER_NAME: createError( + 'FST_ERR_BAD_TRAILER_NAME', + 'Called reply.trailer with an invalid header name: %s' + ), + FST_ERR_BAD_TRAILER_VALUE: createError( + 'FST_ERR_BAD_TRAILER_VALUE', + "Called reply.trailer('%s', fn) with an invalid type: %s. Expected a function." + ), /** * schemas diff --git a/lib/reply.js b/lib/reply.js index f852ad60d2..920b81ea6b 100644 --- a/lib/reply.js +++ b/lib/reply.js @@ -16,6 +16,7 @@ const { kReplySerializerDefault, kReplyIsError, kReplyHeaders, + kReplyTrailers, kReplyHasStatusCode, kReplyIsRunningOnErrorHook, kDisableRequestLogging @@ -47,7 +48,9 @@ const { FST_ERR_REP_ALREADY_SENT, FST_ERR_REP_SENT_VALUE, FST_ERR_SEND_INSIDE_ONERR, - FST_ERR_BAD_STATUS_CODE + FST_ERR_BAD_STATUS_CODE, + FST_ERR_BAD_TRAILER_NAME, + FST_ERR_BAD_TRAILER_VALUE } = require('./errors') const warning = require('./warnings') @@ -60,6 +63,7 @@ function Reply (res, request, log) { this[kReplyIsRunningOnErrorHook] = false this.request = request this[kReplyHeaders] = {} + this[kReplyTrailers] = null this[kReplyHasStatusCode] = false this[kReplyStartTime] = undefined this.log = log @@ -261,6 +265,47 @@ Reply.prototype.headers = function (headers) { return this } +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer#directives +// https://httpwg.org/specs/rfc7230.html#chunked.trailer.part +const INVALID_TRAILERS = new Set([ + 'transfer-encoding', + 'content-length', + 'host', + 'cache-control', + 'max-forwards', + 'te', + 'authorization', + 'set-cookie', + 'content-encoding', + 'content-type', + 'content-range', + 'trailer' +]) + +Reply.prototype.trailer = function (key, fn) { + key = key.toLowerCase() + if (INVALID_TRAILERS.has(key)) { + throw new FST_ERR_BAD_TRAILER_NAME(key) + } + if (typeof fn !== 'function') { + throw new FST_ERR_BAD_TRAILER_VALUE(key, typeof fn) + } + if (this[kReplyTrailers] === null) this[kReplyTrailers] = {} + this[kReplyTrailers][key] = fn + return this +} + +Reply.prototype.hasTrailer = function (key) { + if (this[kReplyTrailers] === null) return false + return this[kReplyTrailers][key.toLowerCase()] !== undefined +} + +Reply.prototype.removeTrailer = function (key) { + if (this[kReplyTrailers] === null) return this + this[kReplyTrailers][key.toLowerCase()] = undefined + return this +} + Reply.prototype.code = function (code) { const intValue = parseInt(code) if (isNaN(intValue) || intValue < 100 || intValue > 600) { @@ -416,6 +461,20 @@ function onSendEnd (reply, payload) { const req = reply.request const statusCode = res.statusCode + // we check if we need to update the trailers header and set it + if (reply[kReplyTrailers] !== null) { + const trailerHeaders = Object.keys(reply[kReplyTrailers]) + let header = '' + for (const trailerName of trailerHeaders) { + if (typeof reply[kReplyTrailers][trailerName] !== 'function') continue + header += ' ' + header += trailerName + } + // it must be chunked for trailer to work + reply.header('Transfer-Encoding', 'chunked') + reply.header('Trailer', header.trim()) + } + if (payload === undefined || payload === null) { reply[kReplySent] = true @@ -428,6 +487,7 @@ function onSendEnd (reply, payload) { } res.writeHead(statusCode, reply[kReplyHeaders]) + sendTrailer(payload, res, reply) // avoid ArgumentsAdaptorTrampoline from V8 res.end(null, null, null) return @@ -453,9 +513,12 @@ function onSendEnd (reply, payload) { reply[kReplySent] = true res.writeHead(statusCode, reply[kReplyHeaders]) - + // write payload first + res.write(payload) + // then send trailers + sendTrailer(payload, res, reply) // avoid ArgumentsAdaptorTrampoline from V8 - res.end(payload, null, null) + res.end(null, null, null) } function logStreamError (logger, err, res) { @@ -472,6 +535,9 @@ function sendStream (payload, res, reply) { let sourceOpen = true let errorLogged = false + // set trailer when stream ended + sendStreamTrailer(payload, res, reply) + eos(payload, { readable: true, writable: false }, function (err) { sourceOpen = false if (err != null) { @@ -520,6 +586,22 @@ function sendStream (payload, res, reply) { payload.pipe(res) } +function sendTrailer (payload, res, reply) { + if (reply[kReplyTrailers] === null) return + const trailerHeaders = Object.keys(reply[kReplyTrailers]) + const trailers = {} + for (const trailerName of trailerHeaders) { + if (typeof reply[kReplyTrailers][trailerName] !== 'function') continue + trailers[trailerName] = reply[kReplyTrailers][trailerName](reply, payload) + } + res.addTrailers(trailers) +} + +function sendStreamTrailer (payload, res, reply) { + if (reply[kReplyTrailers] === null) return + payload.on('end', () => sendTrailer(null, res, reply)) +} + function onErrorHook (reply, error, cb) { reply[kReplySent] = true if (reply.context.onError !== null && reply[kReplyErrorHandlerCalled] === true) { @@ -684,6 +766,7 @@ function buildReply (R) { this[kReplySerializer] = null this.request = request this[kReplyHeaders] = {} + this[kReplyTrailers] = null this[kReplyStartTime] = undefined this[kReplyEndTime] = undefined this.log = log diff --git a/lib/symbols.js b/lib/symbols.js index 63925ae1a3..77af5bf6ef 100644 --- a/lib/symbols.js +++ b/lib/symbols.js @@ -30,6 +30,7 @@ const keys = { kReplySerializer: Symbol('fastify.reply.serializer'), kReplyIsError: Symbol('fastify.reply.isError'), kReplyHeaders: Symbol('fastify.reply.headers'), + kReplyTrailers: Symbol('fastify.reply.trailers'), kReplyHasStatusCode: Symbol('fastify.reply.hasStatusCode'), kReplySent: Symbol('fastify.reply.sent'), kReplySentOverwritten: Symbol('fastify.reply.sentOverwritten'), diff --git a/test/internals/reply.test.js b/test/internals/reply.test.js index 3d42cd0d8b..54ec88e70f 100644 --- a/test/internals/reply.test.js +++ b/test/internals/reply.test.js @@ -53,6 +53,7 @@ test('reply.send will logStream error and destroy the stream', { only: true }, t hasHeader: () => false, getHeader: () => undefined, writeHead: () => {}, + write: () => {}, headersSent: true }) @@ -74,6 +75,7 @@ test('reply.send throw with circular JSON', t => { hasHeader: () => false, getHeader: () => undefined, writeHead: () => {}, + write: () => {}, end: () => {} } const reply = new Reply(response, { context: { onSend: [] } }) @@ -91,6 +93,7 @@ test('reply.send returns itself', t => { hasHeader: () => false, getHeader: () => undefined, writeHead: () => {}, + write: () => {}, end: () => {} } const reply = new Reply(response, { context: { onSend: [] } }) diff --git a/test/reply-trailers.test.js b/test/reply-trailers.test.js new file mode 100644 index 0000000000..8757913c22 --- /dev/null +++ b/test/reply-trailers.test.js @@ -0,0 +1,270 @@ +'use strict' + +const t = require('tap') +const test = t.test +const Fastify = require('..') +const { Readable } = require('stream') +const { createHash } = require('crypto') + +test('send trailers when payload is empty string', t => { + t.plan(4) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + reply.trailer('ETag', function (reply, payload) { + return 'custom-etag' + }) + reply.send('') + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + }) +}) + +test('send trailers when payload is empty buffer', t => { + t.plan(4) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + reply.trailer('ETag', function (reply, payload) { + return 'custom-etag' + }) + reply.send(Buffer.alloc(0)) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + }) +}) + +test('send trailers when payload is undefined', t => { + t.plan(4) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + reply.trailer('ETag', function (reply, payload) { + return 'custom-etag' + }) + reply.send(undefined) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + }) +}) + +test('send trailers when payload is json', t => { + t.plan(6) + + const fastify = Fastify() + const data = JSON.stringify({ hello: 'world' }) + const hash = createHash('md5') + hash.update(data) + const md5 = hash.digest('hex') + + fastify.get('/', function (request, reply) { + reply.trailer('Content-MD5', function (reply, payload) { + t.equal(data, payload) + const hash = createHash('md5') + hash.update(payload) + return hash.digest('hex') + }) + reply.send(data) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers['transfer-encoding'], 'chunked') + t.equal(res.headers.trailer, 'content-md5') + t.equal(res.trailers['content-md5'], md5) + }) +}) + +test('send trailers when payload is stream', t => { + t.plan(6) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + reply.trailer('ETag', function (reply, payload) { + t.same(payload, null) + return 'custom-etag' + }) + const stream = Readable.from([JSON.stringify({ hello: 'world' })]) + reply.send(stream) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers['transfer-encoding'], 'chunked') + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + }) +}) + +test('removeTrailer', t => { + t.plan(5) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + reply.removeTrailer('ETag') // remove nothing + reply.trailer('ETag', function (reply, payload) { + return 'custom-etag' + }) + reply.trailer('Should-Not-Call', function (reply, payload) { + t.fail('it should not called as this trailer is removed') + return 'should-not-call' + }) + reply.removeTrailer('Should-Not-Call') + reply.send(undefined) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + t.notOk(res.trailers['should-not-call']) + }) +}) + +test('hasTrailer', t => { + t.plan(9) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + t.equal(reply.hasTrailer('ETag'), false) + reply.trailer('ETag', function (reply, payload) { + return 'custom-etag' + }) + t.equal(reply.hasTrailer('ETag'), true) + reply.trailer('Should-Not-Call', function (reply, payload) { + t.fail('it should not called as this trailer is removed') + return 'should-not-call' + }) + t.equal(reply.hasTrailer('Should-Not-Call'), true) + reply.removeTrailer('Should-Not-Call') + t.equal(reply.hasTrailer('Should-Not-Call'), false) + reply.send(undefined) + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + t.equal(res.headers.trailer, 'etag') + t.equal(res.trailers.etag, 'custom-etag') + t.notOk(res.trailers['should-not-call']) + }) +}) + +test('throw error when trailer header name is not allowed', t => { + const INVALID_TRAILERS = [ + 'transfer-encoding', + 'content-length', + 'host', + 'cache-control', + 'max-forwards', + 'te', + 'authorization', + 'set-cookie', + 'content-encoding', + 'content-type', + 'content-range', + 'trailer' + ] + t.plan(INVALID_TRAILERS.length + 2) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + for (const key of INVALID_TRAILERS) { + try { + reply.trailer(key, () => {}) + } catch (err) { + t.equal(err.message, `Called reply.trailer with an invalid header name: ${key}`) + } + } + reply.send('') + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + }) +}) + +test('throw error when trailer header value is not function', t => { + const INVALID_TRAILERS_VALUE = [ + undefined, + null, + true, + false, + 'invalid', + [], + new Date(), + {} + ] + t.plan(INVALID_TRAILERS_VALUE.length + 2) + + const fastify = Fastify() + + fastify.get('/', function (request, reply) { + for (const value of INVALID_TRAILERS_VALUE) { + try { + reply.trailer('invalid', value) + } catch (err) { + t.equal(err.message, `Called reply.trailer('invalid', fn) with an invalid type: ${typeof value}. Expected a function.`) + } + } + reply.send('') + }) + + fastify.inject({ + method: 'GET', + url: '/' + }, (error, res) => { + t.error(error) + t.equal(res.statusCode, 200) + }) +}) From e36570110c5832c166bc0a0f201315380d2ae2c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E8=8F=9C?= Date: Sat, 2 Apr 2022 17:36:31 +0800 Subject: [PATCH 2/2] chore: remove content-length when Transfer-Encoding is added (#3814) --- lib/reply.js | 16 ++++++++++------ test/reply-trailers.test.js | 21 ++++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/lib/reply.js b/lib/reply.js index 920b81ea6b..33b3227ec9 100644 --- a/lib/reply.js +++ b/lib/reply.js @@ -480,9 +480,11 @@ function onSendEnd (reply, payload) { // 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. + // < 200 + // A sender MUST NOT send a Content-Length header field in any message + // that contains a Transfer-Encoding header field. // For HEAD we don't overwrite the `content-length` - if (statusCode >= 200 && statusCode !== 204 && statusCode !== 304 && req.method !== 'HEAD') { + if (statusCode >= 200 && statusCode !== 204 && statusCode !== 304 && req.method !== 'HEAD' && reply[kReplyTrailers] === null) { reply[kReplyHeaders]['content-length'] = '0' } @@ -504,10 +506,12 @@ function onSendEnd (reply, payload) { throw new FST_ERR_REP_INVALID_PAYLOAD_TYPE(typeof payload) } - if (!reply[kReplyHeaders]['content-length']) { - reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) - } else if (req.raw.method !== 'HEAD' && reply[kReplyHeaders]['content-length'] !== Buffer.byteLength(payload)) { - reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) + if (reply[kReplyTrailers] === null) { + if (!reply[kReplyHeaders]['content-length']) { + reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) + } else if (req.raw.method !== 'HEAD' && reply[kReplyHeaders]['content-length'] !== Buffer.byteLength(payload)) { + reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) + } } reply[kReplySent] = true diff --git a/test/reply-trailers.test.js b/test/reply-trailers.test.js index 8757913c22..e9ae60cdf7 100644 --- a/test/reply-trailers.test.js +++ b/test/reply-trailers.test.js @@ -7,7 +7,7 @@ const { Readable } = require('stream') const { createHash } = require('crypto') test('send trailers when payload is empty string', t => { - t.plan(4) + t.plan(5) const fastify = Fastify() @@ -26,11 +26,12 @@ test('send trailers when payload is empty string', t => { t.equal(res.statusCode, 200) t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') + t.notHas(res.headers, 'content-length') }) }) test('send trailers when payload is empty buffer', t => { - t.plan(4) + t.plan(5) const fastify = Fastify() @@ -49,11 +50,12 @@ test('send trailers when payload is empty buffer', t => { t.equal(res.statusCode, 200) t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') + t.notHas(res.headers, 'content-length') }) }) test('send trailers when payload is undefined', t => { - t.plan(4) + t.plan(5) const fastify = Fastify() @@ -72,11 +74,12 @@ test('send trailers when payload is undefined', t => { t.equal(res.statusCode, 200) t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') + t.notHas(res.headers, 'content-length') }) }) test('send trailers when payload is json', t => { - t.plan(6) + t.plan(7) const fastify = Fastify() const data = JSON.stringify({ hello: 'world' }) @@ -103,11 +106,12 @@ test('send trailers when payload is json', t => { t.equal(res.headers['transfer-encoding'], 'chunked') t.equal(res.headers.trailer, 'content-md5') t.equal(res.trailers['content-md5'], md5) + t.notHas(res.headers, 'content-length') }) }) test('send trailers when payload is stream', t => { - t.plan(6) + t.plan(7) const fastify = Fastify() @@ -129,11 +133,12 @@ test('send trailers when payload is stream', t => { t.equal(res.headers['transfer-encoding'], 'chunked') t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') + t.notHas(res.headers, 'content-length') }) }) test('removeTrailer', t => { - t.plan(5) + t.plan(6) const fastify = Fastify() @@ -159,11 +164,12 @@ test('removeTrailer', t => { t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') t.notOk(res.trailers['should-not-call']) + t.notHas(res.headers, 'content-length') }) }) test('hasTrailer', t => { - t.plan(9) + t.plan(10) const fastify = Fastify() @@ -192,6 +198,7 @@ test('hasTrailer', t => { t.equal(res.headers.trailer, 'etag') t.equal(res.trailers.etag, 'custom-etag') t.notOk(res.trailers['should-not-call']) + t.notHas(res.headers, 'content-length') }) })