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

toJSON incorrectly stringifies object #1073

Open
Grafu opened this issue Aug 25, 2019 · 13 comments
Open

toJSON incorrectly stringifies object #1073

Grafu opened this issue Aug 25, 2019 · 13 comments
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@Grafu
Copy link
Contributor

Grafu commented Aug 25, 2019

Describe the bug

When serializing a standalone object, the returned JSON puts it into an array. This is incorrect because the object is not in an array before serialization. An argument for correct behavior should solve the problem.

To reproduce

srun outputChatBox(toJSON ( { "dogs", cat = "hungry", mouse = "food", birds = 4 } ))

Expected behaviour

Command results: { "1": "dogs", "mouse": "food", "cat": "hungry", "birds": 4 } [string]

Actual behaviour

Command results: [ { "1": "dogs", "mouse": "food", "cat": "hungry", "birds": 4 } ] [string]

This should only be triggered by:
toJSON ( { { "dogs", cat = "hungry", mouse = "food", birds = 4 } } )

Version

MTA:SA Server v1.5.6-release-16345
Multi Theft Auto v1.5.6-release-18706

@Grafu Grafu added the bug Something isn't working label Aug 25, 2019
@patrikjuvonen
Copy link
Contributor

Indeed we need a new argument for this correct behavior.

@patrikjuvonen patrikjuvonen added this to the Backlog milestone Aug 25, 2019
@dmi7ry
Copy link

dmi7ry commented Aug 25, 2019

Also I would like to have abillity to disable json optimisation

@qaisjp
Copy link
Contributor

qaisjp commented Aug 25, 2019

JSON optimisation?

@dmi7ry
Copy link

dmi7ry commented Aug 25, 2019

If there are same tables, toJSON() may store them as links on the first table, it looks like ... ,"^T^9","^T^9","^T^9", ... ,"^T^15" etc.

@qaisjp
Copy link
Contributor

qaisjp commented Aug 25, 2019

If there are same tables, toJSON() may store them as links on the first table, it looks like ... ,"^T^9","^T^9","^T^9", ... ,"^T^15" etc.

This seems like a bug and should only happen for self-referencing tables. Please can you create a separate issue for this?

i.e. this should not happen:

run a = { this = "that" }
run toJSON({ alpha = a, beta = a })
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": "^T^1" } ] [string]

Relevant: e883f1c

case LUA_TTABLE:
{
ulong* pTableId;
if (pKnownTables && (pTableId = MapFind(*pKnownTables, m_pTableData)))
{
// Self-referencing table
char szTableID[10];
snprintf(szTableID, sizeof(szTableID), "^T^%lu", *pTableId);
return json_object_new_string(szTableID);
}
else
{
return m_pTableData->WriteTableToJSONObject(bSerialize, pKnownTables);
}
}
(and the corresponding client file).

@dmi7ry
Copy link

dmi7ry commented Aug 25, 2019

I'm exactly about self-referencing tables.
So it would be useful, for example, if I need later edit the result by hands (in DB), or work with it on web, etc.

@qaisjp
Copy link
Contributor

qaisjp commented Aug 25, 2019

It is not possible to represent self-referencing tables in JSON

image

@dmi7ry
Copy link

dmi7ry commented Aug 25, 2019

I want it just writes a copy of the table instead of the link to it. Something like this:

run a = { this = "that" }
run toJSON({ alpha = a, beta = a }, true, "none", true ) -- last `true` to diable references on tables
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": { "this": "that" } } ] [string]

@patrikjuvonen
Copy link
Contributor

patrikjuvonen commented Aug 25, 2019

But how would a copy work? It's an infinite loop or I'm missing something?

Nevermind. Seems like something we should add an option for.

I propose we change toJSON to accept an options table with the current option arguments and add a new one named maybe useReferencing (?) (default: true).

@qaisjp
Copy link
Contributor

qaisjp commented Aug 25, 2019

I actually think it's a bug and only meant to be for truly self-referencing (recursive) tables — in this case I think we should fix the ^T happening without any args.

@Pirulax
Copy link
Contributor

Pirulax commented Nov 13, 2020

No shit, so thats what is causing MTA's famous double array JSON.. I see..
FYI the whole CLuaArgument class is bad as a whole.
I wanted to refactor it, but i got brain damage.

@Disinterpreter
Copy link
Member

I made a small "investigation" and found the problem in json-c library

Just look at 1387 and 1417-1418

static int json_object_array_to_json_string(struct json_object *jso, struct printbuf *pb, int level,
int flags)
{
int had_children = 0;
size_t ii;
printbuf_strappend(pb, "[");
if (flags & JSON_C_TO_STRING_PRETTY)
printbuf_strappend(pb, "\n");
for (ii = 0; ii < json_object_array_length(jso); ii++)
{
struct json_object *val;
if (had_children)
{
printbuf_strappend(pb, ",");
if (flags & JSON_C_TO_STRING_PRETTY)
printbuf_strappend(pb, "\n");
}
had_children = 1;
if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
printbuf_strappend(pb, " ");
indent(pb, level + 1, flags);
val = json_object_array_get_idx(jso, ii);
if (val == NULL)
printbuf_strappend(pb, "null");
else if (val->_to_json_string(val, pb, level + 1, flags) < 0)
return -1;
}
if (flags & JSON_C_TO_STRING_PRETTY)
{
if (had_children)
printbuf_strappend(pb, "\n");
indent(pb, level, flags);
}
if (flags & JSON_C_TO_STRING_SPACED && !(flags & JSON_C_TO_STRING_PRETTY))
return printbuf_strappend(pb, " ]");
return printbuf_strappend(pb, "]");
}

The authors of json-c always pack data into arrays, It cannot be fixed from MTA, as a solution we can just make a parameter which would be cut first and last brackets.

@hawicz
Copy link

hawicz commented Feb 4, 2021

If I'm reading things right, that toJSON call triggers the toJSON function which calls WriteToJSONString, which calls WriteToJSONArray to explicitly create an array type json_object and then calls json_object_to_json_string_ext on that.

The json-c code is doing exactly what you're asking it to do. This probably needs that extra array wrapping code removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants