Regression in toJSON

[braunsonm]

In the newer builds after our update to MTASA server 1.5.6, toJSON no longer works as expected.

Large integers passed to toJSON will now return a float, which differs from the expected and previous behavior.

toJSON(1234567890000) -- four zeroes

Outputs incorrectly by converting my integer to a float [ 1234567890000.0 ]

I'm unfamiliar with the codebase so bear with me.
The check here: https://github.com/multitheftauto/mtasa-blue/blame/40118f7875516304ff585c4c9ef360a7e887ca41/Client/mods/deathmatch/logic/lua/CLuaArgument.cpp#L912

Maybe fails?

https://github.com/multitheftauto/mtasa-blue/blame/40118f7875516304ff585c4c9ef360a7e887ca41/Shared/sdk/SharedUtil.Math.h#L47

Perhaps the value is too large? But this worked with 1.5.5, also it needs to work for larger ints because of the way we use it for milliseconds since epoch.

[ghost]

Basically, non of your provided Files are affecting this. Commit 424560d causes the problem.

• Luxy.c

[Dutchman101]

Basically, non of your provided Files are affecting this. Commit 424560d causes the problem.

• Luxy.c

did you test that to be certain? accurate testing would require you to build master, but with a manually downgraded json-c version.

[ghost]

Yes, i reverted changes by this commit and tested - expected result is returned:

[2018-09-25 07:36:00] Console executed command: toJSON({hey=1234567890000})
[2018-09-25 07:36:00] Command results: [ { "hey": 1234567890000 } ] [string]

• Luxy.c

[qaisjp]

Thanks for your bug report, we'll look into this.

cc @patrikjuvonen

[patrikjuvonen]

No I can certainly confirm this is an issue and has changed double behavior since the update which I did not test at the time. Sorry for any inconvenience.

json-c 0.13.1

[
  {
    "1234567890.1": 1234567890,
    "12345678901111.1": 12345678901111.1,
    "12345678901111.0": 12345678901111.0,
    "1234567890.0": 1234567890,
    "1234567890": 1234567890,
    "12.0": 12,
    "12.1": 12.1,
    "12345678.1": 12345678.1,
    "12345678901.0": 12345678901.0,
    "12345678901": 12345678901.0,
    "12": 12,
    "12345678901.1": 12345678901.1,
    "12345678901111": 12345678901111.0
  }
]

json-c 0.12

[
  {
    "1234567890.1": 1234567890,
    "12345678901111.1": 12345678901111.1,
    "12345678901111.0": 12345678901111,
    "1234567890.0": 1234567890,
    "1234567890": 1234567890,
    "12.0": 12,
    "12.1": 12.1,
    "12345678.1": 12345678.1,
    "12345678901.0": 12345678901,
    "12345678901": 12345678901,
    "12": 12,
    "12345678901.1": 12345678901.1,
    "12345678901111": 12345678901111
  }
]

Since we haven't appended a trailing zero before, do we want to change behavior? Changing behavior requires scripters to force a math.floor in code later on every time they deal with a double together with JSON if they don't want a trailing zero to appear all of a sudden.

Note that this behavior only occurs with a double. I guess we can raise a question why they changed this behavior in json-c library since the change seems very obvious and fairly easy to come across in the end.

As for whether this is a truly code-breaking issue, I don't think so. Trailing zero or not, the number is the same. Only problem I can think of is if you're counting on it not having any non-numeric characters, but at that point you might as well handle that case separately... and another issue would be a slight increase in storage size as it appends two characters more than it used to.

I thought I found the exact commits that caused this change in behavior but it seems it's somewhere deeper in the changes since 0.12 (modf produces same results). I'll try to dig up some more...

[qaisjp]

Difference visualised

@patrikjuvonen said: No I can certainly confirm this is an issue

So the extra 1 was a typo and the actual problem is the trailing .0. I've edited the original issue to reflect this.

@qaisjp said: if it is just a trailing .0 then I would close as wontfix

Testing against Python, json.dumps(1234567890.0) returns '12345678901111.0'. That is, it does not truncate the dot zero.

Lua stores all numbers (even numbers that happen to be integers) as doubles (in 5.1) and we shouldn't hide that behaviour. Yes, the behaviour has changed from before but I think it has changed for the better.

There is no real problem with the change unless people are (for some reason) doing equality comparisons on JSON strings... which is kinda an odd thing to do.

