Response schema compiler generated validation code does not check for null values on optional object fields
rhighs opened this issue · 9 comments
Prerequisites
- I have written a descriptive issue title
- I have searched existing issues to ensure the bug has not already been reported
Fastify version
4.17.0
Plugin version
No response
Node.js version
18.14.2
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
13.2
Description
Defining a response schema with some non-required fields whose type is object, will carry @fastify/fast-json-stringify-compiler to generate static validation code that does not check for the possibility of such fields being null. In other words, it will only check for those fields being strictly equal to undefined. While it might seem reasonable to only check for undefined, the error caused by this scenario is very inconvenient and hard to track down as the generated code does not seem to lie on any specific file, instead it is allocated on some runtime memory.
I know this is kind of confusing so i'll provide a screenshot showing the portion of generated code I'm referring to, along with the schema that was associated with it.
and the schema that led to it:
const schema = {
response: {
200: {
type: 'array',
items: {
type: 'object',
properties: {
tracks: {
type: 'array',
items: {
type: 'object',
properties: {
...
genres: {
type: 'array',
items: {
type: 'object',
properties: {
id: { type: 'string' },
name: { type: 'string' },
parent: {
type: 'object',
properties: {
id: { type: 'string' },
name: { type: 'string' }
}
}
}
}
...
}
}
},
additionalProperties: true
}
}
},
additionalProperties: true
}
}
}
If, for some reason, the "parent" field inside "genres" is presen with a value of null
the validation code will enter the if branch calling whatever other generated function to validate its fields. Since accessing keys on null is illegal, the code will simply crash leading to this issue.
Steps to Reproduce
Just run this
const Fastify = require('fastify')
const fastify = Fastify()
const HOST = 'localhost'
const PORT = 3000
fastify.get('/', {
handler: (req, res) => res.code(200).send({ someObject: null }),
schema: {
response: {
200: {
type: 'object',
properties: {
someObject: {
type: 'object',
properties: { id: { type: 'string' }, name: { type: 'string' } }
}
}
}
}
}
})
fastify.listen({ host: HOST, port: PORT }).then(_ => console.log(`listening at ${HOST}:${PORT}`))
Then
$ curl -X GET localhost:3000
Expected Behavior
With the steps provided above you should get this response:
{"statusCode":500,"error":"Internal Server Error","message":"Cannot read properties of null (reading 'id')"}
Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.
I suspect this is a bug in fast-json-stringify.
I'd say that we should transfer it, it seems scoped to fast-json-stringify
using a normal JSON.stringify
solves the problem.
Most likely fast-json-stringify
might require to handle null
as is.
Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.
Sure, I can do it :)
@rhighs If you want to cover the case when object can be null I think you should use nullable
keyword or type: ['object', 'null']
.
@rhighs If you want to cover the case when object can be null I think you should use
nullable
keyword ortype: ['object', 'null']
.
Ok, sounds good. I'll have a look at the code first!
I mean use the nullable
keyword in your schema.
I mean use the
nullable
keyword in your schema.
I saw the docs already at the time being, what I'm trying to say with this issue is that it's really hard to see what's happening when you stumble upon this bug. Having some sort of message would be great instead of exploding on the stack like above.
Unfortunately there are cases where you can't say for sure if a type is nullable
, so you might want some sort of error when that happens.