From c18e3c462f257ec72d9187c95a62acc75399840c Mon Sep 17 00:00:00 2001 From: Marko Mikulicic Date: Wed, 23 Jun 2021 17:48:32 +0200 Subject: [PATCH] fix: Escape non-printable characters in logfmt The is no specification logfmt, most implementations only unescape `\"` and leave `\n` as is when parsing. Some implementations like go-logfmt do escape newlines as `\n` (and other chars according to Go's escaping rules). This PR just escapes non-printable characters using rust's string literal rules; they are not perfect but they don't seem to be worse than other players. (If this bites us some more we will probably be better of switching to json log output format and then equip ourselves with some simple shell scripts to reformat the logs on the fly while we're streaming them from k8s) As a bonus point, this effectively simplifies the escaping code, possibly also speeding it up a bit since it doesn't have to call replace twice (although I don't know what magic would the rust compiler have done to this double replace). Closes #1791 Closes influxdata/k8s-iox#1 --- logfmt/src/lib.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/logfmt/src/lib.rs b/logfmt/src/lib.rs index dfe70bee92..1edd5a0aa1 100644 --- a/logfmt/src/lib.rs +++ b/logfmt/src/lib.rs @@ -281,16 +281,15 @@ fn needs_quotes_and_escaping(value: &str) -> bool { return true; } - value.contains(' ') && !pre_quoted + let has_not_printable = value.bytes().any(|b| b <= b' '); + + has_not_printable && !pre_quoted } /// escape any characters in name as needed, otherwise return string as is fn quote_and_escape(value: &'_ str) -> Cow<'_, str> { if needs_quotes_and_escaping(value) { - Cow::Owned(format!( - "\"{}\"", - value.replace(r#"\"#, r#"\\"#).replace(r#"""#, r#"\""#) - )) + Cow::Owned(format!("{:?}", value)) } else { Cow::Borrowed(value) } @@ -376,4 +375,16 @@ mod test { r#""a \"0 \\\"1\\\" 2\" c""# ); } + + #[test] + fn quote_not_printable() { + assert_eq!(quote_and_escape("foo\nbar"), r#""foo\nbar""#); + assert_eq!(quote_and_escape("foo\r\nbar"), r#""foo\r\nbar""#); + assert_eq!(quote_and_escape("foo\0bar"), r#""foo\u{0}bar""#); + } + + #[test] + fn not_quote_unicode_unnecessarily() { + assert_eq!(quote_and_escape("mikuličić"), "mikuličić"); + } }