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

Mapping int on 32-bit Perl

Open mla opened this issue 6 years ago • 8 comments

The docs say:

If the number consists of digits only, Cpanel::JSON::XS will try to
represent it as an integer value. If that fails, it will try to
represent it as a numeric (floating point) value if that is
possible without loss of precision.

When it says "it will try," does that mean it will only be treated as an int if the number fits within a perl int, even if allow_bignum() is enabled?

Because I have a case with a VM running 32-bit perl:

$ perl -V:ivsize 
ivsize='4';

And this script converts the assignee_id to a float:

#!/usr/bin/env perl

use strict;
use warnings;
use Math::BigInt;
use Cpanel::JSON::XS ();

my $assignee_id = 5439409363;
#my $assignee_id = Math::BigInt->new('5439409363');

my $json = Cpanel::JSON::XS->new->allow_blessed->allow_bignum->convert_blessed->encode({
  assignee_id => $assignee_id,
});
print "$json\n";

That prints:

{"assignee_id":5439409363.0}

But on 64-bit, it's treated as an int. Is the only way to get that treated as an int consistently, to create it as a BigInt instance first?

Thanks.

mla avatar Jun 19 '18 23:06 mla

And just to note, JSON::XS keeps that as an int. I realize it has other issues.

JSON::XS 3.01 Cpanel::JSON::XS 4.02

perl 5.18.2

mla avatar Jun 19 '18 23:06 mla

At least our current behavior matches the documentation. But JSON::XS is right. Encoding should assume int64 not just int32. Javascript doesn't care anyways as they represent ints as floats, and only internally use 31bit ints I think. So the docs need to be fixed to encode int64 to int without the decimal point separator, and the implementation.

But on 64-bit, it's treated as an int. Is the only way to get that treated as an int consistently, to create it as a BigInt instance first?

Looks like so, yes. This at least guarantees no precision loss, even if MAX_INT64. For the normal case we should really assume int64 targets, and only when decoding back to 32bit ints convert to double.

rurban avatar Jun 20 '18 08:06 rurban

@mla This is not problem in Cpanel::JSON::XS, but rather in your provided example.

This code

my $assignee_id = 5439409363;

on 32bit perl assigns into variable assignee_id floating point number and not integer!

You can verify it via following test:

$ perl -MDevel::Peek -e 'my $assignee_id = 5439409363; Dump $assignee_id'

On 32bit perl with 32bit integers it prints:

SV = NV(0x574c385c) at 0x574bb810                                                                                                                                                                                  
  REFCNT = 1                                                                                                                                                                                                       
  FLAGS = (NOK,pNOK)                                                                                                                                                                                               
  NV = 5439409363

(NV means floating point)

On 64bit perl with 64bit integers it prints:

SV = IV(0x560c223a9ca0) at 0x560c223a9cb0                                                                                                                                                                          
  REFCNT = 1                                                                                                                                                                                                       
  FLAGS = (IOK,pIOK)                                                                                                                                                                                               
  IV = 5439409363

(IV means integer)

Therefore Cpanel::JSON::XS takes floating point number from that variable and encodes it into json. So Cpanel::JSON::XS does the right thing.

But on 64-bit, it's treated as an int.

Yes, this is how is Perl working. Your number cannot be represented by native processor integer then it is represented by native processor floating point number.

Is the only way to get that treated as an int consistently, to create it as a BigInt instance first?

No. BigInt is exactly for this purpose.

@rurban I think you can close this issue as it is not issue at all.

pali avatar May 17 '19 12:05 pali

@pali I'm not so sure. integers in JSON should be independent on use64bitint or not. JSON is architecture independent. That's why we also promote to bigint. In the 32int case when perl promotes to float we might need to strip off the dot for proper roundtrips. or even promote to bigint when we loose precision.

rurban avatar May 17 '19 12:05 rurban

@rurban Problem is that conversion to float is done by perl itself prior passing value to Cpanel::JSON::XS. So it is something which cannot be changed or fixed in Cpanel::JSON::XS. It is how Perl works.

See another example:

$ perl -e 'my $var = 12345678901234567890; print $var;'
1.23456789012346e+19

Precision is lost by perl, even without loading Cpanel::JSON::XS.

Main problem is that above Perl code my $assignee_id = 5439409363; needs 64bit perl. It is problem in that provided code.

pali avatar May 17 '19 12:05 pali

Let say it in other words: For 32bit perl is token 12345678901234567890 floating point number equivalent to 1.23456789012346e+19. In same way token 5439409363 is also floating point. Also token 4.5 is floating point.

And if user writes floating point token, json encoder needs to encode it as floating point number. Or not?

Btw, for enforcing JSON types, e.g. having integer in json output, there is Cpanel::JSON::XS::Type. You can write encode_json([$var], [JSON_TYPE_INT]) and you would always get json integer, not floating point.

pali avatar May 17 '19 12:05 pali

Thank you, @pali. That clarifies a lot for me.

The example I gave was contrived, of course. IIRC, the issue was found with an API integration. It had been working for a while with no issues and then we switched from JSON::XS to Cpanel::JSON::XS and we started seeing rejected messages due to IDs being converted to floats.

I think we did resolve the issue by converting to BigInt, but I still wonder what exactly JSON::XS is doing that it didn't exhibit the same behavior.

A quick scan of the JSON::XS code shows this:

 else if (SvNOKp (sv))
    {
      // trust that perl will do the right thing w.r.t. JSON syntax.
      need (enc, NV_DIG + 32);
      Gconvert (SvNVX (sv), NV_DIG, 0, enc->cur);
      enc->cur += strlen (enc->cur);
    }

Does that seem like the relevant bit? You guys understand that a lot better than I do.

mla avatar May 18 '19 17:05 mla

I do not know what is JSON::XS doing differently, but as stated in previous comments, I think that Cpanel::JSON::XS gives correct result for your provided example. We know that JSON::XS has (or had) bugs, so this may be another one.

pali avatar Jun 03 '19 08:06 pali