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

JSON Integer/Double conversion problem. #22

Open
jezwhite opened this issue Sep 30, 2020 · 4 comments
Open

JSON Integer/Double conversion problem. #22

jezwhite opened this issue Sep 30, 2020 · 4 comments

Comments

@jezwhite
Copy link

Hello all,

I'm having an odd problem, and I'm not sure what the solution should be...

I'm using JavaScript-Duktape-XS to generate some dynamic config files which is then JSON posted to a 3rd party (I am usingMojo::UserAgent to post) . During the conversion of objects from Duktape->Perl->JSON integers are converted to doubles which causes the the json to look like:

{
   "an_id":1.0,
   "another_id":252.0
}

rather than:

{
   "an_id":1,
   "another_id":252
}

the 3rd party is throwing an exception as it's expecting an integer for these fields. I can't get the 3rd party to change their parsing rules. The actual objects passed to the 3rd party are much more complex/dynamic than this example.

One solution that solves this for me is to explicitly check of the value returned via duk_get_number to see if it looks like an integer (in pl_duk.c, pl_duk_to_perl_impl) , ie, something like:

        case DUK_TYPE_NUMBER: {
            duk_double_t val = duk_get_number(ctx, pos);
            if (IsAnInteger(val)) {
              ret = newSViv(val);
            }
            else {
              ret = newSVnv(val);  /* JS numbers are always doubles */
            }
          break;

But I am not sure what the broader implicates would be?

Any thoughts/comments?

Regards,

@gonzus
Copy link
Owner

gonzus commented Oct 5, 2020

Hey, sorry for the long delay. I am not doing much with Duktape and Perl these days.

What you are seeing is an expected result of the fact that all numbers in JS are doubles, so there is not a good way to treat integers differently.

Your idea looks good to me; I would just check if Duktape has added any APIs that facilitate having a special case for integer numbers. I would gladly take a PR, especially if it includes a test case that fails before and passes after your proposed changes.

@jezwhite
Copy link
Author

Apologies for the delay. I'll put together a PR with test cases etc.

I assume you would be OK with other PR's as well? I may have found a couple of bugs (but I'll need to dig more), along with updating the Duktape library to the latest version?

@FGasper
Copy link
Contributor

FGasper commented Sep 1, 2021

@jezwhite Did you make any progress here?

FWIW I would think it better either to make this an opt-in behaviour or just to += 0 in Perl. Otherwise it incurs a performance penalty for everyone that only certain applications may need.

Notably, not all JSON serializers behave this way; JSON::XS doesn’t, for example, while Cpanel::JSON::XS (which Mojo::JSON will prefer if it’s available) does.

@jezwhite
Copy link
Author

jezwhite commented Sep 1, 2021

@FGasper So...no. I ended up 'fixing' it via the javascript running inside the Duktape container. The json object flowing between perl->javascipt->perl was too complex to fix on the perl side as it was acting as a message handler/broker so it knew nothing about the structure/data of the objects.

I wasn't convinced that the solution was correct, but I do like your opt-in behavior suggestion though. I am keeping an eye out for other similar situations and if it crops up again, I will look at it.

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

3 participants