Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support async constraint #4323

Merged
merged 1 commit into from Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/Reference/Errors.md
Expand Up @@ -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
<a id="FST_ERR_ASYNC_CONSTRAINT"></a>

The router received error when using asynchronous constraints.

#### FST_ERR_DEFAULT_ROUTE_INVALID_TYPE
<a id="FST_ERR_DEFAULT_ROUTE_INVALID_TYPE"></a>

Expand Down
54 changes: 54 additions & 0 deletions docs/Reference/Routes.md
Expand Up @@ -732,6 +732,60 @@ fastify.route({
})
```

#### Asynchronous Custom Constraints
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fdawgs Do you have time to helps me check the document changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @climba03003, missed this. Will take a look next week.


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
Expand Down
3 changes: 3 additions & 0 deletions docs/Reference/Server.md
Expand Up @@ -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)
}
Expand Down
67 changes: 47 additions & 20 deletions fastify.js
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you evaluate res.destroy instead?

The only difference is whether or not emit an error from the request object. Can't recall any other differences

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite understand what you means.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to refresh this

const { fastify } = require('fastify')
const app = fastify()

app.get('/destroy', async (request, reply) => {
  reply.hijack()
  reply.raw.destroy('hello world')
})

app.get('/end', async (request, reply) => {
  reply.hijack()
  reply.raw.end('hello world')
})

app.inject('/end', check)
  .then(() => app.inject('/destroy', check))

function check (err, res) {
  console.log(err)
  console.log(res?.statusCode)
  console.log(res?.payload)
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.destory cannot and will not send any payload to the client. Instead, it close the socket immediately.
.end return a proper error message to the client and they can deal with it other than just wait for browser socket timeout.

}
}
}

function setNotFoundHandler (opts, handler) {
throwIfAlreadyStarted('Cannot call "setNotFoundHandler" when fastify instance is already started!')

Expand Down Expand Up @@ -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}"`))
climba03003 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
router.routing(req, res, buildAsyncConstraintCallback(isAsync, req, res))
}
}
}

fastify.errorCodes = errorCodes
Expand All @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions lib/errors.js
Expand Up @@ -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',
Expand Down
7 changes: 6 additions & 1 deletion lib/route.js
Expand Up @@ -101,7 +101,8 @@ function buildRouting (options) {
closeRoutes: () => { closing = true },
printRoutes: router.prettyPrint.bind(router),
addConstraintStrategy,
hasConstraintStrategy
hasConstraintStrategy,
isAsyncConstraint
}

function addConstraintStrategy (strategy) {
Expand All @@ -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') {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -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",
Expand Down
114 changes: 114 additions & 0 deletions test/constrained-routes.test.js
Expand Up @@ -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)
}
})