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

ftp/eve: Convert to JsonBuilder #5049

Closed
wants to merge 4 commits into from
Closed

Conversation

jlucovsky
Copy link
Contributor

Continuation of #5030
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:

jasonish and others added 4 commits June 9, 2020 08:17
I think this is a bad use of transmute, while the end result
is the same, Box::from_raw is more correct as we created this
pointer with Box::into_raw.

(cherry picked from commit 40b1f51)
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.
Copy link
Member

@jasonish jasonish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see a SV test for ftp-data.. My quick attempts didn't show ftp-data being logged at all in master.

Comment on lines +142 to +147
fail:
if (js_resplist) {
jb_free(js_resplist);
} else {
jb_free(js_respcode_list);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not free both?

src/output-json-ftp.c Show resolved Hide resolved
src/output-json-ftp.c Show resolved Hide resolved
@jlucovsky
Copy link
Contributor Author

Would like to see a SV test for ftp-data.. My quick attempts didn't show ftp-data being logged at all in master.

Opened issue 3770 to track.

@jlucovsky
Copy link
Contributor Author

Continued in #5070

@jlucovsky jlucovsky closed this Jun 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants