Cpanel-JSON-XS icon indicating copy to clipboard operation
Cpanel-JSON-XS copied to clipboard

Rename to JSON::Safe

Open rurban opened this issue 6 years ago • 22 comments

Despite the name the company cPanel never used this module, because of their famous middle management. Now also plaguing p5p. They also don't maintain it. We shouldn't continue with this lie.

The best name JSON::XS is already taken, so maybe JSON::Safe, analog to the new YAML::Safe is better and shorter. JSON is already safe, yes. But this is the best idea I came up with.

It also has the same API as YAML::Safe, and totally different to YAML::XS which sets its options via globals. We get and set our options via methods, and keep state in an object. We could add a whitelist of allowed classed to be blessed to or from. (SafeClass, SafeLoad, ...)

rurban avatar Jul 05 '19 09:07 rurban

I don't think it's the best name as far as representing the module's purpose, but I do agree that Cpanel::JSON::XS is worse and there aren't a lot of good choices.

Grinnz avatar Jul 05 '19 15:07 Grinnz

Sounds good to me. The cpanel name has always caused confusion.

Of course, please continue to include Cpanel::JSON::XS as a wrapper module, so all the code that uses it continues to work :)

karenetheridge avatar Jul 05 '19 21:07 karenetheridge

adapted from a suggestion on irc -- how about JSON::RFC8259?

karenetheridge avatar Jul 05 '19 21:07 karenetheridge

Also not great, but indicates XS at least: JSON::ReXS Re as in retry, another take on JSON in XS

exodist avatar Jul 05 '19 21:07 exodist

JSON::RFC8259 is insecure by default, and literally a joke. All the missing problems were left undefined. If so then the very first RFC4627. JSON::ReXS sounds good, but JSON::Safe still sounds better. It's the only safe-by-default JSON module, and it mimics the YAML::Safe API.

rurban avatar Jul 06 '19 10:07 rurban

EUMM used to depend on Parse::CPAN::Meta which used to depend in (unmainted?) JSON.PM but I think the PCM dep was eventually replaced by JSON::MaybeXS which is C::J::XS compatible so currently I do have the ENV var set and 99% of my Perl builds run with C::J::X since JSON::XS dropped C89 years ago, C::J::X did not. I dont have time to research all of these backend wrappers and loaders, but there is QUITE a bit of regexs and absolute name of C::J::X coded on CPAN, breathing on them by changing CJX's name in 2019 requires alot of testing or new releases of other CPAN modules that they dont fall back to JSON::PP. A poisoned JSON::PP.pm file and a poisoned JSON::XS that both die() at "use" time is only way to test a CJX rename works. I'll have to fix my env vars too obviously but that is dark code and ill deal with it when it comes.

https://github.com/Perl-Toolchain-Gang/CPAN-Meta/pull/107 https://github.com/p5sagit/JSON-MaybeXS/pull/1 https://github.com/makamaka/JSON/issues/32

bulk88 avatar Jul 07 '19 23:07 bulk88

https://github.com/makamaka/JSON/blob/master/lib/JSON.pm#L72 example of sketchy code

bulk88 avatar Jul 07 '19 23:07 bulk88

JSON::Safe is fine. +1 for the change. Can your new dist still export the old module name so as not to break all the wrappers bulk88 lists above? At least until they change.

dotandimet avatar Jul 14 '19 14:07 dotandimet

IMHO JSON::Safe is the best proposal so far.

Maybe JSON::Std would be worth to discuss.

wollmers avatar Nov 24 '19 16:11 wollmers

It might be worth considering a name that still indicates XS, as this is an important factor in choosing the module.

Grinnz avatar Nov 24 '19 17:11 Grinnz

I would suggest to enable ->unblessed_bool, ->type_all_string and ->require_types options by default in new module to have JSON encoding deterministic across different Perl versions, see: http://e-choroba.eu/18-yapc

pali avatar Dec 16 '19 09:12 pali

I like the talk. thinking that Safe also implies type-safety, it would make sense. But require_types would be too much. It would break most existing code. I'll test it. still have to finish setting up my new computers, with a few hundred perl's and its modules. so it will need at least another month or so.

rurban avatar Dec 16 '19 09:12 rurban

->type_all_string already implies types, so ->require_types is OK and existing code would not be broken. It is also written in documentation: https://metacpan.org/pod/Cpanel::JSON::XS#$json-=-$json-%3Erequire_types-([$enable])

pali avatar Dec 16 '19 09:12 pali

Ah good. Then it's ok I think.

rurban avatar Dec 16 '19 09:12 rurban

I don't think any of these should be enabled since it will just be another behavior change everyone needs to keep track of. unblessed_bool and type_all_string both break roundtripping.

Grinnz avatar Dec 16 '19 14:12 Grinnz

Currently all other JSON encoders randomly changes their behavior when updating Perl itself. This is really problematic and for more people very surprised. I already heard advice: Do not upgrade Perl because it will change encoding of your JSONs. I'm looking at this problem from different side: Ensure that encoding and decoding would be same independenly of used version. And options above can achieve it.

pali avatar Jan 23 '20 19:01 pali

Currently all other JSON encoders randomly changes their behavior when updating Perl itself. This is really problematic and for more people very surprised.

Is this referring to JSON::PP changing the default of allow_ref to true in 4.0, or something else?

karenetheridge avatar Jan 23 '20 21:01 karenetheridge

What about JSON::SafeXS ?

mj41-gdc avatar Jan 24 '20 08:01 mj41-gdc

@karenetheridge I'm referring to the fact which @choroba described that JSON types are being changed when updating Perl.

pali avatar Jan 24 '20 09:01 pali

@mj41-gdc nice, but I already published the equivalent YAML::Safe

rurban avatar Jan 24 '20 18:01 rurban

JSON::Excess

tobyink avatar Feb 06 '20 21:02 tobyink