[qaisjp]

Hmm, our 1234567890.0 doesn't actually go to 1234567890.0 as I expected (see line 8 of screenshot), it goes to 1234567890... I'm not sure what to do now 🤔

[patrikjuvonen]

Hmm, our 123456789.0 doesn't actually go to 123456789.0, it goes to 123456789... I'm not sure what to do now 🤔

You mean the .1 one? I think it has to do with our custom accuracy (%.16g).

[braunsonm]

@patrikjuvonen @qaisjp I mean no disrespect but this is not a “better” behaviour.

It’s nonstandard JSON. If you guys plan to keep the change then please at least have an option to disable this. It breaks certain pipelines that expect integers in the JSON(since that’s what we pass it).

Furthermore, yes Lua can represent all integers as doubles but doesn't kt have lua_Integer data type for "long long" and a lua_isinteger check? It would become impossible to represent large integers if this change stays :(

In a particular example, JSON files are outputted and picked up by Filebeat for elasticsearch for logging capabilities. Doubles are expected and without them completely break the pipeline. Please consider adding an option if you plan to make a nonstandard the default.

[patrikjuvonen]

@patrikjuvonen @qaisjp I mean no disrespect but this is not a “better” behaviour.

It’s nonstandard JSON. If you guys plan to keep the change then please at least have an option to disable this. It breaks certain pipelines that expect integers in the JSON(since that’s what we pass it).

Furthermore, yes Lua can represent all integers as doubles but it does have lua_Integer data type for "long long" and a lua_isinteger check.

In a particular example, JSON files are outputted and picked up by Filebeat for elasticsearch for logging capabilities. Doubles are expected and without them completely break the pipeline. Please consider adding an option if you plan to make a nonstandard the default.

Well, a digit of any kind in the fractional part is standard JSON according to the ECMA-404 JSON Data Interchange Standard Syntax.

So it's not about the standard, it's about how we want our functions to behave, which in this case is indeed debatable. I don't know should we just remove the trailing zero part or wrap it around some flag. Either way we will end up changing json-c source code if we decide to do something about this.

[braunsonm]

What I refer to is it's not standard compared to other JSON serializers to implicitly convert integers to a double.

Anyways just hoping there will be someway soon to return to the previous functionality. I suppose the only way possible to represent a large integer as JSON in MTA is to now do some string manipulation to remove the .0.

[qaisjp]

@Chaosca No worries, we get what you mean. 😄

Both versions in this screenshot seem to have inconsistent behaviour so I'm not sure what we need to do here...

@patrikjuvonen I've just noticed you've edited your comment to reflect that you're still looking for the correct commit. Thanks — keep us updated! 😊

You mean the .1 one? I think it has to do with our custom accuracy (%.16g).

Whoops, I made a typo in my comment and forgot the extra 0. I've edited my comment to fix this and have rectified which line of the screenshot I'm referring to.

In addition, I would expect lines 9, 10, and 15 (in the screenshot) to have a trailing .0 (currently both versions snip off .0)

I find toJSON(1234567890.1) converting to 123456789 very odd... I'm guessing that's what you're referring to with custom accuracy?

---

Both the old and new behaviour seems like weird behaviour to me... I think we should do whatever Python does.

Running this:

# Python
print(json.dumps(
    {
        "1234567890.1": 1234567890.1,
        "12345678901111.1": 12345678901111.1,
        "12345678901111.0": 12345678901111.0,
        "1234567890.0": 1234567890.0,
        "1234567890": 1234567890,
        "12.0": 12.0,
        "12.1": 12.1,
        "12345678.1": 12345678.1,
        "12345678901.0": 12345678901.0,
        "12345678901": 12345678901,
        "12": 12,
        "12345678901.1": 12345678901.1,
        "12345678901111": 12345678901111
    },
    indent=4, separators=(',', ': ')
))

gives this:

// JSON
{
    "1234567890.1": 1234567890.1,
    "12345678901111.1": 12345678901111.1,
    "12345678901111.0": 12345678901111.0,
    "1234567890.0": 1234567890.0,
    "1234567890": 1234567890,
    "12.0": 12.0,
    "12.1": 12.1,
    "12345678.1": 12345678.1,
    "12345678901.0": 12345678901.0,
    "12345678901": 12345678901,
    "12": 12,
    "12345678901.1": 12345678901.1,
    "12345678901111": 12345678901111
}

