From 19c627bbf03a2a5e38967124743f54e077775a7f Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 13:24:33 -0500 Subject: [PATCH 1/6] Fix string quoting to match other implementations --- tests/test_formatter.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index da0a8c0..3a0c984 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -48,6 +48,15 @@ def test_string_value(self): ])) self.assertEqual(data, '''key1="some random !@#$%^\\"&**_+-={}\\|;\':,./<>?)" key2="here\'s a line with\nmore stuff on the next"''') + + def test_string_value_quoting(self): + data = format_line(OrderedDict([ + ("measure.a", "1ms"), + ("measure.b", 10), + ("measure.c", "100MB"), + ("measure.d", "1s"), + ])) + self.assertEqual(data, "measure.a=1ms measure.b=10 measure.c=100MB measure.d=1s") From 562c40823651ba07886bbccd4adc9e17ee73e6d4 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 14:25:30 -0500 Subject: [PATCH 2/6] fix --- logfmt/formatter.py | 7 ++++++- tests/test_formatter.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/logfmt/formatter.py b/logfmt/formatter.py index e0937fa..ffb064f 100644 --- a/logfmt/formatter.py +++ b/logfmt/formatter.py @@ -16,7 +16,12 @@ def format_line(extra): else: if isinstance(v, (dict, object)): v = str(v) - v = '"%s"' % v.replace('"', '\\"') + needs_quoting = ' ' in v or '=' in v + needs_escaping = '"' in v + if needs_escaping: + v = '%s' % v.replace('"', '\\"') + if needs_quoting: + v = '"%s"' % v outarr.append("%s=%s" % (k, v)) return " ".join(outarr) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 3a0c984..0cac175 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -38,7 +38,7 @@ def test_custom_float_format(self): ("key2", "%.7f" % -234234234.2342342) ])) - self.assertEqual(data, "key1=342.23424 key2=\"-234234234.2342342\"") + self.assertEqual(data, "key1=342.23424 key2=-234234234.2342342") def test_string_value(self): data = format_line( OrderedDict([ @@ -50,6 +50,15 @@ def test_string_value(self): self.assertEqual(data, '''key1="some random !@#$%^\\"&**_+-={}\\|;\':,./<>?)" key2="here\'s a line with\nmore stuff on the next"''') def test_string_value_quoting(self): + """ + Other implementation do not quote strings when not necessary. + + Examples: + - https://github.com/kr/logfmt + - https://github.com/csquared/node-logfmt + - https://github.com/cyberdelia/logfmt-ruby + - https://github.com/Sirupsen/logrus + """ data = format_line(OrderedDict([ ("measure.a", "1ms"), ("measure.b", 10), From 051c352ae2a1fa1f8fc822db08075a45dbc65b3c Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 18:18:56 -0500 Subject: [PATCH 3/6] fmt --- tests/test_formatter.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 0cac175..a9668de 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -41,7 +41,7 @@ def test_custom_float_format(self): self.assertEqual(data, "key1=342.23424 key2=-234234234.2342342") def test_string_value(self): - data = format_line( OrderedDict([ + data = format_line(OrderedDict([ ("key1", """some random !@#$%^"&**_+-={}\\|;':,./<>?)"""), ("key2", """here's a line with more stuff on the next""") @@ -49,15 +49,9 @@ def test_string_value(self): self.assertEqual(data, '''key1="some random !@#$%^\\"&**_+-={}\\|;\':,./<>?)" key2="here\'s a line with\nmore stuff on the next"''') - def test_string_value_quoting(self): - """ - Other implementation do not quote strings when not necessary. - - Examples: - - https://github.com/kr/logfmt - - https://github.com/csquared/node-logfmt - - https://github.com/cyberdelia/logfmt-ruby - - https://github.com/Sirupsen/logrus + def test_kr_logfmt(self): + """ + Test using canonical example from https://github.com/kr/logfmt/blob/b84e30acd515aadc4b783ad4ff83aff3299bdfe0/example_test.go#L45 """ data = format_line(OrderedDict([ ("measure.a", "1ms"), From db8ff56bd0026af495b99fec2f1d920ae23a9efd Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 18:24:12 -0500 Subject: [PATCH 4/6] Reduce diff --- tests/test_formatter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index a9668de..4356353 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -41,7 +41,7 @@ def test_custom_float_format(self): self.assertEqual(data, "key1=342.23424 key2=-234234234.2342342") def test_string_value(self): - data = format_line(OrderedDict([ + data = format_line( OrderedDict([ ("key1", """some random !@#$%^"&**_+-={}\\|;':,./<>?)"""), ("key2", """here's a line with more stuff on the next""") From 9a7cc48b0309032073c2eb8df26d453c7c8d7c41 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 22:08:27 -0500 Subject: [PATCH 5/6] Add failing test for empty string --- tests/test_formatter.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 4356353..df9d1c9 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -9,6 +9,10 @@ class FormatterTestCase(TestCase): def test_empty_log_line(self): data = format_line({}) self.assertEqual(data, "") + + def test_empty_string(self): + data = format_line({'key':''}) + self.assertEqual(data, 'key=""') def test_key_without_value(self): data = format_line( OrderedDict([("key1", None), ("key2", None)]) ) From 8ea311d2d87a2ca0cbf5671466b4b1a403c2bbbb Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Sat, 5 Jan 2019 22:11:56 -0500 Subject: [PATCH 6/6] Fix test --- logfmt/formatter.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/logfmt/formatter.py b/logfmt/formatter.py index ffb064f..74cbf43 100644 --- a/logfmt/formatter.py +++ b/logfmt/formatter.py @@ -22,6 +22,8 @@ def format_line(extra): v = '%s' % v.replace('"', '\\"') if needs_quoting: v = '"%s"' % v + if v == "": + v = '""' outarr.append("%s=%s" % (k, v)) return " ".join(outarr)