-
-
Notifications
You must be signed in to change notification settings - Fork 376
Improved table encoding code to handle sparse arrays. #28
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
ad6ac0f
32a59c8
f521e4a
75127d2
0e6634a
8ce4842
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,44 +57,56 @@ end | |
|
|
||
|
|
||
| local function encode_table(val, stack) | ||
| local res = {} | ||
| stack = stack or {} | ||
|
|
||
| -- Circular reference? | ||
| if stack[val] then error("circular reference") end | ||
|
|
||
| stack[val] = true | ||
|
|
||
| if rawget(val, 1) ~= nil or next(val) == nil then | ||
| -- Treat as array -- check keys are valid and it is not sparse | ||
| local n = 0 | ||
| for k in pairs(val) do | ||
| if type(k) ~= "number" then | ||
| error("invalid table: mixed or invalid key types") | ||
| end | ||
| n = n + 1 | ||
| end | ||
| if n ~= #val then | ||
| error("invalid table: sparse array") | ||
| end | ||
| -- Encode | ||
| for i, v in ipairs(val) do | ||
| table.insert(res, encode(v, stack)) | ||
| end | ||
| stack[val] = nil | ||
| return "[" .. table.concat(res, ",") .. "]" | ||
|
|
||
| else | ||
| -- Treat as an object | ||
| for k, v in pairs(val) do | ||
| if type(k) ~= "string" then | ||
| error("invalid table: mixed or invalid key types") | ||
| end | ||
| table.insert(res, encode(k, stack) .. ":" .. encode(v, stack)) | ||
| end | ||
| stack[val] = nil | ||
| return "{" .. table.concat(res, ",") .. "}" | ||
| end | ||
| local res = {} | ||
| stack = stack or {} | ||
|
|
||
| -- Circular reference? | ||
| if stack[val] then error("circular reference") end | ||
|
|
||
| stack[val] = true | ||
| -- Check whether to treat as a array or object | ||
| local array = true | ||
| local length = 0 | ||
| local nLen = 0 | ||
| local count = 0 | ||
| for k,v in pairs(val) do | ||
| if (type(k) ~= "number" or k<=0) and not (k == "n" and type(v) == "number") then | ||
|
||
| array = nil | ||
| break -- Treat as object | ||
| else | ||
| if k > length then | ||
| length = k | ||
| end | ||
| if k == "n" and type(v) == "number" then | ||
| nLen = v | ||
| end | ||
| count = count + 1 | ||
| end | ||
| end | ||
|
|
||
| if nLen > length then | ||
| length = nLen | ||
| end | ||
| if array and not (length > 100 and count ~= length) then -- Check Array detected but sparse > 100 length then treat as object | ||
| -- Encode | ||
| for i=1,length do | ||
| table.insert(res, encode(val[i], stack)) | ||
| end | ||
| stack[val] = nil | ||
| return "[" .. table.concat(res, ",") .. "]" | ||
| else | ||
| -- Treat as an object | ||
| for k, v in pairs(val) do | ||
| --[[ | ||
| if type(k) ~= "string" then | ||
| error("invalid table: mixed or invalid key types") | ||
| end | ||
| ]] | ||
| table.insert(res, encode(k, stack) .. ":" .. encode(v, stack)) | ||
| end | ||
| stack[val] = nil | ||
| return "{" .. table.concat(res, ",") .. "}" | ||
| end | ||
| end | ||
|
|
||
|
|
||
|
|
@@ -108,7 +120,7 @@ local function encode_number(val) | |
| if val ~= val or val <= -math.huge or val >= math.huge then | ||
| error("unexpected number value '" .. tostring(val) .. "'") | ||
| end | ||
| return string.format("%.14g", val) | ||
| return tostring(val) | ||
|
||
| end | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered the worst-case this might create: What if I serialize
{[1e9] = true}?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed all these. Revisiting after a long time. Thanks for catching that. Had not considered it at all. Been using it, did not have issues. Fixed it in the next commit by preventing sparse arrays greater than 100 (arbitrarility) length getting through. Seems like other json modules also handle sparse arrays in a undefined way. 100 should be good for many cases I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a tricky issue, and behavior varies wildly. I believe ideally it should be left up to the user. Perhaps use something like a "density" factor as a threshold?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a variable called SPARSELIMIT in the module that the user can change after requiring the module to change it. Default set it to 100.