[logging] always escape control chars (#170826)

## Summary

Follow-up of https://github.com/elastic/kibana/pull/169773

Escape message even if it's not a string
This commit is contained in:
Pierre Gayvallet 2023-11-08 15:12:01 +01:00 committed by GitHub
parent c834113fb1
commit 3d2235949a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 11 deletions

View file

@ -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');
});
});

View file

@ -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)
);
},
};