From 2df8712dd92d5cf9e24292417f43059481f78535 Mon Sep 17 00:00:00 2001 From: KaKa Date: Mon, 10 Oct 2022 15:06:21 +0800 Subject: [PATCH] feat: support async constraint (#4323) --- docs/Reference/Errors.md | 5 ++ docs/Reference/Routes.md | 54 +++++++++++++++ docs/Reference/Server.md | 3 + fastify.js | 67 +++++++++++++------ lib/errors.js | 5 ++ lib/route.js | 7 +- package.json | 2 +- test/constrained-routes.test.js | 114 ++++++++++++++++++++++++++++++++ test/router-options.test.js | 58 +++++++++++++++- 9 files changed, 291 insertions(+), 24 deletions(-) diff --git a/docs/Reference/Errors.md b/docs/Reference/Errors.md index ac70020921..54b224795c 100644 --- a/docs/Reference/Errors.md +++ b/docs/Reference/Errors.md @@ -329,6 +329,11 @@ The HTTP method already has a registered controller for that URL The router received an invalid url. +### FST_ERR_ASYNC_CONSTRAINT + + +The router received error when using asynchronous constraints. + #### FST_ERR_DEFAULT_ROUTE_INVALID_TYPE diff --git a/docs/Reference/Routes.md b/docs/Reference/Routes.md index 90f1703483..03969d5091 100644 --- a/docs/Reference/Routes.md +++ b/docs/Reference/Routes.md @@ -732,6 +732,60 @@ fastify.route({ }) ``` +#### Asynchronous Custom Constraints + +You can provide your custom constraints and the `constraint` criteria can be +fetched from other source, for example `database`. Usage of asynchronous custom +constraint should place at the last resort since it impacts the router +performance. + +```js +function databaseOperation(field, done) { + done(null, field) +} + +const secret = { + // strategy name for referencing in the route handler `constraints` options + name: 'secret', + // storage factory for storing routes in the find-my-way route tree + storage: function () { + let handlers = {} + return { + get: (type) => { return handlers[type] || null }, + set: (type, store) => { handlers[type] = store } + } + }, + // function to get the value of the constraint from each incoming request + deriveConstraint: (req, ctx, done) => { + databaseOperation(req.headers['secret'], done) + }, + // optional flag marking if handlers without constraints can match requests that have a value for this constraint + mustMatchWhenDerived: true +} +``` + +> ## ⚠ Security Notice +> When using with asynchronous constraint. It is highly recommend never return error +> inside the callback. If the error is not preventable, it is recommended to provide +> a custom `frameworkErrors` handler to deal with it. Otherwise, you route selection +> may break or expose sensitive information to attackers. +> +> ```js +> const Fastify = require('fastify') +> +> const fastify = Fastify({ +> frameworkErrors: function(err, res, res) { +> if(err instanceof Fastify.errorCodes.FST_ERR_ASYNC_CONSTRAINT) { +> res.code(400) +> return res.send("Invalid header provided") +> } else { +> res.send(err) +> } +> } +> }) +> ``` + + ### ⚠ HTTP version check Fastify will check the HTTP version of every request, based on configuration diff --git a/docs/Reference/Server.md b/docs/Reference/Server.md index 248e5a105c..c3150dfcdd 100644 --- a/docs/Reference/Server.md +++ b/docs/Reference/Server.md @@ -731,6 +731,9 @@ const fastify = require('fastify')({ if (error instanceof FST_ERR_BAD_URL) { res.code(400) return res.send("Provided url is not valid") + } else if(error instanceof FST_ERR_ASYNC_CONSTRAINT) { + res.code(400) + return res.send("Provided header is not valid") } else { res.send(err) } diff --git a/fastify.js b/fastify.js index 679129db1f..20b9b80dba 100644 --- a/fastify.js +++ b/fastify.js @@ -57,6 +57,7 @@ const { const { defaultInitOptions } = getSecuredInitialConfig const { + FST_ERR_ASYNC_CONSTRAINT, FST_ERR_BAD_URL, FST_ERR_FORCE_CLOSE_CONNECTIONS_IDLE_NOT_AVAILABLE } = errorCodes @@ -182,7 +183,7 @@ function fastify (options) { const fourOhFour = build404(options) // HTTP server and its handler - const httpHandler = wrapRouting(router.routing, options) + const httpHandler = wrapRouting(router, options) // we need to set this before calling createServer options.http2SessionTimeout = initialConfig.http2SessionTimeout @@ -666,6 +667,30 @@ function fastify (options) { res.end(body) } + function buildAsyncConstraintCallback (isAsync, req, res) { + if (isAsync === false) return undefined + return function onAsyncConstraintError (err) { + if (err) { + if (frameworkErrors) { + const id = genReqId(req) + const childLogger = logger.child({ reqId: id }) + + childLogger.info({ req }, 'incoming request') + + const request = new Request(id, null, req, null, childLogger, onBadUrlContext) + const reply = new Reply(res, request, childLogger) + return frameworkErrors(new FST_ERR_ASYNC_CONSTRAINT(), request, reply) + } + const body = '{"error":"Internal Server Error","message":"Unexpected error from async constraint","statusCode":500}' + res.writeHead(500, { + 'Content-Type': 'application/json', + 'Content-Length': body.length + }) + res.end(body) + } + } + } + function setNotFoundHandler (opts, handler) { throwIfAlreadyStarted('Cannot call "setNotFoundHandler" when fastify instance is already started!') @@ -722,6 +747,27 @@ function fastify (options) { opts.includeMeta = opts.includeHooks ? opts.includeMeta ? supportedHooks.concat(opts.includeMeta) : supportedHooks : opts.includeMeta return router.printRoutes(opts) } + + function wrapRouting (router, { rewriteUrl, logger }) { + let isAsync + return function preRouting (req, res) { + // only call isAsyncConstraint once + if (isAsync === undefined) isAsync = router.isAsyncConstraint() + if (rewriteUrl) { + const originalUrl = req.url + const url = rewriteUrl(req) + if (originalUrl !== url) { + logger.debug({ originalUrl, url }, 'rewrite url') + if (typeof url === 'string') { + req.url = url + } else { + req.destroy(new Error(`Rewrite url for "${req.url}" needs to be of type "string" but received "${typeof url}"`)) + } + } + } + router.routing(req, res, buildAsyncConstraintCallback(isAsync, req, res)) + } + } } fastify.errorCodes = errorCodes @@ -734,25 +780,6 @@ function validateSchemaErrorFormatter (schemaErrorFormatter) { } } -function wrapRouting (httpHandler, { rewriteUrl, logger }) { - if (!rewriteUrl) { - return httpHandler - } - return function preRouting (req, res) { - const originalUrl = req.url - const url = rewriteUrl(req) - if (originalUrl !== url) { - logger.debug({ originalUrl, url }, 'rewrite url') - if (typeof url === 'string') { - req.url = url - } else { - req.destroy(new Error(`Rewrite url for "${req.url}" needs to be of type "string" but received "${typeof url}"`)) - } - } - httpHandler(req, res) - } -} - /** * These export configurations enable JS and TS developers * to consumer fastify in whatever way best suits their needs. diff --git a/lib/errors.js b/lib/errors.js index d06288473b..a992a202ea 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -226,6 +226,11 @@ const codes = { "'%s' is not a valid url component", 400 ), + FST_ERR_ASYNC_CONSTRAINT: createError( + 'FST_ERR_ASYNC_CONSTRAINT', + 'Unexpected error from async constraint', + 500 + ), FST_ERR_DEFAULT_ROUTE_INVALID_TYPE: createError( 'FST_ERR_DEFAULT_ROUTE_INVALID_TYPE', 'The defaultRoute type should be a function', diff --git a/lib/route.js b/lib/route.js index e89a3c4fa4..5aeef399e5 100644 --- a/lib/route.js +++ b/lib/route.js @@ -101,7 +101,8 @@ function buildRouting (options) { closeRoutes: () => { closing = true }, printRoutes: router.prettyPrint.bind(router), addConstraintStrategy, - hasConstraintStrategy + hasConstraintStrategy, + isAsyncConstraint } function addConstraintStrategy (strategy) { @@ -113,6 +114,10 @@ function buildRouting (options) { return router.hasConstraintStrategy(strategyName) } + function isAsyncConstraint () { + return router.constrainer.asyncStrategiesInUse.size > 0 + } + // Convert shorthand to extended route declaration function prepareRoute ({ method, url, options, handler, isFastify }) { if (typeof url !== 'string') { diff --git a/package.json b/package.json index cfdcababd3..24ac69daf8 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,7 @@ "@fastify/fast-json-stringify-compiler": "^4.1.0", "abstract-logging": "^2.0.1", "avvio": "^8.2.0", - "find-my-way": "^7.2.0", + "find-my-way": "^7.3.0", "light-my-request": "^5.6.1", "pino": "^8.5.0", "process-warning": "^2.0.0", diff --git a/test/constrained-routes.test.js b/test/constrained-routes.test.js index 11fd919a11..867b6d9b37 100644 --- a/test/constrained-routes.test.js +++ b/test/constrained-routes.test.js @@ -660,3 +660,117 @@ test('allows separate constrained and unconstrained HEAD routes', async (t) => { t.ok(true) }) + +test('allow async constraints', async (t) => { + t.plan(5) + + const constraint = { + name: 'secret', + storage: function () { + const secrets = {} + return { + get: (secret) => { return secrets[secret] || null }, + set: (secret, store) => { secrets[secret] = store } + } + }, + deriveConstraint: (req, ctx, done) => { + done(null, req.headers['x-secret']) + }, + validate () { return true } + } + + const fastify = Fastify({ constraints: { secret: constraint } }) + + fastify.route({ + method: 'GET', + url: '/', + constraints: { secret: 'alpha' }, + handler: (req, reply) => { + reply.send({ hello: 'from alpha' }) + } + }) + + fastify.route({ + method: 'GET', + url: '/', + constraints: { secret: 'beta' }, + handler: (req, reply) => { + reply.send({ hello: 'from beta' }) + } + }) + + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'alpha' } }) + t.same(JSON.parse(payload), { hello: 'from alpha' }) + t.equal(statusCode, 200) + } + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'beta' } }) + t.same(JSON.parse(payload), { hello: 'from beta' }) + t.equal(statusCode, 200) + } + { + const { statusCode } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'gamma' } }) + t.equal(statusCode, 404) + } +}) + +test('error in async constraints', async (t) => { + t.plan(8) + + const constraint = { + name: 'secret', + storage: function () { + const secrets = {} + return { + get: (secret) => { return secrets[secret] || null }, + set: (secret, store) => { secrets[secret] = store } + } + }, + deriveConstraint: (req, ctx, done) => { + done(Error('kaboom')) + }, + validate () { return true } + } + + const fastify = Fastify({ constraints: { secret: constraint } }) + + fastify.route({ + method: 'GET', + url: '/', + constraints: { secret: 'alpha' }, + handler: (req, reply) => { + reply.send({ hello: 'from alpha' }) + } + }) + + fastify.route({ + method: 'GET', + url: '/', + constraints: { secret: 'beta' }, + handler: (req, reply) => { + reply.send({ hello: 'from beta' }) + } + }) + + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'alpha' } }) + t.same(JSON.parse(payload), { error: 'Internal Server Error', message: 'Unexpected error from async constraint', statusCode: 500 }) + t.equal(statusCode, 500) + } + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'beta' } }) + t.same(JSON.parse(payload), { error: 'Internal Server Error', message: 'Unexpected error from async constraint', statusCode: 500 }) + t.equal(statusCode, 500) + } + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/', headers: { 'X-Secret': 'gamma' } }) + t.same(JSON.parse(payload), { error: 'Internal Server Error', message: 'Unexpected error from async constraint', statusCode: 500 }) + t.equal(statusCode, 500) + } + { + const { statusCode, payload } = await fastify.inject({ method: 'GET', path: '/' }) + t.same(JSON.parse(payload), { error: 'Internal Server Error', message: 'Unexpected error from async constraint', statusCode: 500 }) + t.equal(statusCode, 500) + } +}) diff --git a/test/router-options.test.js b/test/router-options.test.js index c26c0094dc..0b83aabcea 100644 --- a/test/router-options.test.js +++ b/test/router-options.test.js @@ -2,7 +2,10 @@ const test = require('tap').test const Fastify = require('../') -const { FST_ERR_BAD_URL } = require('../lib/errors') +const { + FST_ERR_BAD_URL, + FST_ERR_ASYNC_CONSTRAINT +} = require('../lib/errors') test('Should honor ignoreTrailingSlash option', async t => { t.plan(4) @@ -137,7 +140,7 @@ test('Should set is404 flag for unmatched paths', t => { }) }) -test('Should honor frameworkErrors option', t => { +test('Should honor frameworkErrors option - FST_ERR_BAD_URL', t => { t.plan(3) const fastify = Fastify({ frameworkErrors: function (err, req, res) { @@ -165,3 +168,54 @@ test('Should honor frameworkErrors option', t => { } ) }) + +test('Should honor frameworkErrors option - FST_ERR_ASYNC_CONSTRAINT', t => { + t.plan(3) + + const constraint = { + name: 'secret', + storage: function () { + const secrets = {} + return { + get: (secret) => { return secrets[secret] || null }, + set: (secret, store) => { secrets[secret] = store } + } + }, + deriveConstraint: (req, ctx, done) => { + done(Error('kaboom')) + }, + validate () { return true } + } + + const fastify = Fastify({ + frameworkErrors: function (err, req, res) { + if (err instanceof FST_ERR_ASYNC_CONSTRAINT) { + t.ok(true) + } else { + t.fail() + } + res.send(`${err.message} - ${err.code}`) + }, + constraints: { secret: constraint } + }) + + fastify.route({ + method: 'GET', + url: '/', + constraints: { secret: 'alpha' }, + handler: (req, reply) => { + reply.send({ hello: 'from alpha' }) + } + }) + + fastify.inject( + { + method: 'GET', + url: '/' + }, + (err, res) => { + t.error(err) + t.equal(res.body, 'Unexpected error from async constraint - FST_ERR_ASYNC_CONSTRAINT') + } + ) +})