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

libflux/flog: sanitize log messages containing line breaks #1530

Merged
merged 6 commits into from May 18, 2018

Conversation

Projects
None yet
3 participants
@garlick
Copy link
Member

garlick commented May 18, 2018

This PR sanitizes log messages that have trailing newlines or contain multiple lines. Any trailing CR or LF characters are stripped, and multiple lines are split into separate log messages.

garlick added some commits May 10, 2018

libutil/stdlog: drop trailing \r \n chars from msg
Problem: newlines and carriage returns appended to
log messages make the output hard to read.

Drop any number of trailing \r or \n characters
from message, after encoding.
libflux/flog: split multi-line log messages
Problem: if a log message contains multiple lines,
the line breaks are preserved in the message,
which causes flux-dmesg to display lines of log
messages with no header information.

Use stdlog_split_message() to break up lines into
separate log messages.

Fixes #1516
@coveralls

This comment has been minimized.

Copy link

coveralls commented May 18, 2018

Coverage Status

Coverage increased (+0.02%) to 79.081% when pulling a053e32 on garlick:log_split into 18fed52 on flux-framework:master.

@grondo

grondo approved these changes May 18, 2018

Copy link
Contributor

grondo left a comment

This is great! Just one question in stdlog_split_message but now that I think about it, it will be such a rare case it hardly matters...

Will merge once travis complets.

xtra[xtra_len] = '\0';
*len -= xtra_len; // truncate original (N.B. message is last, no \0 term)
while (xtra_len > 0 && strchr (sep, xtra[0]))
memmove (xtra, xtra + 1, xtra_len--); // drop leading separator(s)

This comment has been minimized.

@grondo

grondo May 18, 2018

Contributor

Just curious if this memmove could be eliminated if the offset was advanced past any leading separator(s) before memcpy?

This comment has been minimized.

@garlick

garlick May 18, 2018

Author Member

Yep that would have been a little better, though I'm inclined to agree that changing it now might not be worth the effort given that we don't expect multi-line log messages to occur much.

This comment has been minimized.

@grondo

grondo May 18, 2018

Contributor

yeah, perfectly fine, I was more curious if there was something I was missing..

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 18, 2018

Restarted a builder that mysteriously ERRORed in the t0017-security.t test

@grondo grondo merged commit 6fbffbd into flux-framework:master May 18, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 79.081%
Details

@garlick garlick deleted the garlick:log_split branch May 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.