Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename to JSON::Safe #142

Open
rurban opened this issue Jul 5, 2019 · 21 comments
Open

Rename to JSON::Safe #142

rurban opened this issue Jul 5, 2019 · 21 comments
Assignees

Comments

@rurban
Copy link
Owner

rurban commented Jul 5, 2019

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 rurban self-assigned this Jul 5, 2019
@Grinnz
Copy link

Grinnz commented Jul 5, 2019

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.

@karenetheridge
Copy link
Contributor

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
Copy link
Contributor

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

@exodist
Copy link

exodist commented Jul 5, 2019

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

@rurban
Copy link
Owner Author

rurban commented Jul 6, 2019

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.

@bulk88
Copy link
Contributor

bulk88 commented Jul 7, 2019

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.

Perl-Toolchain-Gang/CPAN-Meta#107
p5sagit/JSON-MaybeXS#1
makamaka/JSON#32

@bulk88
Copy link
Contributor

bulk88 commented Jul 7, 2019

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

@dotandimet
Copy link

dotandimet commented Jul 14, 2019

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.

@wollmers
Copy link

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

Maybe JSON::Std would be worth to discuss.

@Grinnz
Copy link

Grinnz commented Nov 24, 2019

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

@pali
Copy link
Contributor

pali commented Dec 16, 2019

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

@rurban
Copy link
Owner Author

rurban commented Dec 16, 2019

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.

@pali
Copy link
Contributor

pali commented Dec 16, 2019

->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])

@rurban
Copy link
Owner Author

rurban commented Dec 16, 2019

Ah good. Then it's ok I think.

@Grinnz
Copy link

Grinnz commented Dec 16, 2019

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.

@pali
Copy link
Contributor

pali commented Jan 23, 2020

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.

@karenetheridge
Copy link
Contributor

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?

@mj41-gdc
Copy link

What about JSON::SafeXS ?

@pali
Copy link
Contributor

pali commented Jan 24, 2020

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

@rurban
Copy link
Owner Author

rurban commented Jan 24, 2020

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

@tobyink
Copy link

tobyink commented Feb 6, 2020

JSON::Excess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants