`toString` existence check on objects does not work using Object.hasOwnProperty if the object is a class instance

[pvanagtmaal]

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.2.1

Plugin version

5.1.0

Node.js version

18.4.0

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

12.4

Description

The generated code for creating strings from objects checks for the existence of a toString function, but through Object.hasOwnProperty.call(object, 'toString') (https://github.com/fastify/fast-json-stringify/blob/master/index.js#L904). This does not work for class instances.

An example for where this goes south: if you're using Luxon DateTimes in your code for dates instead of the regular JavaScript Date object and need it as a string in your API response, the conversion fails (but the error message references a stringified Luxon DateTime because there is a toString present on the class, just not as a "own property".

Steps to Reproduce

dependencies:

npm install fastify@4 luxon@3

index.js:

const l = require('luxon')
const f = require('fastify')

const app = new f.fastify()

const opts = {
  schema: {
    response: {
      200: {
        type: 'object',
        properties: {
          date: { type: 'string', format: 'date-time' }
        }
      }
    }
  }
}

app.get('/', opts, () => {
  return { date: l.DateTime.now() }
})

app.listen({ port: 8000 })

call route

curl localhost:8000

error:

{"statusCode":500,"error":"Internal Server Error","message":"The value \"2022-07-13T16:03:06.938+02:00\" does not match schema definition."}

Note that the value reported is a valid date-time string

Expected Behavior

The string conversion should succeed by calling toString() on the instance.

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[pvanagtmaal]

PR added

[pvanagtmaal]

The changes made in the PR were correct but there was some discussion about supporting these kind of objects at all. I'm leaving this to someone else, feel free to close if you see this as a wontfix.

[jsumners]

The bug exists. It just needs to be solved in a generic way. Taking your ball and going home because you don't like the feedback is a poor way to interact.

Anyway, the code below shows the actual bug and the cause. It also shows that the code in this repo is incorrectly referencing Object.hasOwnProperty instead of Object.prototype.hasOwnProperty. Note, I do not think this is the right solution. But I don't know of any other way to determine "this object was created with class":

'use strict'

class Foo {
   toString () {
      return 'foo'
   }
}

let foo = new Foo()

if (Object.prototype.hasOwnProperty.call(foo, 'toString')) {
   console.log(foo.toString())
} else {
   console.log('what?')
}

if (Object.prototype.hasOwnProperty.call(foo.__proto__, 'toString')) {
   console.log('i hate the class syntax')
}

foo = {
   toString() {
      return 'foo2'
   }
}

if (Object.prototype.hasOwnProperty.call(foo, 'toString')) {
   console.log(foo.toString())
} else {
   console.log('what?')
}

if (Object.prototype.hasOwnProperty.call(foo.__proto__, 'toString')) {
   console.log('i hate the class syntax')
}

The above should output:

what?
i hate the class syntax
foo2
i hate the class syntax

[ivan-tymoshenko]

@Uzlopak Does #510 fix this issue?

[Uzlopak]

If we have derived classes, than we need to do 'toString' in object. So i think this does not fix it.

[ivan-tymoshenko]

@Uzlopak @jsumners Will it work properly if I change
this

Object.prototype.hasOwnProperty.call(obj, "toString")

to this?

obj.toString !== Object.prototype.toString

[jsumners]

I'm not sure that is a robust enough solution. You should write some test cases to prove it.