isJSON rejects`"0"`

[avandekleut]

Describe the bug

The function isJSON rejects valid json.

Examples

import validator from 'validator'

const validJson = "0"
if (!validator.isJSON(validJson)) {
  throw Error(`Invalid json: ${validJson}`)
}

You can also use fast-check to generate examples for testing:

import * as fc from 'fast-check'
import validator from 'validator'

test('isJSON', () => {
  fc.assert(
    fc.property(fc.json(), (json) => {
      expect(validator.isJSON(json)).toBeTruthy()
    }),
  )
})

FAIL  src/fast-check/fast-check.spec.ts
  ● isJSON

    Property failed after 4 tests
    { seed: 1003302133, path: "3:0", endOnFailure: true }
    Counterexample: ["0"]
    Shrunk 1 time(s)
    Got error: Error: expect(received).toBeTruthy()

    Received: false

    Stack trace: Error: expect(received).toBeTruthy()

    Received: false

       5 |   fc.assert(
       6 |     fc.property(fc.json(), (json) => {
    >  7 |       expect(validator.isJSON(json)).toBeTruthy()
         |                                      ^
       8 |     }),
       9 |   )
      10 | })

Additional context
Just wondering why we don't do the following?

export default function isJSON(str) {
  assertString(str);

  try {
    var obj = JSON.parse(str);
    return true
  } catch (e) {
    /* ignore */
  }

  return false;
}

[pano9000]

Hello avandekleut,

I can confirm that isJSON returns false for the test case you provided. I didn't know that that was also considered valid "JSON" to be honest :-)

From what I can see, the validator already uses JSON.parse internally, but then also checks, if the output of the JSON.parse operation is of type object – and this is where the "0" examples then fails, because that is of type number (regardless of whether you originally cast it as string or a number).
Additionally it also fails, the validator tries to coerce the output to a boolean via the double exclamation marks, but because 0 is falsy, it will of course also return false.
See here:

validator.js/src/lib/isJSON.js

Line 18 in 43803c0

return primitives.includes(obj) || (!!obj && typeof obj === 'object');

Only question here is, if that was maybe done intentionally, to only allow strings in "expected JSON" formatting? (i.e. anything in curly brackets?)

I guess we should also double-check with the RFC on this as well:
https://www.rfc-editor.org/rfc/rfc8259

[avandekleut]

The RFC states

A JSON value MUST be an object, array, number, or string, or one of the following three literal names:
false
null
true

By this logic, we should be able to delegate validation entirely to JSON.parse. What do you think?

[WikiRik]

That would cause more to be accepted than people expect now, so maybe that's for the next major release? And in the meantime we can add a new option to allow non-object values? Like the allow_primitives option

[avandekleut]

To adhere to semantic versioning, I assume the following behaviour would have to be preserved:

• Arrays are validated

• Objects are validated

• if allow_primitives is set to true:

  • null, true, and false are validated

The following behaviour is not implemented, but could be implemented without breaking the default behaviour:

• if allow_primitives is set to true:
  • strings and numbers are validated.

Since the default is allow_primitives: false, we can add the new behaviour when allow_primitives is set to true.

Here is my implementation of that change, with comments:

import assertString from './util/assertString';
import merge from './util/merge';

const default_json_options = {
  allow_primitives: false,
};

export default function isJSON(str, options) {
  assertString(str);

  /* this doesn't need to be in the try block */
  options = merge(options, default_json_options);

  try {
    /* JSON.parse should succeed if str is a valid stringified JSON object */
    const obj = JSON.parse(str);
    if (allow_primitives) {
      /* by default, primitives are allowed in json */
      return true;
    }
    /* original behaviour is to only validate arrays and objects by default */
    /* replaced !!obj with obj !== null which is more readable */
    return (obj !== null && typeof obj === 'object');
  } catch (e) { /* ignore */ }
  return false;
}