From 3634c6e01e4049e946da68647a6aaf5847dbccbd Mon Sep 17 00:00:00 2001 From: Ayoub El Khattabi Date: Mon, 25 Jan 2021 09:54:18 +0100 Subject: [PATCH 1/3] Exposing fastify `httpHandler` and `defaultRoute` function, and adding support for hijacking FastifyReply (#2733) --- docs/Lifecycle.md | 9 +- docs/Reply.md | 9 ++ docs/Server.md | 28 +++++ fastify.js | 4 + lib/errors.js | 6 + lib/reply.js | 5 + lib/route.js | 13 ++- test/default-route.test.js | 43 +++++++ test/skip-reply-send.test.js | 219 ++++++++++++++++++++++++++++++++++- types/instance.d.ts | 5 +- types/route.d.ts | 5 + 11 files changed, 342 insertions(+), 4 deletions(-) create mode 100644 test/default-route.test.js diff --git a/docs/Lifecycle.md b/docs/Lifecycle.md index a6beabedf76..d18d9c4080c 100644 --- a/docs/Lifecycle.md +++ b/docs/Lifecycle.md @@ -36,6 +36,12 @@ Incoming Request └─▶ onResponse Hook ``` +At any point before or during the `User Handler`, `reply.hijack()` can be called to prevent fastify from: +- Running all the following hooks and user handler +- Sending the response automatically + +NB (*): If `reply.raw` is used to send a response back to the user, `onResponse` hooks will still be executed + ## Reply Lifecycle Whenever the user handles the request the result may be: @@ -45,7 +51,7 @@ Whenever the user handles the request the result may be: - in sync handler: it sends a payload - in sync handler: it sends an `Error` instance -So, when the reply is being submitted, the data flow performed is the following: +If the reply was hijacked, we skip all the below steps. Otherwise, when it is being submitted, the data flow performed is the following: ``` ★ schema validation Error @@ -56,6 +62,7 @@ So, when the reply is being submitted, the data flow performed is the following: │ │ ★ throw an Error ★ send or return │ │ + │ │ │ │ ▼ │ reply sent ◀── JSON ─┴─ Error instance ──▶ setErrorHandler ◀─────┘ │ diff --git a/docs/Reply.md b/docs/Reply.md index 450246b001d..13e7bd7f21f 100644 --- a/docs/Reply.md +++ b/docs/Reply.md @@ -18,6 +18,7 @@ - [.raw](#raw) - [.serializer(func)](#serializerfunc) - [.sent](#sent) + - [.hijack](#hijack) - [.send(data)](#senddata) - [Objects](#objects) - [Strings](#strings) @@ -251,6 +252,14 @@ app.get('/', (req, reply) => { If the handler rejects, the error will be logged. + +### .hijack() +Sometimes you might need to halt the execution of the normal request lifecycle and handle sending the response manually. + +To achieve this, fastify provides the method `reply.hijack()` that can be called during the request lifecycle (At any point before `reply.send()` is called), and allows you to prevent fastify from sending the response, and from running the remaining hooks (and user handler if the reply was hijacked before). + +NB (*): If `reply.raw` is used to send a response back to the user, `onResponse` hooks will still be executed + ### .send(data) As the name suggests, `.send()` is the function that sends the payload to the end user. diff --git a/docs/Server.md b/docs/Server.md index 2eb785a8af9..ab51e95d90e 100644 --- a/docs/Server.md +++ b/docs/Server.md @@ -675,6 +675,34 @@ fastify.listen({ }, (err) => {}) ``` + +#### getDefaultRoute +Method to get the `defaultRoute` for the server: + +```js +const defaultRoute = fastify.getDefaultRoute() +``` + + +#### setDefaultRoute +Method to set the `defaultRoute` for the server: + +```js +const defaultRoute = function (req, res) { + res.end('hello world') +} + +fastify.setDefaultRoute(defaultRoute) +``` + + +#### routing +Method to access the `lookup` method of the internal router and match the request to the appropriate handler: + +```js +fastify.routing(req, res) +``` + #### route Method to add routes to the server, it also has shorthand functions, check [here](Routes.md). diff --git a/fastify.js b/fastify.js index 72842ee33a3..20c5de71dac 100644 --- a/fastify.js +++ b/fastify.js @@ -189,6 +189,10 @@ function fastify (options) { [pluginUtils.registeredPlugins]: [], [kPluginNameChain]: [], [kAvvioBoot]: null, + // routing method + routing: httpHandler, + getDefaultRoute: router.getDefaultRoute.bind(router), + setDefaultRoute: router.setDefaultRoute.bind(router), // routes shorthand methods delete: function _delete (url, opts, handler) { return router.prepareRoute.call(this, 'DELETE', url, opts, handler) diff --git a/lib/errors.js b/lib/errors.js index bfb609494e1..022937c2ed0 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -200,6 +200,12 @@ const codes = { "'%s' is not a valid url component", 400 ), + FST_ERR_DEFAULT_ROUTE_INVALID_TYPE: createError( + 'FST_ERR_DEFAULT_ROUTE_INVALID_TYPE', + 'The defaultRoute type should be a function', + 500, + TypeError + ), /** * again listen when close server diff --git a/lib/reply.js b/lib/reply.js index 245e490e6c9..62b74634c69 100644 --- a/lib/reply.js +++ b/lib/reply.js @@ -104,6 +104,11 @@ Object.defineProperties(Reply.prototype, { } }) +Reply.prototype.hijack = function () { + this[kReplySent] = true + return this +} + Reply.prototype.send = function (payload) { if (this[kReplyIsRunningOnErrorHook] === true) { throw new FST_ERR_SEND_INSIDE_ONERR() diff --git a/lib/route.js b/lib/route.js index 8022a041fcc..b763f52db8d 100644 --- a/lib/route.js +++ b/lib/route.js @@ -20,7 +20,8 @@ const { const { FST_ERR_SCH_VALIDATION_BUILD, - FST_ERR_SCH_SERIALIZATION_BUILD + FST_ERR_SCH_SERIALIZATION_BUILD, + FST_ERR_DEFAULT_ROUTE_INVALID_TYPE } = require('./errors') const { @@ -87,6 +88,16 @@ function buildRouting (options) { routing: router.lookup.bind(router), // router func to find the right handler to call route, // configure a route in the fastify instance prepareRoute, + getDefaultRoute: function () { + return router.defaultRoute + }, + setDefaultRoute: function (defaultRoute) { + if (typeof defaultRoute !== 'function') { + throw new FST_ERR_DEFAULT_ROUTE_INVALID_TYPE() + } + + router.defaultRoute = defaultRoute + }, routeHandler, closeRoutes: () => { closing = true }, printRoutes: router.prettyPrint.bind(router) diff --git a/test/default-route.test.js b/test/default-route.test.js new file mode 100644 index 00000000000..64de3c259cd --- /dev/null +++ b/test/default-route.test.js @@ -0,0 +1,43 @@ +'use strict' + +const t = require('tap') +const test = t.test +const Fastify = require('..') + +test('should fail if defaultRoute is not a function', t => { + t.plan(1) + + const fastify = Fastify() + const defaultRoute = {} + + fastify.get('/', () => {}) + + try { + fastify.setDefaultRoute(defaultRoute) + } catch (error) { + t.equal(error.code, 'FST_ERR_DEFAULT_ROUTE_INVALID_TYPE') + } +}) + +test('correctly sets, returns, and calls defaultRoute', t => { + t.plan(3) + + const fastify = Fastify() + const defaultRoute = (req, res) => { + res.end('hello from defaultRoute') + } + + fastify.setDefaultRoute(defaultRoute) + const returnedDefaultRoute = fastify.getDefaultRoute() + t.equal(returnedDefaultRoute, defaultRoute) + + fastify.get('/', () => {}) + + fastify.inject({ + method: 'GET', + url: '/random' + }, (err, res) => { + t.error(err) + t.equal(res.body, 'hello from defaultRoute') + }) +}) diff --git a/test/skip-reply-send.test.js b/test/skip-reply-send.test.js index df5edd25b77..7726dd69f99 100644 --- a/test/skip-reply-send.test.js +++ b/test/skip-reply-send.test.js @@ -2,7 +2,22 @@ const { test } = require('tap') const split = require('split2') -const Fastify = require('..') +const net = require('net') +const Fastify = require('../fastify') + +process.removeAllListeners('warning') + +const lifecycleHooks = [ + 'onRequest', + 'preParsing', + 'preValidation', + 'preHandler', + 'preSerialization', + 'onSend', + 'onTimeout', + 'onResponse', + 'onError' +] test('skip automatic reply.send() with reply.sent = true and a body', (t) => { const stream = split(JSON.parse) @@ -96,3 +111,205 @@ test('skip automatic reply.send() with reply.sent = true and an error', (t) => { t.equal(res.body, 'hello world') }) }) + +function testHandlerOrBeforeHandlerHook (test, hookOrHandler) { + const idx = hookOrHandler === 'handler' ? lifecycleHooks.indexOf('preHandler') : lifecycleHooks.indexOf(hookOrHandler) + const previousHooks = lifecycleHooks.slice(0, idx) + const nextHooks = lifecycleHooks.slice(idx + 1) + + test(`Hijacking inside ${hookOrHandler} skips all the following hooks and handler execution`, t => { + t.plan(4) + const test = t.test + + test('Sending a response using reply.raw => onResponse hook is called', t => { + const stream = split(JSON.parse) + const app = Fastify({ + logger: { + stream: stream + } + }) + + stream.on('data', (line) => { + t.notEqual(line.level, 40) // there are no errors + t.notEqual(line.level, 50) // there are no errors + }) + + previousHooks.forEach(h => app.addHook(h, async (req, reply) => t.pass(`${h} should be called`))) + + if (hookOrHandler === 'handler') { + app.get('/', (req, reply) => { + reply.hijack() + reply.raw.end(`hello from ${hookOrHandler}`) + }) + } else { + app.addHook(hookOrHandler, async (req, reply) => { + reply.hijack() + reply.raw.end(`hello from ${hookOrHandler}`) + }) + app.get('/', (req, reply) => t.fail('Handler should not be called')) + } + + nextHooks.forEach(h => { + if (h === 'onResponse') { + app.addHook(h, async (req, reply) => t.pass(`${h} should be called`)) + } else { + app.addHook(h, async (req, reply) => t.fail(`${h} should not be called`)) + } + }) + + return app.inject({ + method: 'GET', + url: '/' + }).then((res) => { + t.equal(res.statusCode, 200) + t.equal(res.body, `hello from ${hookOrHandler}`) + }) + }) + + test('Sending a response using req.socket => onResponse not called', t => { + const stream = split(JSON.parse) + const app = Fastify({ + logger: { + stream: stream + } + }) + t.tearDown(() => app.close()) + + stream.on('data', (line) => { + t.notEqual(line.level, 40) // there are no errors + t.notEqual(line.level, 50) // there are no errors + }) + + previousHooks.forEach(h => app.addHook(h, async (req, reply) => t.pass(`${h} should be called`))) + + if (hookOrHandler === 'handler') { + app.get('/', (req, reply) => { + reply.hijack() + req.socket.write('HTTP/1.1 200 OK\r\n\r\n') + req.socket.write(`hello from ${hookOrHandler}`) + req.socket.end() + }) + } else { + app.addHook(hookOrHandler, async (req, reply) => { + reply.hijack() + req.socket.write('HTTP/1.1 200 OK\r\n\r\n') + req.socket.write(`hello from ${hookOrHandler}`) + req.socket.end() + }) + app.get('/', (req, reply) => t.fail('Handler should not be called')) + } + + nextHooks.forEach(h => app.addHook(h, async (req, reply) => t.fail(`${h} should not be called`))) + + app.listen(0, err => { + t.error(err) + const client = net.createConnection({ port: (app.server.address()).port }, () => { + client.write('GET / HTTP/1.1\r\n\r\n') + client.once('data', data => { + t.match(data.toString(), new RegExp(`hello from ${hookOrHandler}`, 'i')) + client.end(() => t.end()) + }) + }) + }) + }) + + test('Throwing an error doesnt trigger any hooks', t => { + const stream = split(JSON.parse) + const app = Fastify({ + logger: { + stream: stream + } + }) + t.tearDown(() => app.close()) + + let errorSeen = false + stream.on('data', (line) => { + if (hookOrHandler === 'handler') { + if (line.level === 40) { + errorSeen = true + t.equal(line.err.code, 'FST_ERR_REP_ALREADY_SENT') + } + } else { + t.notEqual(line.level, 40) // there are no errors + t.notEqual(line.level, 50) // there are no errors + } + }) + + previousHooks.forEach(h => app.addHook(h, async (req, reply) => t.pass(`${h} should be called`))) + + if (hookOrHandler === 'handler') { + app.get('/', (req, reply) => { + reply.hijack() + throw new Error('This wil be skipped') + }) + } else { + app.addHook(hookOrHandler, async (req, reply) => { + reply.hijack() + throw new Error('This wil be skipped') + }) + app.get('/', (req, reply) => t.fail('Handler should not be called')) + } + + nextHooks.forEach(h => app.addHook(h, async (req, reply) => t.fail(`${h} should not be called`))) + + return Promise.race([ + app.inject({ method: 'GET', url: '/' }), + new Promise((resolve, reject) => setTimeout(resolve, 1000)) + ]).then((err, res) => { + t.error(err) + if (hookOrHandler === 'handler') { + t.equal(errorSeen, true) + } + }) + }) + + test('Calling reply.send() after hijacking logs a warning', t => { + const stream = split(JSON.parse) + const app = Fastify({ + logger: { + stream: stream + } + }) + + let errorSeen = false + + stream.on('data', (line) => { + if (line.level === 40) { + errorSeen = true + t.equal(line.err.code, 'FST_ERR_REP_ALREADY_SENT') + } + }) + + previousHooks.forEach(h => app.addHook(h, async (req, reply) => t.pass(`${h} should be called`))) + + if (hookOrHandler === 'handler') { + app.get('/', (req, reply) => { + reply.hijack() + reply.send('hello from reply.send()') + }) + } else { + app.addHook(hookOrHandler, async (req, reply) => { + reply.hijack() + reply.send('hello from reply.send()') + }) + app.get('/', (req, reply) => t.fail('Handler should not be called')) + } + + nextHooks.forEach(h => app.addHook(h, async (req, reply) => t.fail(`${h} should not be called`))) + + return Promise.race([ + app.inject({ method: 'GET', url: '/' }), + new Promise((resolve, reject) => setTimeout(resolve, 1000)) + ]).then((err, res) => { + t.error(err) + t.equal(errorSeen, true) + }) + }) + }) +} + +testHandlerOrBeforeHandlerHook(test, 'onRequest') +testHandlerOrBeforeHandlerHook(test, 'preParsing') +testHandlerOrBeforeHandlerHook(test, 'preValidation') +testHandlerOrBeforeHandlerHook(test, 'preHandler') +testHandlerOrBeforeHandlerHook(test, 'handler') diff --git a/types/instance.d.ts b/types/instance.d.ts index d801d3f490a..76e58b9d70e 100644 --- a/types/instance.d.ts +++ b/types/instance.d.ts @@ -1,5 +1,5 @@ import { Chain as LightMyRequestChain, InjectOptions, Response as LightMyRequestResponse, CallbackFunc as LightMyRequestCallback } from 'light-my-request' -import { RouteOptions, RouteShorthandMethod, RouteGenericInterface } from './route' +import { RouteOptions, RouteShorthandMethod, RouteGenericInterface, DefaultRoute } from './route' import { FastifySchemaCompiler, FastifySchemaValidationError, FastifySerializerCompiler } from './schema' import { RawServerBase, RawRequestDefaultExpression, RawServerDefault, RawReplyDefaultExpression, ContextConfigDefault } from './utils' import { FastifyLoggerInstance } from './logger' @@ -59,6 +59,9 @@ export interface FastifyInstance< register: FastifyRegister & PromiseLike>; + getDefaultRoute: DefaultRoute; + setDefaultRoute(defaultRoute: DefaultRoute): void; + route< RouteGeneric extends RouteGenericInterface = RouteGenericInterface, ContextConfig = ContextConfigDefault diff --git a/types/route.d.ts b/types/route.d.ts index 8db758b90ce..00f12dea409 100644 --- a/types/route.d.ts +++ b/types/route.d.ts @@ -122,3 +122,8 @@ export type RouteHandler< request: FastifyRequest, reply: FastifyReply ) => void | Promise + +export type DefaultRoute = ( + req: Request, + res: Reply, +) => void; From f3d5742f5398e37401d9ea0766d9a37d6f38f08a Mon Sep 17 00:00:00 2001 From: Mohamed Edrah <43171151+MSE99@users.noreply.github.com> Date: Mon, 25 Jan 2021 11:49:45 +0200 Subject: [PATCH 2/3] Fix(2788): by preventing invalid route creation (#2790) * Fixes #2788 by preventing invalid route creation * Explicitly set path on the `HEAD` route opts * Improve test assertions Co-authored-by: Manuel Spigolon Co-authored-by: Manuel Spigolon --- lib/route.js | 3 ++- test/route.test.js | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lib/route.js b/lib/route.js index b763f52db8d..8a3fa800cfe 100644 --- a/lib/route.js +++ b/lib/route.js @@ -123,6 +123,7 @@ function buildRouting (options) { options = Object.assign({}, options, { method, url, + path: url, handler: handler || (options && options.handler) }) @@ -163,7 +164,7 @@ function buildRouting (options) { this.after((notHandledErr, done) => { const path = opts.url || opts.path - if (path === '/' && prefix.length > 0) { + if (path === '/' && prefix.length > 0 && opts.method !== 'HEAD') { switch (opts.prefixTrailingSlash) { case 'slash': afterRouteAdded.call(this, { path }, notHandledErr, done) diff --git a/test/route.test.js b/test/route.test.js index c00f2e8596b..3133e73b577 100644 --- a/test/route.test.js +++ b/test/route.test.js @@ -1060,3 +1060,56 @@ test('Set a custom HEAD route before GET one without disabling exposeHeadRoutes t.strictEqual(res.body, '') }) }) + +test('HEAD routes properly auto created for GET routes when prefixTrailingSlash: \'no-slash\'', t => { + t.plan(2) + + const fastify = Fastify() + + fastify.register(function routes (f, opts, next) { + f.route({ + method: 'GET', + url: '/', + exposeHeadRoute: true, + prefixTrailingSlash: 'no-slash', + handler: (req, reply) => { + reply.send({ hello: 'world' }) + } + }) + + next() + }, { prefix: '/prefix' }) + + fastify.inject({ url: '/prefix/prefix', method: 'HEAD' }, (err, res) => { + t.error(err) + t.strictEquals(res.statusCode, 404) + }) +}) + +test('HEAD routes properly auto created for GET routes when prefixTrailingSlash: \'both\'', async t => { + t.plan(3) + + const fastify = Fastify() + + fastify.register(function routes (f, opts, next) { + f.route({ + method: 'GET', + url: '/', + exposeHeadRoute: true, + prefixTrailingSlash: 'both', + handler: (req, reply) => { + reply.send({ hello: 'world' }) + } + }) + + next() + }, { prefix: '/prefix' }) + + const doublePrefixReply = await fastify.inject({ url: '/prefix/prefix', method: 'HEAD' }) + const trailingSlashReply = await fastify.inject({ url: '/prefix/', method: 'HEAD' }) + const noneTrailingReply = await fastify.inject({ url: '/prefix', method: 'HEAD' }) + + t.equals(doublePrefixReply.statusCode, 404) + t.equals(trailingSlashReply.statusCode, 200) + t.equals(noneTrailingReply.statusCode, 200) +}) From d1f7754635b9f9c4e714953fdd0c152ac426ea41 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 25 Jan 2021 10:52:06 +0100 Subject: [PATCH 3/3] Bumped v3.11.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bd5e00d83fe..cc25d80ed93 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fastify", - "version": "3.10.1", + "version": "3.11.0", "description": "Fast and low overhead web framework, for Node.js", "main": "fastify.js", "type": "commonjs",