Skip to content

Commit 56a5819

Browse files
authored
fix: always quote control chars
None of the invisible ASCII Control chars 0 - 32 were replaced. This PR replaces them with a unicode representation, the same as go-logfmt does it. e.g. `\x00` → `"\u0000"` This is security critical, as currently `\r` and other things would be part of the raw log. It would e.g. allow via ANSI escape sequences to set information in a linux terminal, like the window name/title.
1 parent a394264 commit 56a5819

File tree

2 files changed

+21
-10
lines changed

2 files changed

+21
-10
lines changed

src/logfmter/formatter.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@
3535
"threadName",
3636
)
3737

38+
QUOTE_CHARS = str.maketrans(
39+
"", "", " =" + "".join(chr(c) for c in list(range(0x20)) + [0x7F])
40+
)
41+
REPLACEMENTS = {chr(c): f"\\u{c:04x}" for c in list(range(0x20)) + [0x7F]}
42+
REPLACEMENTS.update({"\n": "\\n", "\t": "\\t", "\r": "\\r"})
43+
3844

3945
class _DefaultFormatter(logging.Formatter):
4046
def format(self, record):
@@ -59,13 +65,7 @@ def format_string(cls, value: str) -> str:
5965
Process the provided string with any necessary quoting and/or escaping.
6066
"""
6167
needs_dquote_escaping = '"' in value
62-
needs_newline_escaping = "\n" in value
63-
needs_quoting = (
64-
" " in value
65-
or "=" in value
66-
or needs_dquote_escaping
67-
or needs_newline_escaping
68-
)
68+
needs_quoting = needs_dquote_escaping or value.translate(QUOTE_CHARS) != value
6969
needs_backslash_escaping = "\\" in value and needs_quoting
7070

7171
if needs_backslash_escaping:
@@ -74,8 +74,8 @@ def format_string(cls, value: str) -> str:
7474
if needs_dquote_escaping:
7575
value = value.replace('"', '\\"')
7676

77-
if needs_newline_escaping:
78-
value = value.replace("\n", "\\n")
77+
for char, replacement in REPLACEMENTS.items():
78+
value = value.replace(char, replacement)
7979

8080
if needs_quoting:
8181
value = '"{}"'.format(value)

tests/test_formatter.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,24 @@
1616
("=", '"="'),
1717
# All double quotes must be escaped.
1818
('"', '"\\""'),
19+
# Null bytes must be escaped and quoted
20+
("\x00", r'"\u0000"'),
21+
# All whitespace chars must be escaped and quoted
22+
("\n", '"\\n"'),
23+
("\r", '"\\r"'),
24+
("\t", '"\\t"'),
25+
# All other control chars must be escaped and quoted
26+
("\x07", r'"\u0007"'),
27+
(
28+
"".join(chr(c) for c in range(0x20) if chr(c) not in "\t\n\r"),
29+
r'"\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\u000b\u000c\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f"',
30+
),
1931
# If the string requires escaping and quoting, then both
2032
# operations should be performed.
2133
(' "', '" \\""'),
2234
# If the string is empty, then it should be left empty.
2335
("", ""),
2436
# If the string contains a newline, then it should be escaped.
25-
("\n", '"\\n"'),
2637
("\n\n", '"\\n\\n"'),
2738
# If the string contains a backslash and needs to be quoted, then
2839
# the backslashes need to be escaped.

0 commit comments

Comments
 (0)