JSON integer mangling #807

Closed
nnposter opened this Issue Mar 26, 2017 · 5 comments

Comments

Projects
None yet
3 participants

Function json.generate is using string.format("%g", obj) to convert numbers to strings. This has an undesired effect of mangling longer integers as follows:

obj = {session=1684119503}
json.make_object(obj)
str = json.generate(obj)

String str is now {"session": 1.68412e+009}.

I wonder if it would be more appropriate to use tostring(obj), which does not have this side-effect.

Digging into the history, the "%g" conversion has been there since the initial json.lua commit by David in 2010 (r16896).

Varunram commented Mar 27, 2017 edited

%g would round off to five places since its compact float, so clearly we'd have a problem with large numbers. But wouldn't changing the flag to %f solve the issue (unless its greater than 100,000,000,000,000 going by the docs)? @nnposter

Using %f would not work either:

$ lua
Lua 5.3.3  Copyright (C) 1994-2016 Lua.org, PUC-Rio
> x=123
> print(("%f"):format(x))
123.000000
>

Ah, forgot about the extra decimals that come with %f. The fix must work, don't see any side effects (at least none are mentioned in the docs) associated with them :)

Using tostring would be better in most cases, though it does introduce a ".0" at the end if the number is not a Lua Integer. Still, this removes control of the formatting a bit from the JSON library and puts it back in control of the calling script, which would be responsible for calling math.tointeger if necessary.

Here is an overview of the effect:

             ("%g):format()     tostring()
------------------------------------------
0:           0                  0
-3:          -3                 -3
123456789:   1.23457e+008       123456789 <---
0.06:        0.06               0.06
1.0:         1                  1.0       <---
1.23e30:     1.23e+030          1.23e+030
1.23e-030:   1.23e-030          1.23e-030
9e999        inf                inf
9e-999       0                  0.0       <---

IMHO tostring() is doing a better job in all these three cases.

@dmiller-nmap I understand and agree with what you are saying but I am not sure whether you are agreeing with the proposed change or you have concerns over it.

nmap-bot closed this in 3952e2f Apr 9, 2017

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