fastify/fast-json-stringify

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.

image
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 or type: ['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.

Closing as completed by #670