jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 58 forks source link

Allow booleans to be scalar refs #215

Closed waterkip closed 4 years ago

waterkip commented 4 years ago

The JSON validation breaks when one uses \1 or \0 when using the serializion in conjuction with JSON::XS, Mojo::JSON and others.

Signed-off-by: Wesley Schwengle wesley@opndev.io

jhthorsen commented 4 years ago

I don't see any reason to accept this, since scalar refs is not a thing in JSON. Please provide a better argument for why/when this is needed.

waterkip commented 4 years ago
$ cat json-bool.pl; perl json-bool.pl
#!/usr/bin/perl
use warnings;
use strict;
require JSON::XS;
require JSON::PP;
require Mojo::JSON;
require Cpanel::JSON::XS;

print JSON::XS::encode_json({ foo => \1 }), $/;
print JSON::PP::encode_json({ foo => \1 }), $/;
print Mojo::JSON::encode_json({ foo => \1 }), $/;
print Cpanel::JSON::XS::encode_json({ foo => \1 }), $/;
print JSON::XS::encode_json({ foo => \0 }), $/;
print JSON::PP::encode_json({ foo => \0 }), $/;
print Mojo::JSON::encode_json({ foo => \0 }), $/;
print Cpanel::JSON::XS::encode_json({ foo => \0 }), $/;

{"foo":true}
{"foo":true}
{"foo":true}
{"foo":true}
{"foo":false}
{"foo":false}
{"foo":false}
{"foo":false}

They all serialize \1 and \0 to true and false. Which is why I think the validation must respect this syntax as well.

jhthorsen commented 4 years ago

You still haven't given me any argument for when this is beneficial.

waterkip commented 4 years ago

If I use \1 or \0 in code somewhere and I pass it to the Mojolious Plugin for OpenAPI and I set it to validate the JSON before sending it out it croaks. I dug around and saw that the JSON validator which is used by that plugin doesn't allow this syntax. So when I try to validate the output the output is considered invalid while when I disable the validation it works fine and returns a boolean value as shown in the json-bool.pl example with the various serializers.

Which leads me to 1) Not use the validation 2) Patch the validator that is accepts such values. I opted for 2 because I want to know when I do something wrong. Another workaround would be to serialize to json, deserialize it and then pass it to the validator after which another round of serialization happens. Which I find a bit counter intuitive.

jhthorsen commented 4 years ago

Ooops! Closed the wrong issue :(

jhthorsen commented 4 years ago

You should really just use false and true in that case. There's no way to send a scalar ref using JSON, so I'm pretty sure this is the wrong fix.

Keeping it open a bit longer though in case other people have relevant information.

jhthorsen commented 4 years ago

And when I say false and true, I mean functions such as YAML::XS::false or Mojo::JSON::true (and friends)

waterkip commented 4 years ago

From the documentation of Mojo::JSON:

In addition scalar references will be used to generate booleans, based on if their values are true or false.

From the documentation of Types::Serializer (used by JSON::XS):

The recommended way to detect whether a scalar is one of these objects is to check whether the stash is the Types::Serialiser::Boolean or Types::Serialiser::Error stash, and then follow the scalar reference to see if it's 1 (true), 0 (false) or undef (error).

I think Mojo::JSON is used by Mojo and thus by the OpenAPI plugin. So you are saying you don't want to support that native serializer? Seems odd but ok.

karenetheridge commented 4 years ago

Mojo::JSON doesn't use JSON::XS.

waterkip commented 4 years ago

I never said that. I just quoted their documentation and the docs of JSON::XS.

jhthorsen commented 4 years ago

Going to close this issue, since I don't agree with the arguments given.