Skip to content

Commit

Permalink
[logging] always escape control chars (elastic#170826)
Browse files Browse the repository at this point in the history
## Summary

Follow-up of elastic#169773

Escape message even if it's not a string
  • Loading branch information
pgayvallet authored Nov 8, 2023
1 parent c834113 commit 3d22359
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,52 @@ describe('MessageConversion', () => {
'\\u001b\\u0000[31mESC-INJECTION-LFUNICODE:\\u001b[32mSUCCESSFUL\\u001b[0m\\u0007\n\nInjecting 10.000 lols 😂\\u001b[10000;b\\u0007'
);
});

test('it should encode/escape ANSI chars lines from the message when not a string', () => {
expect(
MessageConversion.convert(
{
...baseRecord,
// @ts-expect-error message is supposed to be a string
message: {
toString: () => 'toString...\u001b[5;7;6mThis is Fine\u001b[27m',
},
},
false
)
).toEqual('toString...\\u001b[5;7;6mThis is Fine\\u001b[27m');
});

test('it should encode/escape ANSI chars lines from the error stack', () => {
const error = new Error('Something went bad');
error.stack = 'stack...\u001b[5;7;6mThis is Fine\u001b[27m';
expect(
MessageConversion.convert(
{
...baseRecord,
message: 'Some message that will be ignored',
error,
},
false
)
).toEqual('stack...\\u001b[5;7;6mThis is Fine\\u001b[27m');
});

test('it should encode/escape ANSI chars lines from the error stack when not a string', () => {
expect(
MessageConversion.convert(
{
...baseRecord,
message: 'Some message that will be ignored',
error: {
// @ts-expect-error message is supposed to be a string
stack: {
toString: () => 'stackToString...\u001b[5;7;6mThis is Fine\u001b[27m',
},
},
},
false
)
).toEqual('stackToString...\\u001b[5;7;6mThis is Fine\\u001b[27m');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,19 @@ export const MessageConversion: Conversion = {
pattern: /%message/g,
convert(record: LogRecord) {
// Error stack is much more useful than just the message.
const str = record.error?.stack || record.message;
let str = record.error?.stack || record.message;
// typings may be wrong, there's scenarios where the message is not a plain string (e.g error stacks from the ES client)
if (typeof str !== 'string') {
str = String(str);
}

return typeof str === 'string' // We need to validate it's a string because, despite types, there are use case where it's not a string :/
? str.replace(
CONTROL_CHAR_REGEXP,
// Escaping control chars via JSON.stringify to maintain consistency with `meta` and the JSON layout.
// This way, post analysis of the logs is easier as we can search the same patterns.
// Our benchmark didn't show a big difference in performance between custom-escaping vs. JSON.stringify one.
// The slice is removing the double-quotes.
(substr) => JSON.stringify(substr).slice(1, -1)
)
: str;
return str.replace(
CONTROL_CHAR_REGEXP,
// Escaping control chars via JSON.stringify to maintain consistency with `meta` and the JSON layout.
// This way, post analysis of the logs is easier as we can search the same patterns.
// Our benchmark didn't show a big difference in performance between custom-escaping vs. JSON.stringify one.
// The slice is removing the double-quotes.
(substr) => JSON.stringify(substr).slice(1, -1)
);
},
};

0 comments on commit 3d22359

Please sign in to comment.