From 3dfb490370ef48669378977c3c6c1f6230387c14 Mon Sep 17 00:00:00 2001 From: Chen Yangjian <252317+cyjake@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:29:01 +0800 Subject: [PATCH] fix: reserve quotation marks when handling multiple values (#85) fixes #84 --- src/ssh-config.ts | 53 +++++++----- test/helpers.ts | 9 +++ test/unit/parse.test.ts | 12 ++- test/unit/ssh-config.test.ts | 153 ++++++++++++++++++++++++++--------- test/unit/stringify.test.ts | 28 ++++++- 5 files changed, 191 insertions(+), 64 deletions(-) create mode 100644 test/helpers.ts diff --git a/src/ssh-config.ts b/src/ssh-config.ts index e98bfb3..5f2f707 100644 --- a/src/ssh-config.ts +++ b/src/ssh-config.ts @@ -25,7 +25,7 @@ export interface Directive { after: string; param: string; separator: Separator; - value: string | string[]; + value: string | { val: string, separator: string, quoted?: boolean }[]; quoted?: boolean; } @@ -34,7 +34,7 @@ export interface Section extends Directive { } export interface Match extends Section { - criteria: Record + criteria: Record } export interface Comment { @@ -125,8 +125,9 @@ function match(criteria: Match['criteria'], context: ComputeContext): boolean { for (const key in criteria) { const criterion = criteria[key] + const values = Array.isArray(criterion) ? criterion.map(({ val }) => val) : criterion - if (!testCriterion(key, criterion)) { + if (!testCriterion(key, values)) { return false } } @@ -207,7 +208,7 @@ export default class SSHConfig extends Array { const doPass = () => { for (const line of this) { if (line.type !== LineType.DIRECTIVE) continue - if (line.param === 'Host' && glob(line.value, context.params.Host)) { + if (line.param === 'Host' && glob(Array.isArray(line.value) ? line.value.map(({ val }) => val) : line.value, context.params.Host)) { let canonicalizeHostName = false let canonicalDomains: string[] = [] setProperty(line.param, line.value) @@ -218,7 +219,7 @@ export default class SSHConfig extends Array { canonicalizeHostName = true } if (/^CanonicalDomains$/i.test(subline.param) && Array.isArray(subline.value)) { - canonicalDomains = subline.value + canonicalDomains = subline.value.map(({ val }) => val) } } } @@ -333,7 +334,7 @@ export default class SSHConfig extends Array { type: LineType.DIRECTIVE, param, separator: ' ', - value, + value: Array.isArray(value) ? value.map((val, i) => ({ val, separator: i === 0 ? '' : ' ' })) : value, before: sectionLineFound ? indent : indent.replace(/ |\t/, ''), after: '\n', } @@ -386,7 +387,7 @@ export default class SSHConfig extends Array { type: LineType.DIRECTIVE, param, separator: ' ', - value, + value: Array.isArray(value) ? value.map((val, i) => ({ val, separator: i === 0 ? '' : ' ' })) : value, before: '', after: '\n', } @@ -530,8 +531,13 @@ export function parse(text: string): SSHConfig { // Host * !local.dev // Host "foo bar" function values() { - const results: string[] = [] + const results: { val: string, separator: string, quoted: boolean }[] = [] let val = '' + // whether current value is quoted or not + let valQuoted = false + // the separator preceding current value + let valSeparator = ' ' + // whether current context is within quotations or not let quoted = false let escaped = false @@ -548,11 +554,14 @@ export function parse(text: string): SSHConfig { } else if (quoted) { val += chr + valQuoted = true } else if (/[ \t=]/.test(chr)) { if (val) { - results.push(val) + results.push({ val, separator: valSeparator, quoted: valQuoted }) val = '' + valQuoted = false + valSeparator = chr } // otherwise ignore the space } @@ -567,10 +576,10 @@ export function parse(text: string): SSHConfig { } if (quoted || escaped) { - throw new Error(`Unexpected line break at ${results.concat(val).join(' ')}`) + throw new Error(`Unexpected line break at ${results.map(({ val }) => val).concat(val).join(' ')}`) } - if (val) results.push(val) - return results.length > 1 ? results : results[0] + if (val) results.push({ val, separator: valSeparator, quoted: valQuoted }) + return results.length > 1 ? results : results[0].val } function directive() { @@ -592,12 +601,12 @@ export function parse(text: string): SSHConfig { const criteria: Match['criteria'] = {} if (typeof result.value === 'string') { - result.value = [result.value] + result.value = [{ val: result.value, separator: '', quoted: result.quoted }] } let i = 0 while (i < result.value.length) { - const keyword = result.value[i] + const { val: keyword } = result.value[i] switch (keyword.toLowerCase()) { case 'all': @@ -610,7 +619,7 @@ export function parse(text: string): SSHConfig { if (i + 1 >= result.value.length) { throw new Error(`Missing value for match criteria ${keyword}`) } - criteria[keyword] = result.value[i + 1] + criteria[keyword] = result.value[i + 1].val i += 2 break } @@ -663,7 +672,11 @@ export function stringify(config: SSHConfig): string { function formatValue(value: string | string[] | Record, quoted: boolean) { if (Array.isArray(value)) { - return value.map(chunk => formatValue(chunk, RE_SPACE.test(chunk))).join(' ') + let result = '' + for (const { val, separator, quoted } of value) { + result += (result ? separator : '') + formatValue(val, quoted || RE_SPACE.test(val)) + } + return result } return quoted ? `"${value}"` : value } @@ -675,15 +688,15 @@ export function stringify(config: SSHConfig): string { return `${line.param}${line.separator}${value}` } - const format = line => { + const format = (line: Line) => { str += line.before if (line.type === LineType.COMMENT) { str += line.content } else if (line.type === LineType.DIRECTIVE && MULTIPLE_VALUE_PROPS.includes(line.param)) { - [].concat(line.value).forEach(function (value, i, values) { - str += formatDirective({ ...line, value }) + (Array.isArray(line.value) ? line.value : [line.value]).forEach((value, i, values) => { + str += formatDirective({ ...line, value: typeof value !== 'string' ? value.val : value }) if (i < values.length - 1) str += `\n${line.before}` }) } @@ -693,7 +706,7 @@ export function stringify(config: SSHConfig): string { str += line.after - if (line.config) { + if ('config' in line) { line.config.forEach(format) } } diff --git a/test/helpers.ts b/test/helpers.ts new file mode 100644 index 0000000..82dbb0b --- /dev/null +++ b/test/helpers.ts @@ -0,0 +1,9 @@ +const stripPattern = /^[ \t]*(?=[^\s]+)/mg + +export function heredoc(text: string) { + const indentLen = text.match(stripPattern)!.reduce((min, line) => Math.min(min, line.length), Infinity) + const indent = new RegExp('^[ \\t]{' + indentLen + '}', 'mg') + return indentLen > 0 + ? text.replace(indent, '').trimStart().replace(/ +?$/, '') + : text +} diff --git a/test/unit/parse.test.ts b/test/unit/parse.test.ts index c1cbfbe..c919444 100644 --- a/test/unit/parse.test.ts +++ b/test/unit/parse.test.ts @@ -145,7 +145,8 @@ describe('parse', function() { const config = parse('ProxyCommand ssh -W "%h:%p" firewall.example.org') assert.equal(config[0].type, DIRECTIVE) assert.equal(config[0].param, 'ProxyCommand') - assert.deepEqual(config[0].value, ['ssh', '-W', '%h:%p', 'firewall.example.org']) + assert.ok(Array.isArray(config[0].value)) + assert.deepEqual(config[0].value.map(({ val }) => val), ['ssh', '-W', '%h:%p', 'firewall.example.org']) }) // https://github.com/microsoft/vscode-remote-release/issues/5562 @@ -159,7 +160,8 @@ describe('parse', function() { assert.ok('config' in config[0]) assert.equal(config[0].config[0].type, DIRECTIVE) assert.equal(config[0].config[0].param, 'ProxyCommand') - assert.deepEqual(config[0].config[0].value, ['C:\\foo bar\\baz.exe', 'arg', 'arg', 'arg']) + assert.ok(Array.isArray(config[0].config[0].value)) + assert.deepEqual(config[0].config[0].value.map(({ val }) => val), ['C:\\foo bar\\baz.exe', 'arg', 'arg', 'arg']) }) it('.parse open ended values', function() { @@ -180,7 +182,8 @@ describe('parse', function() { assert.equal(config[0].type, DIRECTIVE) assert.equal(config[0].param, 'Host') - assert.deepEqual(config[0].value, [ + assert.ok(Array.isArray(config[0].value)) + assert.deepEqual(config[0].value.map(({ val }) => val), [ 'foo', '!*.bar', 'baz ham', @@ -192,7 +195,8 @@ describe('parse', function() { const config = parse('Host me local wi*ldcard? thisVM "two words"') assert.equal(config[0].type, DIRECTIVE) - assert.deepEqual(config[0].value, [ + assert.ok(Array.isArray(config[0].value)) + assert.deepEqual(config[0].value.map(({ val }) => val), [ 'me', 'local', 'wi*ldcard?', diff --git a/test/unit/ssh-config.test.ts b/test/unit/ssh-config.test.ts index 73e8bc8..91a3575 100644 --- a/test/unit/ssh-config.test.ts +++ b/test/unit/ssh-config.test.ts @@ -4,6 +4,7 @@ import path from 'path' import SSHConfig from '../..' import sinon from 'sinon' import os from 'os' +import { heredoc } from '../helpers' const { DIRECTIVE } = SSHConfig @@ -38,7 +39,33 @@ describe('SSHConfig', function() { IdentityFile: [ '~/.ssh/id_rsa' ], - ProxyCommand: ['ssh', '-q', 'gateway', '-W', '%h:%p'], + ProxyCommand: [ + { + 'quoted': false, + 'separator': ' ', + 'val': 'ssh', + }, + { + 'quoted': false, + 'separator': ' ', + 'val': '-q', + }, + { + 'quoted': false, + 'separator': ' ', + 'val': 'gateway', + }, + { + 'quoted': false, + 'separator': ' ', + 'val': '-W', + }, + { + 'quoted': false, + 'separator': ' ', + 'val': '%h:%p', + }, + ], ServerAliveInterval: '80', User: 'nil', ForwardAgent: 'true' @@ -68,7 +95,23 @@ describe('SSHConfig', function() { for (const host of ['foo', 'foo.bar', 'baz ham']) { assert.deepEqual(config.compute(host), { - Host: ['foo', '*.bar', 'baz ham'], + Host: [ + { + 'quoted': false, + 'separator': ' ', + 'val': 'foo', + }, + { + 'quoted': true, + 'separator': ' ', + 'val': '*.bar', + }, + { + 'quoted': true, + 'separator': ' ', + 'val': 'baz ham', + }, + ], HostName: 'example.com', User: 'robb' }) @@ -127,6 +170,17 @@ describe('SSHConfig', function() { assert.equal(result.ServerAliveCountMax, '2') }) + it('.compute By Host with multiple values', async () => { + const config = SSHConfig.parse(` + Host example.com example.me + HostName example-inc.com + `) + + const result = config.compute('example.me') + assert.ok(result) + assert.equal(result.HostName, 'example-inc.com') + }) + it('.compute by Match host', async function() { const config = SSHConfig.parse(` Match host tahoe1 @@ -427,11 +481,12 @@ describe('SSHConfig', function() { HostName: 'example.com' }) - assert.equal(config.toString(), `IdentityFile ~/.ssh/id_rsa + assert.equal(config.toString(), heredoc(` + IdentityFile ~/.ssh/id_rsa -Host test2 - HostName example.com -`) + Host test2 + HostName example.com + `)) }) it('.append to empty config with new section', function() { @@ -441,9 +496,10 @@ Host test2 HostName: 'example.com', }) - assert.equal(config.toString(), `Host test - HostName example.com -`) + assert.equal(config.toString(), heredoc(` + Host test + HostName example.com + `)) }) it('.append to empty section config', function() { @@ -452,9 +508,10 @@ Host test2 HostName: 'example.com' }) - assert.equal(config.toString(), `Host test - HostName example.com -`) + assert.equal(config.toString(), heredoc(` + Host test + HostName example.com + `)) }) it('.compute with properties with multiple values', async function() { @@ -581,12 +638,13 @@ Host test2 HostName: 'example.com' }) - assert.equal(config.toString(), `IdentityFile ~/.ssh/id_rsa + assert.equal(config.toString().trim(), heredoc(` + IdentityFile ~/.ssh/id_rsa -Host prependTest - HostName example.com + Host prependTest + HostName example.com -`) + `).trim()) }) it('.prepend to empty config with new section', function() { @@ -596,45 +654,50 @@ Host prependTest HostName: 'example.com', }) - assert.equal(config.toString(), `Host prependTest - HostName example.com + assert.equal(config.toString(), heredoc(` + Host prependTest + HostName example.com -`) + `)) }) it('.prepend to empty section config', function() { - const config = SSHConfig.parse('Host test') + const config = SSHConfig.parse('Host test\n') config.prepend({ HostName: 'example.com', User: 'brian' }) - assert.equal(config.toString(), `HostName example.com -User brian + assert.equal(config.toString(), heredoc(` + HostName example.com + User brian -Host test`) + Host test + `)) }) it('.prepend to empty section and existing section config', function() { - const config = SSHConfig.parse(` -Host test + const config = SSHConfig.parse(heredoc(` + Host test -Host test2 - HostName google.com`) + Host test2 + HostName google.com + `)) config.prepend({ HostName: 'example.com', User: 'brian' }) - assert.equal(config.toString(), `HostName example.com -User brian - + assert.equal(config.toString(), heredoc(` + HostName example.com + User brian -Host test + Host test -Host test2 - HostName google.com`) + Host test2 + HostName google.com + `)) }) it('.prepend to with Include', function() { @@ -667,10 +730,26 @@ Host test2 HostName: 'microsoft.com', }, true) - assert.equal(config.toString(), `Include ~/.ssh/configs/* + assert.equal(config.toString(), heredoc(` + Include ~/.ssh/configs/* -Host example - HostName microsoft.com -`) + Host example + HostName microsoft.com + `)) + }) + + it('.prepend directive with multiple values', function() { + const config = new SSHConfig() + + config.prepend({ + Host: ['example.com', 'example.me'], + HostName: 'example-inc.dev', + }) + + assert.equal(config.toString(), heredoc(` + Host example.com example.me + HostName example-inc.dev + + `)) }) }) diff --git a/test/unit/stringify.test.ts b/test/unit/stringify.test.ts index f54b205..57f3d67 100644 --- a/test/unit/stringify.test.ts +++ b/test/unit/stringify.test.ts @@ -87,7 +87,7 @@ describe('stringify', function() { `) assert.equal(stringify(config), ` - Host foo bar baz "egg ham" + Host foo bar "baz" "egg ham" HostName example.com `) }) @@ -160,12 +160,12 @@ describe('stringify', function() { it('.stringify ProxyCommand with spaces', function() { const config = parse(` Host foo - ProxyCommand "C:\foo bar\baz.exe" "arg" "arg" "arg" + ProxyCommand "C:\\foo bar\\baz.exe" "arg" "arg" "arg" `) assert.equal(stringify(config), ` Host foo - ProxyCommand "C:\foo bar\baz.exe" arg arg arg + ProxyCommand "C:\\foo bar\\baz.exe" "arg" "arg" "arg" `) }) @@ -179,4 +179,26 @@ describe('stringify', function() { HostName localhost `) }) + + // https://github.com/cyjake/ssh-config/issues/84 + it('.stringify ProxyCommand with =', function() { + const config = parse(` + Host YYYY + HostName YYYY + IdentityFile ~/.ssh/id_rsa + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + ProxyCommand ssh -i ~/.ssh/id_rsa -W %h:%p -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null XXX@ZZZ + User XXX + `) + assert.equal(stringify(config), ` + Host YYYY + HostName YYYY + IdentityFile ~/.ssh/id_rsa + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + ProxyCommand ssh -i ~/.ssh/id_rsa -W %h:%p -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null XXX@ZZZ + User XXX + `) + }) })