to_json mishandles strings with both \ and unprintable characters

[zeek]

to_json mishandles strings with both \ and unprintable characters

[timwoj]

local g = table(
    [192.168.0.2, 22/tcp] = "ssh",
    [192.168.0.3, 80/tcp] = "http"
);

turns into

{"[\"192.168.0.2\",\"22/tcp\"]":"ssh","[\"192.168.0.3\",\"80/tcp\"]":"http"}

That looks ok and is readable enough.

[rsmmr]

Not quite sure which piece here triggered the 3.0 tag, but I'd suggest we tackle the part on moving to_json() into C++ bif.

[jsiwek]

Not quite sure which piece here triggered the 3.0 tag

Think it was mostly added in the context of looking for any "bad" bugs and this one seemed like it's a fairly broken usability issue. But I wouldn't consider it a showstopper for 3.0.

[timwoj]

At a quick glance, this is basically wanting to replace the to_json method in scripts/base/utils/json.zeek with one in a bif that's written in C++. Is that correct?

[jsiwek]

@timwoj yeah, my understanding of the scope here: re-implement to_json as BIF/C++ and also properly escape characters in strings that require it

[timwoj]

A couple questions so far:

• How should a set be represented? They're technically a table internally, but it's really just a list of values similar to a vector.
• Is it possible to call other non-C++ bif methods from inside C++ in a bif? For example, I could really use the record_fields method.

[rsmmr]

• Representing a set like a vector sounds good to me
• Yes, you can call them, though it's not pretty: look at build/src/bro.bif.func_h to see how record_fields ends up looking at the C++ level. In this specific case it might be easier to just replicate the code or factor it out into a separate C++ function.

[timwoj]

How should a table be represented when the key isn't an elementary type? Right now it looks like json.zeek just converts the key object into json and uses a string of that as the key for the table entry, but that seems ugly at best.

EDIT: This question actually applies to any non-string type. For example right now a table like this:

local e: table[count] of count = {
  [1] = 5,
  [2] = 6
  };

I have outputting as {"1":5,"2":6}. JSON keys must always be strings, so there has to be a conversion there somewhere.

[rsmmr]

For atomic keys, that sounds right: just convert to string. For composite keys, not sure ... converting them to string isn't great, but I don't see much better either; other than just declining to work on such tables.

[jsiwek]

For either an atomic key or a composite key, a Zeek table can be defined as a JSON object whose keys are a JSON string containing the content of any arbitrary Zeek value after its conversion to a JSON value. Doesn't seem that "ugly" to me -- it's recursively consistent (plus can't think of another way to do it).

Not sure if there's other gotchas, but inner JSON string values within the key at least need to escape their use of double-quote characters.

[timwoj]

Is there a reason why the field replacement doesn't get passed down to further calls of to_json in the existing code?

# replace the escape pattern in the field.
if( field_escape_pattern in field )
	field = cat(sub(field, field_escape_pattern, ""));

It only uses the user-provided pattern on the first call, and then uses the default of /^_ for any subsequent calls.

[timwoj]

Fixed in 1f329ad