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

Add option for strict number parsing #570

Closed
dgollahon opened this issue Nov 13, 2019 · 5 comments
Closed

Add option for strict number parsing #570

dgollahon opened this issue Nov 13, 2019 · 5 comments

Comments

@dgollahon
Copy link

Hi,

I operate several JSON APIs that use oj in various capacities. In general, we want to force very strict parsing (and use oj's strict mode) so that we don't accidentally support parsing features that we might later not if we switch to a different library. Recently oj has relaxed strictness for number parsing in a couple of ways. After those changes, they were restricted in the json compatibility mode here and here.

I would like to be able to enforce that behavior (failing on number formats not part of the JSON standard) but I don't see another way to enable this behavior in the options options and would prefer the rest of the strict configuration. According to the goals of the strict mode documentation it seems like this should be the default behavior for strict as well, (but if you agree I believe that would require releasing a new major version since it is currently allowed). In any case, I would like to be able to opt-in to the stricter parsing mode, perhaps through an option.

tl;dr: I want to raise an error when parsing Oj.load('+1.') or similar while using strict mode, as was the case < 3.7.1.

Thanks for a great gem!

@ohler55
Copy link
Owner

ohler55 commented Nov 13, 2019

I didn't consider strict mode when making the change to number parsing. I would classify that as a bug. I'd prefer to handle it that way. Are there any other bugs introduced other than the leading + and the trailing .?

@dgollahon
Copy link
Author

@ohler55 I'm not personally aware of any other bugs but I haven't done any differential testing. I caught the change from reading the changelog and then skimmed the commit history briefly.

If you'd like to handle it as a bug, that's your call--it won't impact me negatively, but it is a semantic change that could potentially affect others. I leave that kind of SemVer consideration to your judgment. I suppose treating it as a bugfix would be consistent with previous releases such as 3.7.4 where it changed for the compat mode.

Again, I appreciate your hard work on this gem.

@ohler55
Copy link
Owner

ohler55 commented Nov 21, 2019

A bit slow but the bug/570 branch has the fix.

@ohler55
Copy link
Owner

ohler55 commented Nov 29, 2019

Release 3.10.0 includes a fix for this issue.

@dgollahon
Copy link
Author

Great, thanks!

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

No branches or pull requests

2 participants