Also... I realise I'm being contradictory about the integers in Lua bit... I dunno anymore

[patrikjuvonen]

I did a more throughough test using the following Lua table:

{
    {
        1,
        1.0,
        1.1,
        1.123
    },
    {
        12,
        12.0,
        12.1,
        12.123
    },
    {
        123,
        123.0,
        123.1,
        123.123
    },
    {
        1234,
        1234.0,
        1234.1,
        1234.123
    },
    {
        12345,
        12345.0,
        12345.1,
        12345.123
    },
    {
        123456,
        123456.0,
        123456.1,
        123456.123
    },
    {
        1234567,
        1234567.0,
        1234567.1,
        1234567.123
    },
    {
        12345678,
        12345678.0,
        12345678.1,
        12345678.123
    },
    {
        123456789,
        123456789.0,
        123456789.1,
        123456789.123
    },
    {
        1234567890,
        1234567890.0,
        1234567890.1,
        1234567890.123
    },
    {
        12345678901,
        12345678901.0,
        12345678901.1,
        12345678901.123
    },
    {
        123456789012,
        123456789012.0,
        123456789012.1,
        123456789012.123
    },
    {
        1234567890123,
        1234567890123.0,
        1234567890123.1,
        1234567890123.123
    },
    {
        12345678901234,
        12345678901234.0,
        12345678901234.1,
        12345678901234.123
    },
    {
        123456789012345,
        123456789012345.0,
        123456789012345.1,
        123456789012345.123
    },
    {
        1234567890123456,
        1234567890123456.0,
        1234567890123456.1,
        1234567890123456.123
    },
    {
        12345678901234567,
        12345678901234567.0,
        12345678901234567.1,
        12345678901234567.123
    },
    {
        123456789012345678,
        123456789012345678.0,
        123456789012345678.1,
        123456789012345678.123
    },
    {
        1234567890123456789,
        1234567890123456789.0,
        1234567890123456789.1,
        1234567890123456789.123
    },
    {
        12345678901234567890,
        12345678901234567890.0,
        12345678901234567890.1,
        12345678901234567890.123
    }
}

Essentially the only difference I found was a number value containing 11 or more characters will always have at least one fractional digit in 0.13.1, but not in 0.12.

Both share same sort of weird behavior here:

{
    123456789,    -- becomes 123456789
    123456789.0,  -- becomes 123456789
    123456789.1,  -- becomes 123456789
    123456789.123 -- becomes 123456789
},
{
    1234567890,    -- becomes 1234567890
    1234567890.0,  -- becomes 1234567890
    1234567890.1,  -- becomes 1234567890
    1234567890.123 -- becomes 1234567890
}

So I'm assuming that has to do with some int/float/double size/accuracy (either in Lua, json-c or somewhere else) and has been there for a while.
According to @ccw808 it indeed has to do with the initially referenced ShouldUseInt math method which initially wasn't made to support doubles, but should be adjusted to support them properly. So this is a separate issue, but good to realize now while messing with the stuff.

As for the trailing zeros I'm still searching for a clear reason behind it.
So far it seems the fractional zero is added just to make it look more like a double. We have a couple options to deal with this, all of which have to do with messing with json-c source code one way or the other.

Some topics that may be of interest:

• Problem serializing double json-c/json-c#394 (Problem serializing double)
• json-c/json-c@0992aac...c652b6a (latest change to double handling)
• bugfix: floating point representaion without fractional part  json-c/json-c#289 (bugfix: floating point representaion without fractional part)
• Make serialization format of doubles configurable json-c/json-c#193 (Make serialization format of doubles configurable)
• Fix double serialization for very large exponents that loose precision with %f printf format flag to %g format flag json-c/json-c#59 (Fix double serialization for very large exponents that loose precision with %f printf format flag to %g format flag)
• 4cc6fda#diff-b0cabd56ec4d6ebe1b3844142f7ba07e (Test to make json rounding more efficient by @ccw808)
• Float Point Precision RedisJSON/RedisJSON#6 (Float Point Precision)
• https://stackoverflow.com/questions/16839658/printf-width-specifier-to-maintain-precision-of-floating-point-value (Printf width specifier to maintain precision of floating-point value)
• d14e3a9 (Fixed #8128 - Integer data change when sending it between server and client)