Skip to content

Commit

Permalink
fix: introduced encodeEscaped utility function for opt-in encoding
Browse files Browse the repository at this point in the history 
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]
  • Loading branch information
@amydevs
amydevs committed Nov 8, 2023
1 parent 469ce59 commit 37db593
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 65 deletions.
6 changes: 4 additions & 2 deletions src/secrets/CommandGet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
154 changes: 112 additions & 42 deletions src/utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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':
Expand All @@ -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
Expand All @@ -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.
*
Expand All @@ -119,34 +195,28 @@ 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);
}
}

function outputFormatterList(items: Array<string>): 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;
}
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -415,6 +481,7 @@ function remoteErrorCause(e: any): [any, number] {
export {
verboseToLogLevel,
standardErrorReplacer,
encodeEscapedReplacer,
outputFormatter,
outputFormatterList,
outputFormatterTable,
Expand All @@ -423,9 +490,12 @@ export {
outputFormatterError,
retryAuthentication,
remoteErrorCause,
encodeWrappedStrings,
encodeQuotes,
encodeNonPrintable,
encodeEscapedWrapped,
encodeEscaped,
encodeEscapedRegex,
decodeEscapedWrapped,
decodeEscaped,
decodeEscapedRegex,
};

export type { OutputObject };
2 changes: 1 addition & 1 deletion src/vaults/CommandScan.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class CommandScan extends CommandPolykey {
const permissions = vault.permissions.join(',');
data.push(
`${vaultName}${' '.repeat(4)}${vaultIdEncoded}${' '.repeat(
5,
4,
)}${permissions}`,
);
}
Expand Down
30 changes: 16 additions & 14 deletions tests/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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',
);
},
);
Expand All @@ -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',
Expand All @@ -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({
Expand All @@ -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),
Expand Down Expand Up @@ -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
);
},
);
});
14 changes: 8 additions & 6 deletions tests/vaults/vaults.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 37db593

Please sign in to comment.