date field serialization crashes when null

[simoami]

🐛 Bug Report

I have a timestamp field in my schema named 'updatedAt' and defined as follows:

{
  updatedAt: { type: ['string', null], format: 'date-time'}
}

When a record is created for the first time, the ORM return updatedAt with null, because only createdAt is set on record creation.

When it goes through serialization, I get the following error:

TypeError: Cannot read property 'toISOString' of null
     at $asDatetime (eval at build (/usr/src/app/node_modules/fast-json-stringify/index.js:142:20), <anonymous>:60:26)
     at $arr$mainusermembershipsi (eval at build (/usr/src/app/node_modules/fast-json-stringify/index.js:142:20), <anonymous>:554:17)
     at $arr$mainusermemberships (eval at build (/usr/src/app/node_modules/fast-json-stringify/index.js:142:20), <anonymous>:452:17)
     at $mainuser (eval at build (/usr/src/app/node_modules/fast-json-stringify/index.js:142:20), <anonymous>:403:17)
     at Object.$main (eval at build (/usr/src/app/node_modules/fast-json-stringify/index.js:142:20), <anonymous>:134:17)
     at serialize (/usr/src/app/node_modules/fastify/lib/validation.js:132:41)
     at preserializeHookEnd (/usr/src/app/node_modules/fastify/lib/reply.js:298:15)
    at next (/usr/src/app/node_modules/fastify/lib/hooks.js:101:7)
     at handleResolve (/usr/src/app/node_modules/fastify/lib/hooks.js:112:5)
     at process._tickCallback (internal/process/next_tick.js:68:7)

To Reproduce

Steps to reproduce the behavior:

The generated eval code strictly checks for underfined, so null is considered a valid value! Here's is what the generated code looks like:

if (obj['updatedAt'] !== undefined) {
  if (addComma) {
    json += ','
  }
  addComma = true
  json += '"updatedAt":'
  json += $asDatetime(obj['updatedAt'])
}

And since the function below doesn't check for null, the second condition date.toISOString triggers the error Cannot read property 'toISOString' because date itself is null!

function $asDatetime (date) {
 if (date instanceof Date) {
   return '"' + date.toISOString() + '"'
 } else if (typeof date.toISOString === 'function') {  <----- Error thrown here
   return '"' + date.toISOString() + '"'
 } else {
   return $asString(date)
 }
}

My suggestion is to make 2 changes as proposed below:

function $asDatetime (date) {
  if (date instanceof Date) {
    return '"' + date.toISOString() + '"'
  } else if (date && typeof date.toISOString' === 'function') {
    return '"' + date.toISOString() + '"'
  } else {
    return $asStringNullable(date)
  }
}

Your Environment

• node version: 10
• fastify version: 2.13.0
• os: Mac and alpine (dockerized)

[Eomm]

You should use type: ['string', 'null'] (null as a string).
"plain" null is a valid data type in JSON schema only as a string

[mcollina]

Would you mind to send a PR to handle this? Otherwise I'll get to it as soon as I can. That if statement should be guarded against this.

[mum-never-proud]

i too faced the same issue, but the code is break here

${index === 0 ? 'if' : 'else if'}(typeof obj${accessor} === "${type}" || obj${accessor} instanceof Date || typeof obj${accessor}.toISOString === "function" || obj${accessor} instanceof RegExp)
precisely typeof obj${accessor}.toISOString === "function"

is this the same issue?

[Eomm]

Maybe I'm missing something, but I don't think it is a bug: null is wrong, 'null' (string) is right.

This is working (and conform to the JSON Schema's standard):

function build (opts) {
  return fastJson({
    type: 'object',
    properties: {
      updatedAt: {
        type: ['string', 'null'],
        format: 'date-time'
      }
    }
  }, opts)
}

const seri = build()
console.log(seri({ updatedAt: null })) // {"updatedAt":null}

But I cannot reproduce the Cannot read property 'toISOString' of null since with null I get

Error: schema is invalid: data.properties['updatedAt'].type should be equal to one of the allowed values

Could you add more info?

[mum-never-proud]

function build (opts) {
  return fastJson({
    type: 'object',
    properties: {
      updatedAt: {
        type: ['string'],
        format: 'date-time'
      }
    }
  }, opts)
}

const seri = build()
console.log(seri({ updatedAt: null })) // TypeError: Cannot read property 'toISOString' of null

maybe we should handle this instead of breaking

your thoughts @mcollina ?

[Eomm]

Thank you for the clarification!
It is a bug 😀

This is even worse:

function build (opts) {
  return fastJson({
    type: 'object',
    properties: {
      updatedAt: {
        type: ['string']
      }
    }
  }, opts)
}

(same error without format)

[mcollina]

It's a bug and it needs to be fixed.

[simoami]

@Eomm I typed ['string', null] manually but actually meant ['string', 'null']. null as a value is the actual content of updatedAt

[Eomm]

@Eomm I typed ['string', null] manually but actually meant ['string', 'null']. null as a value is the actual content of updatedAt

Could you show some code to replicate?

I tried here #218 (comment)

[simoami]

@Eomm Furthermore I'm noticing a different behavior when the array brackets are used:

{
    type: 'object',
    properties: {
      date: {
        type: ['string']
      }
    }
  }

vs:

{
    type: 'object',
    properties: {
      date: {
        type: 'string'
      }
    }
  }

See my PR comment for details: #219 (comment)

[zuck]

@mcollina @Eomm we also encountered this bug, unfortunately it breaks our tests on a production CI / CD pipeline. Do you plan to release a new version after merging #219? Thank you!

[simoami]

Thanks for tackling this quickly. Stay safe everyone!