ftp/eve: Convert to JsonBuilder #5030
Conversation
This commit converts the FTP logging mechanisms to use JsonBuilder.
This commit removes an unused helper function no longer required/used after conversion to JsonBuilder.
| json_object_set_new(cjs, "command_data", | ||
| JsonAddStringN((const char *)tx->request + min_length, | ||
| tx->request_length - min_length)); | ||
| char *s = BytesToString(tx->request + min_length, |
There was a problem hiding this comment.
@jasonish iirc we talked about a direct jb_set_bytes or something to avoid an expensive operation like this?
There was a problem hiding this comment.
I see jb_set_string_from_bytes ... That requires a null-terminated string which I don't have.
An interface that accepted a count and worked with a non-null-terminated string would be needed.
There was a problem hiding this comment.
jb_set_string_from_bytes should work, just looks like it doesn't have an extern "C" wrapper yet (haven't needed it from C yet would be the reason).
There was a problem hiding this comment.
| JsonAddStringN((const char *)where, 3)); | ||
| char *s = BytesToString(where, 3); | ||
| if (s != NULL) { | ||
| jb_append_string(js_respcode_list, s); |
There was a problem hiding this comment.
the goal of the jsonbuilder transition is to avoid creating temporary objects like this, but instead "stream" the whole object into a single jb.
There was a problem hiding this comment.
I modeled my implementation after that in output-json-email-common.c.
Note that I'm building 2 arrays as the response is processed and 2 arrays will be created during that. I'm not sure how the new interfaces support this?
There was a problem hiding this comment.
There isn't in some cases. In DNS, to avoid looping throught the responses 2x, I had to use intermediate objects as well in dns_log_json_answer.
| break; | ||
| case FTP_COMMAND_RETR: | ||
| json_object_set_new(ftpd, "command", json_string("RETR")); | ||
| jb_set_string(jb, "command", "RETR"); |
There was a problem hiding this comment.
@jasonish I still think we can optimize these constant cases with some preproc magic to become a single string that we append
There was a problem hiding this comment.
Yeah, need to look at this again.
|
Continued in #5049 |
Link to redmine ticket: 3714
Describe changes:
#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch: