From db612011bfe8c77dad96c894c8da351ec5e6c863 Mon Sep 17 00:00:00 2001 From: addievo Date: Thu, 2 Nov 2023 10:19:10 +1100 Subject: [PATCH 01/10] feat: encodes non printables and fix clean up table format fix: removed unneeded `arrayToAsyncIterable` utility function fix: made handling of '' and `null` cells consistent in `outputTableFormatter` fix: cleaned up tests in `utils.test.ts` fix: condensed arbitraries in `utils.test.ts` fix: removed unneeded concats of strings in `utils.test.ts` fix: `outputTableFormatter` is no longer async and `outputFormatter` no longer can return `Promise` fix: cleaned up `outputFormatter` for `list` type [ci-skip] --- src/nodes/CommandClaim.ts | 4 +- src/nodes/CommandConnections.ts | 4 +- src/nodes/CommandFind.ts | 2 +- src/nodes/CommandGetAll.ts | 2 +- src/nodes/CommandPing.ts | 2 +- src/notifications/CommandRead.ts | 2 +- src/secrets/CommandGet.ts | 2 +- src/secrets/CommandList.ts | 2 +- src/secrets/CommandStat.ts | 2 +- src/utils/utils.ts | 125 ++++++++++++++++--------------- src/vaults/CommandCreate.ts | 2 +- src/vaults/CommandList.ts | 2 +- src/vaults/CommandLog.ts | 2 +- src/vaults/CommandPermissions.ts | 2 +- src/vaults/CommandScan.ts | 2 +- tests/utils.test.ts | 63 +++++++++++++--- tests/vaults/vaults.test.ts | 10 ++- 17 files changed, 136 insertions(+), 94 deletions(-) diff --git a/src/nodes/CommandClaim.ts b/src/nodes/CommandClaim.ts index 4847fc36..d0a0ec63 100644 --- a/src/nodes/CommandClaim.ts +++ b/src/nodes/CommandClaim.ts @@ -65,7 +65,7 @@ class CommandClaim extends CommandPolykey { ); const claimed = response.success; if (claimed) { - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [ `Successfully generated a cryptolink claim on Keynode with ID ${nodesUtils.encodeNodeId( @@ -75,7 +75,7 @@ class CommandClaim extends CommandPolykey { }); process.stdout.write(formattedOutput); } else { - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [ `Successfully sent Gestalt Invite notification to Keynode with ID ${nodesUtils.encodeNodeId( diff --git a/src/nodes/CommandConnections.ts b/src/nodes/CommandConnections.ts index f67e8060..d47e0b82 100644 --- a/src/nodes/CommandConnections.ts +++ b/src/nodes/CommandConnections.ts @@ -59,7 +59,7 @@ class CommandAdd extends CommandPolykey { }, auth); if (options.format === 'human') { // Wait for outputFormatter to complete and then write to stdout - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: 'table', data: connections, options: { @@ -76,7 +76,7 @@ class CommandAdd extends CommandPolykey { process.stdout.write(formattedOutput); } else { // Wait for outputFormatter to complete and then write to stdout - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: 'json', data: connections, }); diff --git a/src/nodes/CommandFind.ts b/src/nodes/CommandFind.ts index 33d7230c..2576032d 100644 --- a/src/nodes/CommandFind.ts +++ b/src/nodes/CommandFind.ts @@ -90,7 +90,7 @@ class CommandFind extends CommandPolykey { } let output: any = result; if (options.format === 'human') output = [result.message]; - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); diff --git a/src/nodes/CommandGetAll.ts b/src/nodes/CommandGetAll.ts index 21b0da47..a0f754b0 100644 --- a/src/nodes/CommandGetAll.ts +++ b/src/nodes/CommandGetAll.ts @@ -60,7 +60,7 @@ class CommandGetAll extends CommandPolykey { `NodeId ${value.nodeIdEncoded}, Address ${value.host}:${value.port}, bucketIndex ${value.bucketIndex}`, ); } - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); diff --git a/src/nodes/CommandPing.ts b/src/nodes/CommandPing.ts index 49478e43..34ed2358 100644 --- a/src/nodes/CommandPing.ts +++ b/src/nodes/CommandPing.ts @@ -68,7 +68,7 @@ class CommandPing extends CommandPolykey { else status.message = error.message; const output: any = options.format === 'json' ? status : [status.message]; - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); diff --git a/src/notifications/CommandRead.ts b/src/notifications/CommandRead.ts index 543f33e2..81c38062 100644 --- a/src/notifications/CommandRead.ts +++ b/src/notifications/CommandRead.ts @@ -79,7 +79,7 @@ class CommandRead extends CommandPolykey { notifications.push(notification); } for (const notification of notifications) { - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'dict', data: notification, }); diff --git a/src/secrets/CommandGet.ts b/src/secrets/CommandGet.ts index 6b56281f..77649ba2 100644 --- a/src/secrets/CommandGet.ts +++ b/src/secrets/CommandGet.ts @@ -59,7 +59,7 @@ class CommandGet extends CommandPolykey { meta, ); const secretContent = Buffer.from(response.secretContent, 'binary'); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: 'raw', data: secretContent, }); diff --git a/src/secrets/CommandList.ts b/src/secrets/CommandList.ts index 193c2061..2b376c47 100644 --- a/src/secrets/CommandList.ts +++ b/src/secrets/CommandList.ts @@ -57,7 +57,7 @@ class CommandList extends CommandPolykey { return data; }, auth); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); diff --git a/src/secrets/CommandStat.ts b/src/secrets/CommandStat.ts index 31880a86..34440e30 100644 --- a/src/secrets/CommandStat.ts +++ b/src/secrets/CommandStat.ts @@ -66,7 +66,7 @@ class CommandStat extends CommandPolykey { } // Assuming the surrounding function is async - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data, }); diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 1d1b79aa..623fc29d 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -64,20 +64,45 @@ function standardErrorReplacer(key: string, value: any) { return value; } -async function* arrayToAsyncIterable(array: T[]): AsyncIterable { - for (const item of array) { - yield item; - } +/** + * This function: + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \n \r \t to escaped versions, \\n \\r and \\t. + * 3. Converts other control characters to their Unicode escape sequences. + */ +function encodeNonPrintable(str: string) { + // We want to actually match control codes here! + // eslint-disable-next-line no-control-regex + return str.replace(/[\x00-\x1F\x7F-\x9F]/g, (char) => { + switch (char) { + case ' ': + return char; // Preserve regular space + case '\n': + return '\\n'; // Encode newline + case '\r': + return '\\r'; // Encode carriage return + case '\t': + return '\\t'; // Encode tab + case '\v': + return '\\v'; // Encode tab + case '\f': + return '\\f'; // Encode tab + // Add cases for other whitespace characters if needed + default: + // Return the Unicode escape sequence for control characters + return `\\u${char.charCodeAt(0).toString(16).padStart(4, '0')}`; + } + }); } // Function to handle 'table' type output -const outputTableFormatter = async ( - rowStream: TableRow[] | AsyncIterable, +function outputTableFormatter( + rowStream: TableRow[], options?: TableOptions, -): Promise => { +): string { let output = ''; - const maxColumnLengths: Record = {}; let rowCount = 0; + const maxColumnLengths: Record = {}; // Initialize maxColumnLengths with header lengths if headers are provided if (options?.headers) { @@ -86,31 +111,27 @@ const outputTableFormatter = async ( } } - let iterableStream = Array.isArray(rowStream) - ? arrayToAsyncIterable(rowStream) - : rowStream; - - const updateMaxColumnLengths = (row: TableRow) => { - for (const [key, value] of Object.entries(row)) { - const cellValue = - value === null || value === '' || value === undefined ? 'N/A' : value; + // Precompute max column lengths by iterating over the rows first + for (const row of rowStream) { + for (const key in options?.headers ?? row) { + if (row[key] != null) { + row[key] = encodeNonPrintable(row[key].toString()); + } + // Row[key] is definitely a string or null after this point due to encodeNonPrintable + const cellValue: string | null = row[key]; + // Null or '' will both cause cellLength to be 3 + const cellLength = + cellValue == null || cellValue.length === 0 ? 3 : cellValue.length; // 3 is length of 'N/A' maxColumnLengths[key] = Math.max( maxColumnLengths[key] || 0, - cellValue.toString().length, + cellLength, // Use the length of the encoded value ); } - }; - - // Precompute max column lengths by iterating over the rows first - for await (const row of iterableStream) { - updateMaxColumnLengths(row); - } - - // Reset the iterableStream if it's an array so we can iterate over it again - if (Array.isArray(rowStream)) { - iterableStream = arrayToAsyncIterable(rowStream); } + // After this point, maxColumnLengths will have been filled with all the necessary keys. + // Thus, the column keys can be derived from it. + const columnKeys = Object.keys(maxColumnLengths); // If headers are provided, add them to your output first if (options?.headers) { const headerRow = options.headers @@ -119,48 +140,31 @@ const outputTableFormatter = async ( output += headerRow + '\n'; } - // Function to format a single row - const formatRow = (row: TableRow) => { + for (const row of rowStream) { let formattedRow = ''; - if (options?.includeRowCount) { formattedRow += `${++rowCount}\t`; } - - const keysToUse = options?.headers ?? Object.keys(maxColumnLengths); - - for (const key of keysToUse) { - const cellValue = Object.prototype.hasOwnProperty.call(row, key) - ? row[key] - : 'N/A'; - formattedRow += `${cellValue - ?.toString() - .padEnd(maxColumnLengths[key] || 0)}\t`; + for (const key of columnKeys) { + // Assume row[key] has been already encoded as a string or null + const cellValue = + row[key] == null || row[key].length === 0 ? 'N/A' : row[key]; + formattedRow += `${cellValue.padEnd(maxColumnLengths[key] || 0)}\t`; } - - return formattedRow.trimEnd(); - }; - - for await (const row of iterableStream) { - output += formatRow(row) + '\n'; + output += formattedRow.trimEnd() + '\n'; } return output; -}; +} -function outputFormatter( - msg: OutputObject, -): string | Uint8Array | Promise { +function outputFormatter(msg: OutputObject): string | Uint8Array { let output = ''; if (msg.type === 'raw') { return msg.data; } else if (msg.type === 'list') { - for (let elem in msg.data) { - // Empty string for null or undefined values - if (elem == null) { - elem = ''; - } - output += `${msg.data[elem]}\n`; + for (const elem of msg.data) { + // Convert null or undefined to empty string + output += `${elem != null ? encodeNonPrintable(elem) : ''}\n`; } } else if (msg.type === 'table') { return outputTableFormatter(msg.data, msg.options); @@ -178,12 +182,8 @@ function outputFormatter( value = ''; } - // Only trim starting and ending quotes if value is a string - if (typeof value === 'string') { - value = JSON.stringify(value).replace(/^"|"$/g, ''); - } else { - value = JSON.stringify(value); - } + value = JSON.stringify(value); + value = encodeNonPrintable(value); // Re-introduce value.replace logic from old code value = value.replace(/(?:\r\n|\n)$/, ''); @@ -323,6 +323,7 @@ export { outputFormatter, retryAuthentication, remoteErrorCause, + encodeNonPrintable, }; export type { OutputObject }; diff --git a/src/vaults/CommandCreate.ts b/src/vaults/CommandCreate.ts index bf89c439..4ef609c3 100644 --- a/src/vaults/CommandCreate.ts +++ b/src/vaults/CommandCreate.ts @@ -54,7 +54,7 @@ class CommandCreate extends CommandPolykey { }), meta, ); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [`Vault ${response.vaultIdEncoded} created successfully`], }); diff --git a/src/vaults/CommandList.ts b/src/vaults/CommandList.ts index b42d0d21..5eb4ac51 100644 --- a/src/vaults/CommandList.ts +++ b/src/vaults/CommandList.ts @@ -56,7 +56,7 @@ class CommandList extends CommandPolykey { } return data; }, meta); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); diff --git a/src/vaults/CommandLog.ts b/src/vaults/CommandLog.ts index 460b82f0..02b3dd95 100644 --- a/src/vaults/CommandLog.ts +++ b/src/vaults/CommandLog.ts @@ -63,7 +63,7 @@ class CommandLog extends CommandPolykey { } return data; }, meta); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); diff --git a/src/vaults/CommandPermissions.ts b/src/vaults/CommandPermissions.ts index 6a45ec24..5fdc9222 100644 --- a/src/vaults/CommandPermissions.ts +++ b/src/vaults/CommandPermissions.ts @@ -61,7 +61,7 @@ class CommandPermissions extends CommandPolykey { }, meta); if (data.length === 0) data.push('No permissions were found'); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); diff --git a/src/vaults/CommandScan.ts b/src/vaults/CommandScan.ts index 7a769210..59f4e730 100644 --- a/src/vaults/CommandScan.ts +++ b/src/vaults/CommandScan.ts @@ -58,7 +58,7 @@ class CommandScan extends CommandPolykey { } return data; }, meta); - const formattedOutput = await binUtils.outputFormatter({ + const formattedOutput = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); diff --git a/tests/utils.test.ts b/tests/utils.test.ts index b2c590f4..ea60e791 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -3,9 +3,21 @@ import ErrorPolykey from 'polykey/dist/ErrorPolykey'; import * as ids from 'polykey/dist/ids'; import * as nodesUtils from 'polykey/dist/nodes/utils'; import * as polykeyErrors from 'polykey/dist/errors'; +import * as fc from 'fast-check'; import * as binUtils from '@/utils/utils'; import * as testUtils from './utils'; +const nonPrintableCharArb = fc + .oneof( + fc.integer({ min: 0, max: 0x1f }), + fc.integer({ min: 0x7f, max: 0x9f }), + ) + .map((code) => String.fromCharCode(code)); + +const stringWithNonPrintableCharsArb = fc.stringOf( + fc.oneof(fc.char(), nonPrintableCharArb), +); + describe('bin/utils', () => { testUtils.testIf(testUtils.isTestPlatformEmpty)( 'list in human and json format', @@ -29,10 +41,7 @@ describe('bin/utils', () => { testUtils.testIf(testUtils.isTestPlatformEmpty)( 'table in human and in json format', async () => { - // Note the async here - // Table - const tableOutput = await binUtils.outputFormatter({ - // And the await here + const tableOutput = binUtils.outputFormatter({ type: 'table', data: [ { key1: 'value1', key2: 'value2' }, @@ -40,13 +49,10 @@ describe('bin/utils', () => { { key1: null, key2: undefined }, ], }); - expect(tableOutput).toBe( - 'value1\tvalue2\ndata1 \tdata2\nundefined\tundefined\n', - ); + expect(tableOutput).toBe('value1\tvalue2\ndata1 \tdata2\nN/A \tN/A\n'); // JSON - const jsonOutput = await binUtils.outputFormatter({ - // And the await here + const jsonOutput = binUtils.outputFormatter({ type: 'json', data: [ { key1: 'value1', key2: 'value2' }, @@ -67,19 +73,19 @@ describe('bin/utils', () => { type: 'dict', data: { key1: 'value1', key2: 'value2' }, }), - ).toBe('key1\tvalue1\nkey2\tvalue2\n'); + ).toBe('key1\t"value1"\nkey2\t"value2"\n'); expect( binUtils.outputFormatter({ type: 'dict', data: { key1: 'first\nsecond', key2: 'first\nsecond\n' }, }), - ).toBe('key1\tfirst\\nsecond\nkey2\tfirst\\nsecond\\n\n'); + ).toBe('key1\t"first\\nsecond"\nkey2\t"first\\nsecond\\n"\n'); expect( binUtils.outputFormatter({ type: 'dict', data: { key1: null, key2: undefined }, }), - ).toBe('key1\t\nkey2\t\n'); + ).toBe('key1\t""\nkey2\t""\n'); // JSON expect( binUtils.outputFormatter({ @@ -89,6 +95,39 @@ describe('bin/utils', () => { ).toBe('{"key1":"value1","key2":"value2"}\n'); }, ); + testUtils.testIf(testUtils.isTestPlatformEmpty)( + 'outputFormatter should encode non-printable characters within a dict', + () => { + fc.assert( + fc.property( + stringWithNonPrintableCharsArb, + stringWithNonPrintableCharsArb, + (key, value) => { + const formattedOutput = binUtils.outputFormatter({ + type: 'dict', + data: { [key]: value }, + }); + + // Construct the expected output + let expectedValue = JSON.stringify(value); + expectedValue = binUtils.encodeNonPrintable(expectedValue); + expectedValue = expectedValue.replace(/(?:\r\n|\n)$/, ''); + expectedValue = expectedValue.replace(/(\r\n|\n)/g, '$1\t'); + + const maxKeyLength = Math.max( + ...Object.keys({ [key]: value }).map((k) => k.length), + ); + const padding = ' '.repeat(maxKeyLength - key.length); + const expectedOutput = `${key}${padding}\t${expectedValue}\n`; + + // Assert that the formatted output matches the expected output + expect(formattedOutput).toBe(expectedOutput); + }, + ), + { numRuns: 100 }, // Number of times to run the test + ); + }, + ); testUtils.testIf(testUtils.isTestPlatformEmpty)( 'errors in human and json format', () => { diff --git a/tests/vaults/vaults.test.ts b/tests/vaults/vaults.test.ts index 663d4c8a..35578b1b 100644 --- a/tests/vaults/vaults.test.ts +++ b/tests/vaults/vaults.test.ts @@ -907,11 +907,13 @@ describe('CLI vaults', () => { cwd: dataDir, }); expect(result3.exitCode).toBe(0); + expect(result3.stdout).toMatch(/Vault1\\t\\t.*\\t\\tclone/); expect(result3.stdout).toContain( - `Vault1\t\t${vaultsUtils.encodeVaultId(vault1Id)}\t\tclone`, - ); - expect(result3.stdout).toContain( - `Vault2\t\t${vaultsUtils.encodeVaultId(vault2Id)}\t\tpull,clone`, + `Vault1\\t\\t${vaultsUtils.encodeVaultId( + vault1Id, + )}\\t\\tclone\nVault2\\t\\t${vaultsUtils.encodeVaultId( + vault2Id, + )}\\t\\tpull,clone\n`, ); expect(result3.stdout).not.toContain( `Vault3\t\t${vaultsUtils.encodeVaultId(vault3Id)}`, From 18a53310fe704c5cf2259c0f69e2fcd6cb3af4c1 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 17:18:53 +1100 Subject: [PATCH 02/10] fix: `tests/nodes/connections.test.ts` now binds services on port `0` to avoid conflicts [ci-skip] --- tests/nodes/connections.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/nodes/connections.test.ts b/tests/nodes/connections.test.ts index d2a59ef9..fe41375d 100644 --- a/tests/nodes/connections.test.ts +++ b/tests/nodes/connections.test.ts @@ -31,8 +31,8 @@ describe('connections', () => { nodePath, agentServiceHost: '127.0.0.1', clientServiceHost: '127.0.0.1', - agentServicePort: 55555, - clientServicePort: 55554, + agentServicePort: 0, + clientServicePort: 0, keys: { passwordOpsLimit: keysUtils.passwordOpsLimits.min, passwordMemLimit: keysUtils.passwordMemLimits.min, @@ -50,8 +50,8 @@ describe('connections', () => { nodePath: path.join(dataDir, 'remoteNode'), agentServiceHost: '127.0.0.1', clientServiceHost: '127.0.0.1', - agentServicePort: 55553, - clientServicePort: 55552, + agentServicePort: 0, + clientServicePort: 0, keys: { passwordOpsLimit: keysUtils.passwordOpsLimits.min, passwordMemLimit: keysUtils.passwordMemLimits.min, From 71183944bc2d01a9fde8c7c4cad5b42afa18a4a6 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 14:46:18 +1100 Subject: [PATCH 03/10] fix: literal `T[]` types converted to `Array` [ci-skip] --- src/identities/CommandPermissions.ts | 2 +- src/secrets/CommandStat.ts | 2 +- src/types.ts | 2 +- src/utils/parsers.ts | 3 +-- src/utils/utils.ts | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/identities/CommandPermissions.ts b/src/identities/CommandPermissions.ts index 813afe7b..10622ebf 100644 --- a/src/identities/CommandPermissions.ts +++ b/src/identities/CommandPermissions.ts @@ -52,7 +52,7 @@ class CommandPermissions extends CommandPolykey { logger: this.logger.getChild(PolykeyClient.name), }); const [type, id] = gestaltId; - let actions: string[] = []; + let actions: Array = []; switch (type) { case 'node': { diff --git a/src/secrets/CommandStat.ts b/src/secrets/CommandStat.ts index 34440e30..91fae074 100644 --- a/src/secrets/CommandStat.ts +++ b/src/secrets/CommandStat.ts @@ -60,7 +60,7 @@ class CommandStat extends CommandPolykey { meta, ); - const data: string[] = [`Stats for "${secretPath[1]}"`]; + const data: Array = [`Stats for "${secretPath[1]}"`]; for (const [key, value] of Object.entries(response.stat)) { data.push(`${key}: ${value}`); } diff --git a/src/types.ts b/src/types.ts index f29f3189..9440893d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -43,7 +43,7 @@ type AgentChildProcessOutput = type TableRow = Record; interface TableOptions { - headers?: string[]; + headers?: Array; includeRowCount?: boolean; } diff --git a/src/utils/parsers.ts b/src/utils/parsers.ts index 73e98348..3e29646b 100644 --- a/src/utils/parsers.ts +++ b/src/utils/parsers.ts @@ -98,8 +98,7 @@ const parseProviderId: (data: string) => ids.ProviderId = const parseIdentityId: (data: string) => ids.IdentityId = validateParserToArgParser(ids.parseIdentityId); - -const parseProviderIdList: (data: string) => ids.ProviderId[] = +const parseProviderIdList: (data: string) => Array = validateParserToArgListParser(ids.parseProviderId); const parseGestaltAction: (data: string) => 'notify' | 'scan' | 'claim' = diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 623fc29d..463cbde2 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -97,7 +97,7 @@ function encodeNonPrintable(str: string) { // Function to handle 'table' type output function outputTableFormatter( - rowStream: TableRow[], + rowStream: Array, options?: TableOptions, ): string { let output = ''; From fc43a59cd128a9f3ec22e9c1e0c4fa8d0316ff5a Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 15:52:33 +1100 Subject: [PATCH 04/10] feat: introduced mechanism to `outputTableFormatter` for consistant padding in streaming usage fix: moved positioning of `TableRow` and `TableOptions` type [ci-skip] --- src/nodes/CommandConnections.ts | 3 +- src/types.ts | 20 +++---- src/utils/utils.ts | 92 ++++++++++++++++++++++++--------- tests/utils.test.ts | 37 +++++++++++++ 4 files changed, 118 insertions(+), 34 deletions(-) diff --git a/src/nodes/CommandConnections.ts b/src/nodes/CommandConnections.ts index d47e0b82..372952da 100644 --- a/src/nodes/CommandConnections.ts +++ b/src/nodes/CommandConnections.ts @@ -63,7 +63,7 @@ class CommandAdd extends CommandPolykey { type: 'table', data: connections, options: { - headers: [ + columns: [ 'host', 'hostname', 'nodeIdEncoded', @@ -71,6 +71,7 @@ class CommandAdd extends CommandPolykey { 'timeout', 'usageCount', ], + includeHeaders: true, }, }); process.stdout.write(formattedOutput); diff --git a/src/types.ts b/src/types.ts index 9440893d..b9ab8afa 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,6 +5,15 @@ import type { RecoveryCode } from 'polykey/dist/keys/types'; import type { StatusLive } from 'polykey/dist/status/types'; import type { NodeIdEncoded } from 'polykey/dist/ids/types'; + +type TableRow = Record; + +interface TableOptions { + columns?: Array | Record; + includeHeaders?: boolean; + includeRowCount?: boolean; +} + type AgentStatusLiveData = Omit & { nodeId: NodeIdEncoded; }; @@ -40,17 +49,10 @@ type AgentChildProcessOutput = error: POJO; }; -type TableRow = Record; - -interface TableOptions { - headers?: Array; - includeRowCount?: boolean; -} - export type { + TableRow, + TableOptions, AgentStatusLiveData, AgentChildProcessInput, AgentChildProcessOutput, - TableRow, - TableOptions, }; diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 463cbde2..cf2b10c6 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -95,35 +95,61 @@ function encodeNonPrintable(str: string) { }); } -// Function to handle 'table' type output +/** + * Function to handle the `table` output format. + * @param rows + * @param options + * @param options.columns - Can either be an `Array` or `Record`. + * If it is `Record`, the `number` values will be used as the initial padding lengths. + * The object is also mutated if any cells exceed the inital padding lengths. + * This paramater can also be supplied to filter the columns that will be displayed. + * @param options.includeHeaders - Defaults to `True` + * @param options.includeRowCount - Defaults to `False`. + * @returns + */ function outputTableFormatter( - rowStream: Array, - options?: TableOptions, + rows: Array, + options: TableOptions = { + includeHeaders: true, + includeRowCount: false, + }, ): string { let output = ''; let rowCount = 0; + // Default includeHeaders to true + const includeHeaders = options.includeHeaders ?? true; const maxColumnLengths: Record = {}; + const optionColumns = + options?.columns != null + ? Array.isArray(options.columns) + ? options.columns + : Object.keys(options.columns) + : undefined; + // Initialize maxColumnLengths with header lengths if headers are provided - if (options?.headers) { - for (const header of options.headers) { - maxColumnLengths[header] = header.length; + if (optionColumns != null) { + for (const column of optionColumns) { + maxColumnLengths[column] = Math.max( + options?.columns?.[column] ?? 0, + column.length, + ); } } // Precompute max column lengths by iterating over the rows first - for (const row of rowStream) { - for (const key in options?.headers ?? row) { - if (row[key] != null) { - row[key] = encodeNonPrintable(row[key].toString()); + for (const row of rows) { + for (const column in options?.columns ?? row) { + if (row[column] != null) { + row[column] = encodeNonPrintable(row[column].toString()); } // Row[key] is definitely a string or null after this point due to encodeNonPrintable - const cellValue: string | null = row[key]; + const cellValue: string | null = row[column]; // Null or '' will both cause cellLength to be 3 const cellLength = cellValue == null || cellValue.length === 0 ? 3 : cellValue.length; // 3 is length of 'N/A' - maxColumnLengths[key] = Math.max( - maxColumnLengths[key] || 0, + maxColumnLengths[column] = Math.max( + maxColumnLengths[column] || 0, cellLength, // Use the length of the encoded value ); } @@ -131,25 +157,37 @@ function outputTableFormatter( // After this point, maxColumnLengths will have been filled with all the necessary keys. // Thus, the column keys can be derived from it. - const columnKeys = Object.keys(maxColumnLengths); + const columns = Object.keys(maxColumnLengths); // If headers are provided, add them to your output first - if (options?.headers) { - const headerRow = options.headers - .map((header) => header.padEnd(maxColumnLengths[header])) - .join('\t'); - output += headerRow + '\n'; + if (optionColumns != null) { + for (let i = 0; i < optionColumns.length; i++) { + const column = optionColumns[i]; + const maxColumnLength = maxColumnLengths[column]; + // Options.headers is definitely defined as optionHeaders != null + if (!Array.isArray(options!.columns)) { + options!.columns![column] = maxColumnLength; + } + if (includeHeaders) { + output += column.padEnd(maxColumnLength); + if (i !== optionColumns.length - 1) { + output += '\t'; + } else { + output += '\n'; + } + } + } } - for (const row of rowStream) { + for (const row of rows) { let formattedRow = ''; - if (options?.includeRowCount) { + if (options.includeRowCount) { formattedRow += `${++rowCount}\t`; } - for (const key of columnKeys) { + for (const column of columns) { // Assume row[key] has been already encoded as a string or null const cellValue = - row[key] == null || row[key].length === 0 ? 'N/A' : row[key]; - formattedRow += `${cellValue.padEnd(maxColumnLengths[key] || 0)}\t`; + row[column] == null || row[column].length === 0 ? 'N/A' : row[column]; + formattedRow += `${cellValue.padEnd(maxColumnLengths[column] || 0)}\t`; } output += formattedRow.trimEnd() + '\n'; } @@ -157,6 +195,12 @@ function outputTableFormatter( return output; } +/** + * Formats a message suitable for output. + * @param msg - The msg that needs to be formatted. + * @see {@link outputTableFormatter} for information regarding usage where `msg.type === 'table'`. + * @returns + */ function outputFormatter(msg: OutputObject): string | Uint8Array { let output = ''; if (msg.type === 'raw') { diff --git a/tests/utils.test.ts b/tests/utils.test.ts index ea60e791..7c915fa7 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -48,6 +48,9 @@ describe('bin/utils', () => { { key1: 'data1', key2: 'data2' }, { key1: null, key2: undefined }, ], + options: { + includeHeaders: true, + }, }); expect(tableOutput).toBe('value1\tvalue2\ndata1 \tdata2\nN/A \tN/A\n'); @@ -64,6 +67,40 @@ describe('bin/utils', () => { ); }, ); + testUtils.testIf(testUtils.isTestPlatformEmpty)( + 'table in human format for streaming usage', + async () => { + let tableOutput = ''; + const keys = { + key1: 7, + key2: 4, + }; + const generator = function* () { + yield [{ key1: 'value1', key2: 'value2' }]; + yield [{ key1: 'data1', key2: 'data2' }]; + yield [{ key1: null, key2: undefined }]; + }; + let i = 0; + for (const data of generator()) { + tableOutput += binUtils.outputFormatter({ + type: 'table', + data: data, + options: { + columns: keys, + includeHeaders: i === 0, + }, + }); + i++; + } + expect(keys).toStrictEqual({ + key1: 7, + key2: 6, + }); + expect(tableOutput).toBe( + 'key1 \tkey2 \nvalue1 \tvalue2\ndata1 \tdata2\nN/A \tN/A\n', + ); + }, + ); testUtils.testIf(testUtils.isTestPlatformEmpty)( 'dict in human and in json format', () => { From d5deb9dc9a7c2980b29d3d9826a8316008ffb910 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 16:53:51 +1100 Subject: [PATCH 05/10] fix: renamed `outputTableFormatter` to `outputFormatterTable` [ci-skip] --- src/utils/utils.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/utils/utils.ts b/src/utils/utils.ts index cf2b10c6..e6005787 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -97,17 +97,18 @@ function encodeNonPrintable(str: string) { /** * Function to handle the `table` output format. + * * @param rows * @param options * @param options.columns - Can either be an `Array` or `Record`. * If it is `Record`, the `number` values will be used as the initial padding lengths. * The object is also mutated if any cells exceed the inital padding lengths. - * This paramater can also be supplied to filter the columns that will be displayed. + * This parameter can also be supplied to filter the columns that will be displayed. * @param options.includeHeaders - Defaults to `True` * @param options.includeRowCount - Defaults to `False`. * @returns */ -function outputTableFormatter( +function outputFormatterTable( rows: Array, options: TableOptions = { includeHeaders: true, @@ -197,8 +198,9 @@ function outputTableFormatter( /** * Formats a message suitable for output. + * * @param msg - The msg that needs to be formatted. - * @see {@link outputTableFormatter} for information regarding usage where `msg.type === 'table'`. + * @see {@link outputFormatterTable} for information regarding usage where `msg.type === 'table'`. * @returns */ function outputFormatter(msg: OutputObject): string | Uint8Array { @@ -211,7 +213,7 @@ function outputFormatter(msg: OutputObject): string | Uint8Array { output += `${elem != null ? encodeNonPrintable(elem) : ''}\n`; } } else if (msg.type === 'table') { - return outputTableFormatter(msg.data, msg.options); + return outputFormatterTable(msg.data, msg.options); } else if (msg.type === 'dict') { let maxKeyLength = 0; for (const key in msg.data) { From 250665118d22334c5d6c015bda754938c2a1f7e8 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 16:54:24 +1100 Subject: [PATCH 06/10] fix: sectioned out `outputFormatter` into separate functions (`outputFormatterList`, `outputFormatterTable`, etc.) chore: renamed instances of `formattedOutput` to `outputFormatted` [ci-skip] --- src/nodes/CommandClaim.ts | 8 +- src/nodes/CommandConnections.ts | 8 +- src/nodes/CommandFind.ts | 4 +- src/nodes/CommandGetAll.ts | 4 +- src/nodes/CommandPing.ts | 4 +- src/notifications/CommandRead.ts | 4 +- src/secrets/CommandGet.ts | 4 +- src/secrets/CommandList.ts | 4 +- src/secrets/CommandStat.ts | 4 +- src/types.ts | 1 - src/utils/utils.ts | 195 +++++++++++++++++-------------- src/vaults/CommandCreate.ts | 4 +- src/vaults/CommandList.ts | 4 +- src/vaults/CommandLog.ts | 4 +- src/vaults/CommandPermissions.ts | 4 +- src/vaults/CommandScan.ts | 4 +- 16 files changed, 141 insertions(+), 119 deletions(-) diff --git a/src/nodes/CommandClaim.ts b/src/nodes/CommandClaim.ts index d0a0ec63..1d16b4c4 100644 --- a/src/nodes/CommandClaim.ts +++ b/src/nodes/CommandClaim.ts @@ -65,7 +65,7 @@ class CommandClaim extends CommandPolykey { ); const claimed = response.success; if (claimed) { - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [ `Successfully generated a cryptolink claim on Keynode with ID ${nodesUtils.encodeNodeId( @@ -73,9 +73,9 @@ class CommandClaim extends CommandPolykey { )}`, ], }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } else { - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [ `Successfully sent Gestalt Invite notification to Keynode with ID ${nodesUtils.encodeNodeId( @@ -83,7 +83,7 @@ class CommandClaim extends CommandPolykey { )}`, ], }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } } finally { if (pkClient! != null) await pkClient.stop(); diff --git a/src/nodes/CommandConnections.ts b/src/nodes/CommandConnections.ts index 372952da..62ddece0 100644 --- a/src/nodes/CommandConnections.ts +++ b/src/nodes/CommandConnections.ts @@ -59,7 +59,7 @@ class CommandAdd extends CommandPolykey { }, auth); if (options.format === 'human') { // Wait for outputFormatter to complete and then write to stdout - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: 'table', data: connections, options: { @@ -74,14 +74,14 @@ class CommandAdd extends CommandPolykey { includeHeaders: true, }, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } else { // Wait for outputFormatter to complete and then write to stdout - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: 'json', data: connections, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } } finally { if (pkClient! != null) await pkClient.stop(); diff --git a/src/nodes/CommandFind.ts b/src/nodes/CommandFind.ts index 2576032d..e2737636 100644 --- a/src/nodes/CommandFind.ts +++ b/src/nodes/CommandFind.ts @@ -90,11 +90,11 @@ class CommandFind extends CommandPolykey { } let output: any = result; if (options.format === 'human') output = [result.message]; - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); // Like ping it should error when failing to find node for automation reasons. if (!result.success) { throw new errors.ErrorPolykeyCLINodeFindFailed(result.message); diff --git a/src/nodes/CommandGetAll.ts b/src/nodes/CommandGetAll.ts index a0f754b0..8f71006c 100644 --- a/src/nodes/CommandGetAll.ts +++ b/src/nodes/CommandGetAll.ts @@ -60,11 +60,11 @@ class CommandGetAll extends CommandPolykey { `NodeId ${value.nodeIdEncoded}, Address ${value.host}:${value.port}, bucketIndex ${value.bucketIndex}`, ); } - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/nodes/CommandPing.ts b/src/nodes/CommandPing.ts index 34ed2358..410e1cf3 100644 --- a/src/nodes/CommandPing.ts +++ b/src/nodes/CommandPing.ts @@ -68,11 +68,11 @@ class CommandPing extends CommandPolykey { else status.message = error.message; const output: any = options.format === 'json' ? status : [status.message]; - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: output, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); if (error != null) throw error; } finally { diff --git a/src/notifications/CommandRead.ts b/src/notifications/CommandRead.ts index 81c38062..05ed1e3f 100644 --- a/src/notifications/CommandRead.ts +++ b/src/notifications/CommandRead.ts @@ -79,11 +79,11 @@ class CommandRead extends CommandPolykey { notifications.push(notification); } for (const notification of notifications) { - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'dict', data: notification, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } } finally { if (pkClient! != null) await pkClient.stop(); diff --git a/src/secrets/CommandGet.ts b/src/secrets/CommandGet.ts index 77649ba2..42040e53 100644 --- a/src/secrets/CommandGet.ts +++ b/src/secrets/CommandGet.ts @@ -59,11 +59,11 @@ class CommandGet extends CommandPolykey { meta, ); const secretContent = Buffer.from(response.secretContent, 'binary'); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: 'raw', data: secretContent, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/secrets/CommandList.ts b/src/secrets/CommandList.ts index 2b376c47..0dc2d3a7 100644 --- a/src/secrets/CommandList.ts +++ b/src/secrets/CommandList.ts @@ -57,12 +57,12 @@ class CommandList extends CommandPolykey { return data; }, auth); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/secrets/CommandStat.ts b/src/secrets/CommandStat.ts index 91fae074..5cc6934d 100644 --- a/src/secrets/CommandStat.ts +++ b/src/secrets/CommandStat.ts @@ -66,12 +66,12 @@ class CommandStat extends CommandPolykey { } // Assuming the surrounding function is async - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/types.ts b/src/types.ts index b9ab8afa..a0377f61 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,7 +5,6 @@ import type { RecoveryCode } from 'polykey/dist/keys/types'; import type { StatusLive } from 'polykey/dist/status/types'; import type { NodeIdEncoded } from 'polykey/dist/ids/types'; - type TableRow = Record; interface TableOptions { diff --git a/src/utils/utils.ts b/src/utils/utils.ts index e6005787..49717236 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -95,6 +95,39 @@ function encodeNonPrintable(str: string) { }); } +/** + * Formats a message suitable for output. + * + * @param msg - The msg that needs to be formatted. + * @see {@link outputFormatterTable} for information regarding usage where `msg.type === 'table'`. + * @returns + */ +function outputFormatter(msg: OutputObject): string | Uint8Array { + switch (msg.type) { + case 'raw': + return msg.data; + case 'list': + return outputFormatterList(msg.data); + case 'table': + return outputFormatterTable(msg.data, msg.options); + case 'dict': + return outputFormatterDict(msg.data); + case 'json': + return outputFormatterJson(msg.data); + case 'error': + return outputFormatterError(msg.data); + } +} + +function outputFormatterList(items: Array): string { + let output = ''; + for (const elem of items) { + // Convert null or undefined to empty string + output += `${elem != null ? encodeNonPrintable(elem) : ''}\n`; + } + return output; +} + /** * Function to handle the `table` output format. * @@ -196,109 +229,94 @@ function outputFormatterTable( return output; } -/** - * Formats a message suitable for output. - * - * @param msg - The msg that needs to be formatted. - * @see {@link outputFormatterTable} for information regarding usage where `msg.type === 'table'`. - * @returns - */ -function outputFormatter(msg: OutputObject): string | Uint8Array { +function outputFormatterDict(data: POJO): string { let output = ''; - if (msg.type === 'raw') { - return msg.data; - } else if (msg.type === 'list') { - for (const elem of msg.data) { - // Convert null or undefined to empty string - output += `${elem != null ? encodeNonPrintable(elem) : ''}\n`; + let maxKeyLength = 0; + for (const key in data) { + if (key.length > maxKeyLength) { + maxKeyLength = key.length; } - } else if (msg.type === 'table') { - return outputFormatterTable(msg.data, msg.options); - } else if (msg.type === 'dict') { - let maxKeyLength = 0; - for (const key in msg.data) { - if (key.length > maxKeyLength) { - maxKeyLength = key.length; - } + } + for (const key in data) { + let value = data[key]; + if (value == null) { + value = ''; } - for (const key in msg.data) { - let value = msg.data[key]; - if (value == null) { - value = ''; - } + value = JSON.stringify(value); + value = encodeNonPrintable(value); - value = JSON.stringify(value); - value = encodeNonPrintable(value); + // Re-introduce value.replace logic from old code + value = value.replace(/(?:\r\n|\n)$/, ''); + value = value.replace(/(\r\n|\n)/g, '$1\t'); - // Re-introduce value.replace logic from old code - value = value.replace(/(?:\r\n|\n)$/, ''); - value = value.replace(/(\r\n|\n)/g, '$1\t'); + const padding = ' '.repeat(maxKeyLength - key.length); + output += `${key}${padding}\t${value}\n`; + } + return output; +} - const padding = ' '.repeat(maxKeyLength - key.length); - output += `${key}${padding}\t${value}\n`; - } - } else if (msg.type === 'json') { - output = JSON.stringify(msg.data, standardErrorReplacer); - output += '\n'; - } else if (msg.type === 'error') { - let currError = msg.data; - let indent = ' '; - while (currError != null) { - if (currError instanceof networkErrors.ErrorPolykeyRemote) { - output += `${currError.name}: ${currError.description}`; - if (currError.message && currError.message !== '') { - output += ` - ${currError.message}`; - } - if (currError.metadata != null) { - output += '\n'; - for (const [key, value] of Object.entries(currError.metadata)) { - output += `${indent}${key}\t${value}\n`; - } - } - output += `${indent}timestamp\t${currError.timestamp}\n`; - output += `${indent}cause: `; - currError = currError.cause; - } else if (currError instanceof ErrorPolykey) { - output += `${currError.name}: ${currError.description}`; - if (currError.message && currError.message !== '') { - output += ` - ${currError.message}`; - } +function outputFormatterJson(json: string): string { + return `${JSON.stringify(json, standardErrorReplacer)}\n`; +} + +function outputFormatterError(err: Error): string { + let output = ''; + let indent = ' '; + while (err != null) { + if (err instanceof networkErrors.ErrorPolykeyRemote) { + output += `${err.name}: ${err.description}`; + if (err.message && err.message !== '') { + output += ` - ${err.message}`; + } + if (err.metadata != null) { output += '\n'; - // Disabled to streamline output - // output += `${indent}exitCode\t${currError.exitCode}\n`; - // output += `${indent}timestamp\t${currError.timestamp}\n`; - if (currError.data && !utils.isEmptyObject(currError.data)) { - output += `${indent}data\t${JSON.stringify(currError.data)}\n`; + for (const [key, value] of Object.entries(err.metadata)) { + output += `${indent}${key}\t${value}\n`; } - if (currError.cause) { - output += `${indent}cause: `; - if (currError.cause instanceof ErrorPolykey) { - currError = currError.cause; - } else if (currError.cause instanceof Error) { - output += `${currError.cause.name}`; - if (currError.cause.message && currError.cause.message !== '') { - output += `: ${currError.cause.message}`; - } - output += '\n'; - break; - } else { - output += `${JSON.stringify(currError.cause)}\n`; - break; + } + output += `${indent}timestamp\t${err.timestamp}\n`; + output += `${indent}cause: `; + err = err.cause; + } else if (err instanceof ErrorPolykey) { + output += `${err.name}: ${err.description}`; + if (err.message && err.message !== '') { + output += ` - ${err.message}`; + } + output += '\n'; + // Disabled to streamline output + // output += `${indent}exitCode\t${currError.exitCode}\n`; + // output += `${indent}timestamp\t${currError.timestamp}\n`; + if (err.data && !utils.isEmptyObject(err.data)) { + output += `${indent}data\t${JSON.stringify(err.data)}\n`; + } + if (err.cause) { + output += `${indent}cause: `; + if (err.cause instanceof ErrorPolykey) { + err = err.cause; + } else if (err.cause instanceof Error) { + output += `${err.cause.name}`; + if (err.cause.message && err.cause.message !== '') { + output += `: ${err.cause.message}`; } + output += '\n'; + break; } else { + output += `${JSON.stringify(err.cause)}\n`; break; } } else { - output += `${currError.name}`; - if (currError.message && currError.message !== '') { - output += `: ${currError.message}`; - } - output += '\n'; break; } - indent = indent + ' '; + } else { + output += `${err.name}`; + if (err.message && err.message !== '') { + output += `: ${err.message}`; + } + output += '\n'; + break; } + indent = indent + ' '; } return output; } @@ -367,6 +385,11 @@ export { verboseToLogLevel, standardErrorReplacer, outputFormatter, + outputFormatterList, + outputFormatterTable, + outputFormatterDict, + outputFormatterJson, + outputFormatterError, retryAuthentication, remoteErrorCause, encodeNonPrintable, diff --git a/src/vaults/CommandCreate.ts b/src/vaults/CommandCreate.ts index 4ef609c3..456cb886 100644 --- a/src/vaults/CommandCreate.ts +++ b/src/vaults/CommandCreate.ts @@ -54,11 +54,11 @@ class CommandCreate extends CommandPolykey { }), meta, ); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: [`Vault ${response.vaultIdEncoded} created successfully`], }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/vaults/CommandList.ts b/src/vaults/CommandList.ts index 5eb4ac51..fa3b7f80 100644 --- a/src/vaults/CommandList.ts +++ b/src/vaults/CommandList.ts @@ -56,11 +56,11 @@ class CommandList extends CommandPolykey { } return data; }, meta); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/vaults/CommandLog.ts b/src/vaults/CommandLog.ts index 02b3dd95..51852ce4 100644 --- a/src/vaults/CommandLog.ts +++ b/src/vaults/CommandLog.ts @@ -63,11 +63,11 @@ class CommandLog extends CommandPolykey { } return data; }, meta); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/vaults/CommandPermissions.ts b/src/vaults/CommandPermissions.ts index 5fdc9222..69251bd2 100644 --- a/src/vaults/CommandPermissions.ts +++ b/src/vaults/CommandPermissions.ts @@ -61,11 +61,11 @@ class CommandPermissions extends CommandPolykey { }, meta); if (data.length === 0) data.push('No permissions were found'); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } diff --git a/src/vaults/CommandScan.ts b/src/vaults/CommandScan.ts index 59f4e730..1d00bd1e 100644 --- a/src/vaults/CommandScan.ts +++ b/src/vaults/CommandScan.ts @@ -58,11 +58,11 @@ class CommandScan extends CommandPolykey { } return data; }, meta); - const formattedOutput = binUtils.outputFormatter({ + const outputFormatted = binUtils.outputFormatter({ type: options.format === 'json' ? 'json' : 'list', data: data, }); - process.stdout.write(formattedOutput); + process.stdout.write(outputFormatted); } finally { if (pkClient! != null) await pkClient.stop(); } From a0221bb228b7c8b0aa4e185133a0334e7e27b00e Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 6 Nov 2023 17:31:04 +1100 Subject: [PATCH 07/10] fix: removed expecting `agent status` to print certificates from `tests/keys/certchain.test.ts` and `tests/keys/cert.test.ts` --- tests/keys/cert.test.ts | 17 +---------------- tests/keys/certchain.test.ts | 17 +---------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/tests/keys/cert.test.ts b/tests/keys/cert.test.ts index 47c01d53..fade65f5 100644 --- a/tests/keys/cert.test.ts +++ b/tests/keys/cert.test.ts @@ -16,7 +16,7 @@ describe('cert', () => { testUtils.testIf( testUtils.isTestPlatformEmpty || testUtils.isTestPlatformDocker, )('cert gets the certificate', async () => { - let { exitCode, stdout } = await testUtils.pkExec( + const { exitCode, stdout } = await testUtils.pkExec( ['keys', 'cert', '--format', 'json'], { env: { @@ -31,20 +31,5 @@ describe('cert', () => { expect(JSON.parse(stdout)).toEqual({ cert: expect.any(String), }); - const certCommand = JSON.parse(stdout).cert; - ({ exitCode, stdout } = await testUtils.pkExec( - ['keys', 'cert', '--format', 'json'], - { - env: { - PK_NODE_PATH: agentDir, - PK_PASSWORD: agentPassword, - }, - cwd: agentDir, - command: globalThis.testCmd, - }, - )); - expect(exitCode).toBe(0); - const certStatus = JSON.parse(stdout).cert; - expect(certCommand).toBe(certStatus); }); }); diff --git a/tests/keys/certchain.test.ts b/tests/keys/certchain.test.ts index c6de7022..16e492f2 100644 --- a/tests/keys/certchain.test.ts +++ b/tests/keys/certchain.test.ts @@ -18,7 +18,7 @@ describe('certchain', () => { testUtils.testIf( testUtils.isTestPlatformEmpty || testUtils.isTestPlatformDocker, )('certchain gets the certificate chain', async () => { - let { exitCode, stdout } = await testUtils.pkExec( + const { exitCode, stdout } = await testUtils.pkExec( ['keys', 'certchain', '--format', 'json'], { env: { @@ -33,20 +33,5 @@ describe('certchain', () => { expect(JSON.parse(stdout)).toEqual({ certchain: expect.any(Array), }); - const certChainCommand = JSON.parse(stdout).certchain.join('\n'); - ({ exitCode, stdout } = await testUtils.pkExec( - ['agent', 'status', '--format', 'json'], - { - env: { - PK_NODE_PATH: agentDir, - PK_PASSWORD: agentPassword, - }, - cwd: agentDir, - command: globalThis.testCmd, - }, - )); - expect(exitCode).toBe(0); - const certChainStatus = JSON.parse(stdout).rootCertChainPem; - expect(certChainCommand.rootPublicKeyPem).toBe(certChainStatus); }); }); From 077b984e1829a4e5a9bcfaec48e0bfd32b868ee3 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Wed, 8 Nov 2023 09:30:37 +1100 Subject: [PATCH 08/10] fix: replaced usage of `\t` as padding in commands with `' '.repeat(2)` [ci-skip] --- src/keys/CommandCert.ts | 2 +- src/keys/CommandCertchain.ts | 2 +- src/keys/CommandDecrypt.ts | 2 +- src/keys/CommandEncrypt.ts | 2 +- src/keys/CommandSign.ts | 2 +- src/keys/CommandVerify.ts | 4 +++- src/utils/utils.ts | 21 +++++++++++++-------- src/vaults/CommandList.ts | 4 +++- src/vaults/CommandScan.ts | 6 +++++- 9 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/keys/CommandCert.ts b/src/keys/CommandCert.ts index 68c39dfa..8c94ebee 100644 --- a/src/keys/CommandCert.ts +++ b/src/keys/CommandCert.ts @@ -54,7 +54,7 @@ class CommandCert extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Root certificate:\t\t${result.cert}`]; + output = ['Root certificate:', result.cert]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/keys/CommandCertchain.ts b/src/keys/CommandCertchain.ts index 046e47e1..ff212622 100644 --- a/src/keys/CommandCertchain.ts +++ b/src/keys/CommandCertchain.ts @@ -57,7 +57,7 @@ class CommandsCertchain extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Root Certificate Chain:\t\t${result.certchain}`]; + output = ['Root Certificate Chain:', ...result.certchain]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/keys/CommandDecrypt.ts b/src/keys/CommandDecrypt.ts index d6760ea2..3aeec793 100644 --- a/src/keys/CommandDecrypt.ts +++ b/src/keys/CommandDecrypt.ts @@ -76,7 +76,7 @@ class CommandDecrypt extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Decrypted data:\t\t${result.decryptedData}`]; + output = [`Decrypted data:${' '.repeat(4)}${result.decryptedData}`]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/keys/CommandEncrypt.ts b/src/keys/CommandEncrypt.ts index c8e1ca8a..39c50ca4 100644 --- a/src/keys/CommandEncrypt.ts +++ b/src/keys/CommandEncrypt.ts @@ -103,7 +103,7 @@ class CommandEncypt extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Encrypted data:\t\t${result.encryptedData}`]; + output = [`Encrypted data:${' '.repeat(4)}${result.encryptedData}`]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/keys/CommandSign.ts b/src/keys/CommandSign.ts index 7a419be2..44f4c0f7 100644 --- a/src/keys/CommandSign.ts +++ b/src/keys/CommandSign.ts @@ -76,7 +76,7 @@ class CommandSign extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Signature:\t\t${result.signature}`]; + output = [`Signature:${' '.repeat(4)}${result.signature}`]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/keys/CommandVerify.ts b/src/keys/CommandVerify.ts index f2f3690d..c93ac84a 100644 --- a/src/keys/CommandVerify.ts +++ b/src/keys/CommandVerify.ts @@ -112,7 +112,9 @@ class CommandVerify extends CommandPolykey { }; let output: any = result; if (options.format === 'human') { - output = [`Signature verified:\t\t${result.signatureVerified}`]; + output = [ + `Signature verified:${' '.repeat(4)}${result.signatureVerified}`, + ]; } process.stdout.write( binUtils.outputFormatter({ diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 49717236..cb35d485 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -70,7 +70,7 @@ function standardErrorReplacer(key: string, value: any) { * 2. Converts \n \r \t to escaped versions, \\n \\r and \\t. * 3. Converts other control characters to their Unicode escape sequences. */ -function encodeNonPrintable(str: string) { +function encodeNonPrintable(str: string): string { // We want to actually match control codes here! // eslint-disable-next-line no-control-regex return str.replace(/[\x00-\x1F\x7F-\x9F]/g, (char) => { @@ -102,20 +102,25 @@ function encodeNonPrintable(str: string) { * @see {@link outputFormatterTable} for information regarding usage where `msg.type === 'table'`. * @returns */ -function outputFormatter(msg: OutputObject): string | Uint8Array { +function outputFormatter(msg: OutputObject): string { + let data = msg.data; switch (msg.type) { case 'raw': - return msg.data; + if (ArrayBuffer.isView(data)) { + const td = new TextDecoder('utf-8'); + data = encodeNonPrintable(td.decode(data)); + } + return data; case 'list': - return outputFormatterList(msg.data); + return outputFormatterList(data); case 'table': - return outputFormatterTable(msg.data, msg.options); + return outputFormatterTable(data, msg.options); case 'dict': - return outputFormatterDict(msg.data); + return outputFormatterDict(data); case 'json': - return outputFormatterJson(msg.data); + return outputFormatterJson(data); case 'error': - return outputFormatterError(msg.data); + return outputFormatterError(data); } } diff --git a/src/vaults/CommandList.ts b/src/vaults/CommandList.ts index fa3b7f80..8fa098fa 100644 --- a/src/vaults/CommandList.ts +++ b/src/vaults/CommandList.ts @@ -51,7 +51,9 @@ class CommandList extends CommandPolykey { }); for await (const vaultListMessage of stream) { data.push( - `${vaultListMessage.vaultName}:\t\t${vaultListMessage.vaultIdEncoded}`, + `${vaultListMessage.vaultName}:${' '.repeat(4)}${ + vaultListMessage.vaultIdEncoded + }`, ); } return data; diff --git a/src/vaults/CommandScan.ts b/src/vaults/CommandScan.ts index 1d00bd1e..191b958e 100644 --- a/src/vaults/CommandScan.ts +++ b/src/vaults/CommandScan.ts @@ -54,7 +54,11 @@ class CommandScan extends CommandPolykey { const vaultName = vault.vaultName; const vaultIdEncoded = vault.vaultIdEncoded; const permissions = vault.permissions.join(','); - data.push(`${vaultName}\t\t${vaultIdEncoded}\t\t${permissions}`); + data.push( + `${vaultName}${' '.repeat(4)}${vaultIdEncoded}${' '.repeat( + 5, + )}${permissions}`, + ); } return data; }, meta); From 469ce59f093bad96a27f89a48e849e11c07dcf65 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Wed, 8 Nov 2023 12:50:11 +1100 Subject: [PATCH 09/10] feat: character encoding is now only applied in the output formatter when strings are wrapped with `\"\"` double quotes fix: fixed raw escape character encoding on `outputFormatter` fix: encoding of quotes in `outputFormatter` is now correctly escaped [ci-skip] --- src/secrets/CommandGet.ts | 2 +- src/utils/utils.ts | 46 +++++++++++++++++++++++++++++++-------- tests/utils.test.ts | 24 ++++++++++++++------ 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/secrets/CommandGet.ts b/src/secrets/CommandGet.ts index 42040e53..e2e72047 100644 --- a/src/secrets/CommandGet.ts +++ b/src/secrets/CommandGet.ts @@ -61,7 +61,7 @@ class CommandGet extends CommandPolykey { const secretContent = Buffer.from(response.secretContent, 'binary'); const outputFormatted = binUtils.outputFormatter({ type: 'raw', - data: secretContent, + data: `"${binUtils.encodeQuotes(secretContent.toString('utf-8'))}"`, // Encodes any secret content with escape characters }); process.stdout.write(outputFormatted); } finally { diff --git a/src/utils/utils.ts b/src/utils/utils.ts index cb35d485..a9b3fa52 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -64,6 +64,22 @@ function standardErrorReplacer(key: string, value: any) { return value; } +/** + * This function calls encodeNonPrintable on substrings within the input enclosed with `""`. + * Any usage of `\\"` within `""` will escape it. + * + * @param str + * @see {@link encodeNonPrintable} + * @returns + */ +function encodeWrappedStrings(str: string) { + return str.replace(/"(\\.|[^"])*"/g, encodeNonPrintable); +} + +function encodeQuotes(str: string): string { + return str.replace(/[`"']/g, (char) => `\\` + char); +} + /** * This function: * 1. Keeps regular spaces, only ' ', as they are. @@ -100,6 +116,7 @@ function encodeNonPrintable(str: string): string { * * @param msg - The msg that needs to be formatted. * @see {@link outputFormatterTable} for information regarding usage where `msg.type === 'table'`. + * @see {@link encodeWrappedStrings} for information regarding wrapping strings with `""` for encoding escaped characters * @returns */ function outputFormatter(msg: OutputObject): string { @@ -108,8 +125,9 @@ function outputFormatter(msg: OutputObject): string { case 'raw': if (ArrayBuffer.isView(data)) { const td = new TextDecoder('utf-8'); - data = encodeNonPrintable(td.decode(data)); + data = td.decode(data); } + data = encodeWrappedStrings(data); return data; case 'list': return outputFormatterList(data); @@ -128,7 +146,7 @@ function outputFormatterList(items: Array): string { let output = ''; for (const elem of items) { // Convert null or undefined to empty string - output += `${elem != null ? encodeNonPrintable(elem) : ''}\n`; + output += `${elem != null ? encodeWrappedStrings(elem) : ''}\n`; } return output; } @@ -180,13 +198,17 @@ function outputFormatterTable( for (const row of rows) { for (const column in options?.columns ?? row) { if (row[column] != null) { - row[column] = encodeNonPrintable(row[column].toString()); + if (typeof row[column] === 'string') { + row[column] = encodeQuotes(row[column]); + row[column] = `"${row[column]}"`; + } else { + row[column] = JSON.stringify(row[column]); + } + row[column] = encodeWrappedStrings(row[column]); } - // Row[key] is definitely a string or null after this point due to encodeNonPrintable - const cellValue: string | null = row[column]; // Null or '' will both cause cellLength to be 3 const cellLength = - cellValue == null || cellValue.length === 0 ? 3 : cellValue.length; // 3 is length of 'N/A' + row[column] == null || row[column] === '""' ? 3 : row[column].length; // 3 is length of 'N/A' maxColumnLengths[column] = Math.max( maxColumnLengths[column] || 0, cellLength, // Use the length of the encoded value @@ -248,10 +270,14 @@ function outputFormatterDict(data: POJO): string { value = ''; } - value = JSON.stringify(value); - value = encodeNonPrintable(value); + if (typeof value === 'string') { + value = encodeQuotes(value); + value = `"${value}"`; + } else { + value = JSON.stringify(value); + } + value = encodeWrappedStrings(value); - // Re-introduce value.replace logic from old code value = value.replace(/(?:\r\n|\n)$/, ''); value = value.replace(/(\r\n|\n)/g, '$1\t'); @@ -397,6 +423,8 @@ export { outputFormatterError, retryAuthentication, remoteErrorCause, + encodeWrappedStrings, + encodeQuotes, encodeNonPrintable, }; diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 7c915fa7..5b4d0158 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -52,7 +52,9 @@ describe('bin/utils', () => { includeHeaders: true, }, }); - expect(tableOutput).toBe('value1\tvalue2\ndata1 \tdata2\nN/A \tN/A\n'); + expect(tableOutput).toBe( + '"value1"\t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', + ); // JSON const jsonOutput = binUtils.outputFormatter({ @@ -72,7 +74,7 @@ describe('bin/utils', () => { async () => { let tableOutput = ''; const keys = { - key1: 7, + key1: 10, key2: 4, }; const generator = function* () { @@ -93,11 +95,11 @@ describe('bin/utils', () => { i++; } expect(keys).toStrictEqual({ - key1: 7, - key2: 6, + key1: 10, + key2: 8, }); expect(tableOutput).toBe( - 'key1 \tkey2 \nvalue1 \tvalue2\ndata1 \tdata2\nN/A \tN/A\n', + 'key1 \tkey2 \n"value1" \t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', ); }, ); @@ -146,17 +148,17 @@ describe('bin/utils', () => { }); // Construct the expected output - let expectedValue = JSON.stringify(value); + let expectedValue = binUtils.encodeQuotes(value); expectedValue = binUtils.encodeNonPrintable(expectedValue); expectedValue = expectedValue.replace(/(?:\r\n|\n)$/, ''); expectedValue = expectedValue.replace(/(\r\n|\n)/g, '$1\t'); + expectedValue = `"${expectedValue}"`; const maxKeyLength = Math.max( ...Object.keys({ [key]: value }).map((k) => k.length), ); const padding = ' '.repeat(maxKeyLength - key.length); const expectedOutput = `${key}${padding}\t${expectedValue}\n`; - // Assert that the formatted output matches the expected output expect(formattedOutput).toBe(expectedOutput); }, @@ -279,4 +281,12 @@ describe('bin/utils', () => { ); }, ); + testUtils.testIf(testUtils.isTestPlatformEmpty)( + 'encoding non-printable strings works', + () => { + expect(binUtils.encodeWrappedStrings('\n"\n"')).toBe('\n"\\n"'); + expect(binUtils.encodeWrappedStrings('"\\""')).toBe('"\\""'); + expect(binUtils.encodeWrappedStrings('"\\"\n\\""')).toBe('"\\"\\n\\""'); + }, + ); }); From 37db5939019d61fdbc8eb78a183c4dc5f4905481 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Wed, 8 Nov 2023 16:26:08 +1100 Subject: [PATCH 10/10] fix: introduced `encodeEscaped` utility function for opt-in encoding chore: lintfix fix: removal of related tests to `encodeWrappedStrings` feat: encoding is now enabled/disabled based on if the string contains any encodable content feat: merged `encodeQuotes` and `encodePrintable` into `encodeEscaped` fix: corrected `encodeEscape` and `decodeEscape` behaviour regarding quotes and unicode escape sequences feat: tests for `encodeEscaped` and `decodeEscaped` fix: inline documentation for `decodeEscaped` fix: fixed expected formatting for `CommandScan` test in `vaults.test.ts` [ci-skip] --- src/secrets/CommandGet.ts | 6 +- src/utils/utils.ts | 154 ++++++++++++++++++++++++++---------- src/vaults/CommandScan.ts | 2 +- tests/utils.test.ts | 30 +++---- tests/vaults/vaults.test.ts | 14 ++-- 5 files changed, 141 insertions(+), 65 deletions(-) diff --git a/src/secrets/CommandGet.ts b/src/secrets/CommandGet.ts index e2e72047..917edd88 100644 --- a/src/secrets/CommandGet.ts +++ b/src/secrets/CommandGet.ts @@ -58,10 +58,12 @@ class CommandGet extends CommandPolykey { }), meta, ); - const secretContent = Buffer.from(response.secretContent, 'binary'); + const secretContent = response.secretContent; const outputFormatted = binUtils.outputFormatter({ type: 'raw', - data: `"${binUtils.encodeQuotes(secretContent.toString('utf-8'))}"`, // Encodes any secret content with escape characters + data: binUtils.encodeEscapedWrapped(secretContent) + ? binUtils.encodeEscaped(secretContent) + : secretContent, }); process.stdout.write(outputFormatted); } finally { diff --git a/src/utils/utils.ts b/src/utils/utils.ts index a9b3fa52..4854d1c1 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -50,7 +50,7 @@ type OutputObject = data: Error; }; -function standardErrorReplacer(key: string, value: any) { +function standardErrorReplacer(_key: string, value: any) { if (value instanceof Error && !(value instanceof ErrorPolykey)) { return { type: value.name, @@ -64,35 +64,64 @@ function standardErrorReplacer(key: string, value: any) { return value; } +function encodeEscapedReplacer(_key: string, value: any) { + if (typeof value === 'string') { + return encodeEscaped(value); + } + return value; +} + /** - * This function calls encodeNonPrintable on substrings within the input enclosed with `""`. - * Any usage of `\\"` within `""` will escape it. + * This function: * - * @param str - * @see {@link encodeNonPrintable} - * @returns + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\n \\r \\t to escaped versions, \\\\n \\\\r and \\\\t. + * 3. Converts other control characters to their Unicode escape sequences. + * 4. Converts ' \` " to escaped versions, \\\\' \\\\\` and \\\\" + * 5. Wraps the whole thing in `""` if any characters have been encoded. */ -function encodeWrappedStrings(str: string) { - return str.replace(/"(\\.|[^"])*"/g, encodeNonPrintable); +function encodeEscapedWrapped(str: string): string { + if (!encodeEscapedRegex.test(str)) { + return str; + } + return `"${encodeEscaped(str)}"`; } -function encodeQuotes(str: string): string { - return str.replace(/[`"']/g, (char) => `\\` + char); +/** + * This function: + * + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\\\n \\\\r and \\\\t to unescaped versions, \\n \\r \\t. + * 3. Converts Unicode escape sequences to their control characters. + * 4. Converts \\\\' \\\\\` and \\\\" to their unescaped versions, ' \` ". + * 5. If it is wrapped in "" double quotes, the double quotes will be trimmed. + */ +function decodeEscapedWrapped(str: string): string { + if (!decodeEscapedRegex.test(str)) { + return str; + } + return decodeEscaped(str.substring(1, str.length - 1)); } +// We want to actually match control codes here! +// eslint-disable-next-line no-control-regex +const encodeEscapedRegex = /[\x00-\x1F\x7F-\x9F"'`]/g; + /** * This function: + * * 1. Keeps regular spaces, only ' ', as they are. - * 2. Converts \n \r \t to escaped versions, \\n \\r and \\t. - * 3. Converts other control characters to their Unicode escape sequences. + * 2. Converts \\n \\r \\t to escaped versions, \\\\n \\\\r and \\\\t. + * 3. Converts other control characters to their Unicode escape sequences.\ + * 4. Converts ' \` " to escaped versions, \\\\' \\\\\` and \\\\" + * + * Unless you're using this in a `JSON.stringify` replacer, you probably want to use {@link encodeEscapedWrapped} instead. */ -function encodeNonPrintable(str: string): string { - // We want to actually match control codes here! - // eslint-disable-next-line no-control-regex - return str.replace(/[\x00-\x1F\x7F-\x9F]/g, (char) => { +function encodeEscaped(str: string): string { + return str.replace(encodeEscapedRegex, (char) => { switch (char) { case ' ': - return char; // Preserve regular space + return char; case '\n': return '\\n'; // Encode newline case '\r': @@ -103,6 +132,10 @@ function encodeNonPrintable(str: string): string { return '\\v'; // Encode tab case '\f': return '\\f'; // Encode tab + case '"': + case "'": + case '`': + return '\\' + char; // Add cases for other whitespace characters if needed default: // Return the Unicode escape sequence for control characters @@ -111,6 +144,49 @@ function encodeNonPrintable(str: string): string { }); } +const decodeEscapedRegex = /\\([nrtvf"'`]|u[0-9a-fA-F]{4})/g; + +/** + * This function: + * + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\\\n \\\\r and \\\\t to unescaped versions, \\n \\r \\t. + * 3. Converts Unicode escape sequences to their control characters. + * 4. Converts \\\\' \\\\\` and \\\\" to their unescaped versions, ' \` ". + * + * Unless you're using this in a `JSON.parse` reviver, you probably want to use {@link decodeEscapedWrapped} instead. + */ +function decodeEscaped(str: string): string { + return str.replace(decodeEscapedRegex, (substr) => { + // Unicode escape sequence must be characters (e.g. `\u0000`) + if (substr.length === 6 && substr.at(1) === 'u') { + return String.fromCharCode(parseInt(substr.substring(2), 16)); + } + // Length of substr will always be at least 1 + const lastChar = substr.at(-1); + if (lastChar == null) { + utils.never(); + } + switch (lastChar) { + case 'n': + return '\n'; + case 'r': + return '\r'; + case 't': + return '\t'; + case 'v': + return '\v'; + case 'f': + return '\f'; + case '"': + case "'": + case '`': + return lastChar; + } + utils.never(); + }); +} + /** * Formats a message suitable for output. * @@ -119,26 +195,20 @@ function encodeNonPrintable(str: string): string { * @see {@link encodeWrappedStrings} for information regarding wrapping strings with `""` for encoding escaped characters * @returns */ -function outputFormatter(msg: OutputObject): string { - let data = msg.data; +function outputFormatter(msg: OutputObject): string | Uint8Array { switch (msg.type) { case 'raw': - if (ArrayBuffer.isView(data)) { - const td = new TextDecoder('utf-8'); - data = td.decode(data); - } - data = encodeWrappedStrings(data); - return data; + return msg.data; case 'list': - return outputFormatterList(data); + return outputFormatterList(msg.data); case 'table': - return outputFormatterTable(data, msg.options); + return outputFormatterTable(msg.data, msg.options); case 'dict': - return outputFormatterDict(data); + return outputFormatterDict(msg.data); case 'json': - return outputFormatterJson(data); + return outputFormatterJson(msg.data); case 'error': - return outputFormatterError(data); + return outputFormatterError(msg.data); } } @@ -146,7 +216,7 @@ function outputFormatterList(items: Array): string { let output = ''; for (const elem of items) { // Convert null or undefined to empty string - output += `${elem != null ? encodeWrappedStrings(elem) : ''}\n`; + output += `${elem ?? ''}\n`; } return output; } @@ -199,12 +269,10 @@ function outputFormatterTable( for (const column in options?.columns ?? row) { if (row[column] != null) { if (typeof row[column] === 'string') { - row[column] = encodeQuotes(row[column]); - row[column] = `"${row[column]}"`; + row[column] = encodeEscapedWrapped(row[column]); } else { - row[column] = JSON.stringify(row[column]); + row[column] = JSON.stringify(row[column], encodeEscapedReplacer); } - row[column] = encodeWrappedStrings(row[column]); } // Null or '' will both cause cellLength to be 3 const cellLength = @@ -271,12 +339,10 @@ function outputFormatterDict(data: POJO): string { } if (typeof value === 'string') { - value = encodeQuotes(value); - value = `"${value}"`; + value = encodeEscapedWrapped(value); } else { - value = JSON.stringify(value); + value = JSON.stringify(value, encodeEscapedReplacer); } - value = encodeWrappedStrings(value); value = value.replace(/(?:\r\n|\n)$/, ''); value = value.replace(/(\r\n|\n)/g, '$1\t'); @@ -415,6 +481,7 @@ function remoteErrorCause(e: any): [any, number] { export { verboseToLogLevel, standardErrorReplacer, + encodeEscapedReplacer, outputFormatter, outputFormatterList, outputFormatterTable, @@ -423,9 +490,12 @@ export { outputFormatterError, retryAuthentication, remoteErrorCause, - encodeWrappedStrings, - encodeQuotes, - encodeNonPrintable, + encodeEscapedWrapped, + encodeEscaped, + encodeEscapedRegex, + decodeEscapedWrapped, + decodeEscaped, + decodeEscapedRegex, }; export type { OutputObject }; diff --git a/src/vaults/CommandScan.ts b/src/vaults/CommandScan.ts index 191b958e..ab017da8 100644 --- a/src/vaults/CommandScan.ts +++ b/src/vaults/CommandScan.ts @@ -56,7 +56,7 @@ class CommandScan extends CommandPolykey { const permissions = vault.permissions.join(','); data.push( `${vaultName}${' '.repeat(4)}${vaultIdEncoded}${' '.repeat( - 5, + 4, )}${permissions}`, ); } diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 5b4d0158..7751dbe1 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -52,9 +52,7 @@ describe('bin/utils', () => { includeHeaders: true, }, }); - expect(tableOutput).toBe( - '"value1"\t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', - ); + expect(tableOutput).toBe('value1\tvalue2\ndata1 \tdata2\nN/A \tN/A\n'); // JSON const jsonOutput = binUtils.outputFormatter({ @@ -96,10 +94,10 @@ describe('bin/utils', () => { } expect(keys).toStrictEqual({ key1: 10, - key2: 8, + key2: 6, }); expect(tableOutput).toBe( - 'key1 \tkey2 \n"value1" \t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', + 'key1 \tkey2 \nvalue1 \tvalue2\ndata1 \tdata2\nN/A \tN/A\n', ); }, ); @@ -112,7 +110,7 @@ describe('bin/utils', () => { type: 'dict', data: { key1: 'value1', key2: 'value2' }, }), - ).toBe('key1\t"value1"\nkey2\t"value2"\n'); + ).toBe('key1\tvalue1\nkey2\tvalue2\n'); expect( binUtils.outputFormatter({ type: 'dict', @@ -124,7 +122,7 @@ describe('bin/utils', () => { type: 'dict', data: { key1: null, key2: undefined }, }), - ).toBe('key1\t""\nkey2\t""\n'); + ).toBe('key1\t\nkey2\t\n'); // JSON expect( binUtils.outputFormatter({ @@ -148,11 +146,10 @@ describe('bin/utils', () => { }); // Construct the expected output - let expectedValue = binUtils.encodeQuotes(value); - expectedValue = binUtils.encodeNonPrintable(expectedValue); + let expectedValue = value; + expectedValue = binUtils.encodeEscapedWrapped(expectedValue); expectedValue = expectedValue.replace(/(?:\r\n|\n)$/, ''); expectedValue = expectedValue.replace(/(\r\n|\n)/g, '$1\t'); - expectedValue = `"${expectedValue}"`; const maxKeyLength = Math.max( ...Object.keys({ [key]: value }).map((k) => k.length), @@ -282,11 +279,16 @@ describe('bin/utils', () => { }, ); testUtils.testIf(testUtils.isTestPlatformEmpty)( - 'encoding non-printable strings works', + 'encodeEscaped should encode all escapable characters', () => { - expect(binUtils.encodeWrappedStrings('\n"\n"')).toBe('\n"\\n"'); - expect(binUtils.encodeWrappedStrings('"\\""')).toBe('"\\""'); - expect(binUtils.encodeWrappedStrings('"\\"\n\\""')).toBe('"\\"\\n\\""'); + fc.assert( + fc.property(stringWithNonPrintableCharsArb, (value) => { + expect(binUtils.decodeEscaped(binUtils.encodeEscaped(value))).toBe( + value, + ); + }), + { numRuns: 100 }, // Number of times to run the test + ); }, ); }); diff --git a/tests/vaults/vaults.test.ts b/tests/vaults/vaults.test.ts index 35578b1b..4f3d04b7 100644 --- a/tests/vaults/vaults.test.ts +++ b/tests/vaults/vaults.test.ts @@ -907,16 +907,18 @@ describe('CLI vaults', () => { cwd: dataDir, }); expect(result3.exitCode).toBe(0); - expect(result3.stdout).toMatch(/Vault1\\t\\t.*\\t\\tclone/); + expect(result3.stdout).toMatch(/Vault1 {4}.* {4}clone/); expect(result3.stdout).toContain( - `Vault1\\t\\t${vaultsUtils.encodeVaultId( + `Vault1${' '.repeat(4)}${vaultsUtils.encodeVaultId( vault1Id, - )}\\t\\tclone\nVault2\\t\\t${vaultsUtils.encodeVaultId( - vault2Id, - )}\\t\\tpull,clone\n`, + )}${' '.repeat(4)}clone\nVault2${' '.repeat( + 4, + )}${vaultsUtils.encodeVaultId(vault2Id)}${' '.repeat( + 4, + )}pull,clone\n`, ); expect(result3.stdout).not.toContain( - `Vault3\t\t${vaultsUtils.encodeVaultId(vault3Id)}`, + `Vault3${' '.repeat(4)}${vaultsUtils.encodeVaultId(vault3Id)}`, ); } finally { await remoteOnline?.stop();