Skip to content

Commit 9e7d980

Browse files
authored
fix(cli-repl): improve explicit display of special characters in output MONGOSH-1910 (#2330)
Escape control characters in output strings if there's a reasonable chance that their are (partially) coming from the server side, instead of printing them directly. We keep printing them directly when it's reasonably possible that they are being used intentionally for output control, e.g. when coming from `print()` or `console.log()` statements.
1 parent b9bdee8 commit 9e7d980

File tree

3 files changed

+243
-64
lines changed

3 files changed

+243
-64
lines changed

packages/cli-repl/src/format-output.spec.ts

Lines changed: 116 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
/* eslint no-control-regex: 0 */
2-
import formatRaw from './format-output';
2+
import type { FormatOptions } from './format-output';
3+
import { formatOutput } from './format-output';
34
import { expect } from 'chai';
45

56
for (const colors of [false, true]) {
67
describe(`formatOutput with 'colors' set to ${colors}`, function () {
7-
const format = (value: any): string => formatRaw(value, { colors });
8+
const format = (
9+
value: { value: unknown; type?: string | null },
10+
opts: Partial<FormatOptions> = {}
11+
): string => formatOutput(value, { colors, ...opts });
812
const stripAnsiColors = colors
913
? (str: string): string => str.replace(/\x1B[[(?);]{0,2}(;?\d)*./g, '')
1014
: (str: string): string => str;
@@ -15,6 +19,31 @@ for (const colors of [false, true]) {
1519
});
1620
});
1721

22+
context(
23+
'when the result is a string that only contains simple special characters',
24+
function () {
25+
it('returns the output', function () {
26+
expect(format({ value: 'test\n\ttest' })).to.equal('test\n\ttest');
27+
});
28+
}
29+
);
30+
31+
context(
32+
'when the result is a string that contains special characters',
33+
function () {
34+
it('returns the output', function () {
35+
expect(stripAnsiColors(format({ value: 'test\bfooo' }))).to.equal(
36+
"'test\\bfooo'"
37+
);
38+
});
39+
it('returns the raw value if control characters are allowed', function () {
40+
expect(
41+
format({ value: 'test\bfooo' }, { allowControlCharacters: true })
42+
).to.equal('test\bfooo');
43+
});
44+
}
45+
);
46+
1847
context('when the result is undefined', function () {
1948
it('returns the output', function () {
2049
expect(format({ value: undefined })).to.equal('');
@@ -253,10 +282,34 @@ for (const colors of [false, true]) {
253282
'\rError: Something went wrong\nCaused by: \n\rError: Something else went wrong'
254283
);
255284
});
285+
286+
it('escapes the message name if the error can be server-generated', function () {
287+
const output = stripAnsiColors(
288+
format({
289+
value: Object.assign(new Error('foo\bbar.'), {
290+
name: 'MongoServerError',
291+
}),
292+
type: 'Error',
293+
})
294+
);
295+
296+
expect(output).to.equal("\rMongoServerError: 'foo\\bbar.'");
297+
});
298+
299+
it('does not escape the message name if the error is a generic one', function () {
300+
const output = stripAnsiColors(
301+
format({
302+
value: Object.assign(new Error('foo\bbar.'), { name: 'FooError' }),
303+
type: 'Error',
304+
})
305+
);
306+
307+
expect(output).to.equal('\rFooError: foo\bbar.');
308+
});
256309
});
257310

258311
context('when the result is ShowDatabasesResult', function () {
259-
it('returns the help text', function () {
312+
it('returns the database list', function () {
260313
const output = stripAnsiColors(
261314
format({
262315
value: [
@@ -265,25 +318,27 @@ for (const colors of [false, true]) {
265318
{ name: 'supplies', sizeOnDisk: 2236416, empty: false },
266319
{ name: 'test', sizeOnDisk: 5664768, empty: false },
267320
{ name: 'test', sizeOnDisk: 599999768000, empty: false },
321+
{ name: 'ab\bdef', sizeOnDisk: 1234, empty: false },
268322
],
269323
type: 'ShowDatabasesResult',
270324
})
271325
);
272326

273-
expect(output).to.equal(
327+
expect(output.replace(/ +/g, ' ')).to.equal(
274328
`
275-
admin 44.00 KiB
276-
dxl 8.00 KiB
277-
supplies 2.13 MiB
278-
test 5.40 MiB
279-
test 558.79 GiB
329+
admin 44.00 KiB
330+
dxl 8.00 KiB
331+
supplies 2.13 MiB
332+
test 5.40 MiB
333+
test 558.79 GiB
334+
'ab\\bdef' 1.21 KiB
280335
`.trim()
281336
);
282337
});
283338
});
284339

285340
context('when the result is ShowCollectionsResult', function () {
286-
it('returns the help text', function () {
341+
it('returns the collections list', function () {
287342
const output = stripAnsiColors(
288343
format({
289344
value: [
@@ -292,13 +347,19 @@ test 558.79 GiB
292347
{ name: 'coll', badge: '' },
293348
{ name: 'people_imported', badge: '[view]' },
294349
{ name: 'cats', badge: '[time-series]' },
350+
{ name: 'cats\bcats', badge: '' },
295351
],
296352
type: 'ShowCollectionsResult',
297353
})
298354
);
299355

300-
expect(output).to.equal(
301-
'nested_documents\ndecimal128\ncoll\npeople_imported [view]\ncats [time-series]'
356+
expect(output.replace(/ +/g, ' ')).to.equal(
357+
'nested_documents\n' +
358+
'decimal128\n' +
359+
'coll\n' +
360+
'people_imported [view]\n' +
361+
'cats [time-series]\n' +
362+
"'cats\\bcats'"
302363
);
303364
});
304365
});
@@ -319,15 +380,41 @@ test 558.79 GiB
319380
'c1\n{ metadata: 1 }\n---\nc2\n{ metadata: 2 }'
320381
);
321382
});
383+
it('accounts for special characters and escapes them', function () {
384+
const output = stripAnsiColors(
385+
format({
386+
value: {
387+
['c\b1']: { metadata: 1 },
388+
c2: { metadata: 2 },
389+
},
390+
type: 'StatsResult',
391+
})
392+
);
393+
394+
expect(output).to.contain(
395+
"'c\\b1'\n{ metadata: 1 }\n---\nc2\n{ metadata: 2 }"
396+
);
397+
});
322398
});
323399

324400
context('when the result is ListCommandsResult', function () {
325401
it('returns the formatted list', function () {
326402
const output = stripAnsiColors(
327403
format({
328404
value: {
329-
c1: { metadata1: 1, help: 'help1' },
330-
c2: { metadata2: 2, help: 'help2' },
405+
c1: { metadata1: true, help: 'help1' },
406+
c2: {
407+
metadata2: true,
408+
help: 'help2',
409+
apiVersions: [],
410+
deprecatedApiVersions: [],
411+
},
412+
c3: {
413+
metadata2: true,
414+
help: 'help2',
415+
apiVersions: ['1'],
416+
deprecatedApiVersions: ['0'],
417+
},
331418
},
332419
type: 'ListCommandsResult',
333420
})
@@ -543,5 +630,20 @@ test 558.79 GiB
543630
expect(output).to.equal('------\n Header\n foo\n bar\n------\n');
544631
});
545632
});
633+
it('returns a formatted banner with escaped special characters', function () {
634+
const output = stripAnsiColors(
635+
format({
636+
value: {
637+
header: 'Heade\br',
638+
content: 'foo\bbar\n',
639+
},
640+
type: 'ShowBannerResult',
641+
})
642+
);
643+
644+
expect(output).to.equal(
645+
"------\n 'Heade\\br'\n 'foo\\bbar\\n'\n------\n"
646+
);
647+
});
546648
});
547649
}

0 commit comments

Comments
 (0)