Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: parser should preserve inner quotes #407

Merged
merged 8 commits into from Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 26 additions & 10 deletions lib/yargs-parser.ts
Expand Up @@ -59,6 +59,9 @@ export class YargsParser {
// allow a string argument to be passed in rather
// than an argv array.
const args = tokenizeArgString(argsInput)
// tokenizeArgString adds extra quotes to args if argsInput is a string
// only strip those extra quotes in processValue if argsInput is a string
const shouldStripQuotes = typeof argsInput === 'string'

// aliases might have transitive relationships, normalize this.
const aliases = combineAliases(Object.assign(Object.create(null), opts.alias))
Expand Down Expand Up @@ -243,7 +246,13 @@ export class YargsParser {
// nargs format = '--f=monkey washing cat'
i = eatNargs(i, m[1], args, m[2])
} else {
setArg(m[1], m[2])
// only strip quotes if they wouldn't already be removed
if (shouldStripQuotes) {
setArg(m[1], m[2])
} else {
setArg(m[1], stripQuotes(m[2]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since setArgs calls processValue, could this be simplified to:

setArg(m[1],, m[2], shouldStripQuotes);

Copy link
Contributor Author

@jly36963 jly36963 Nov 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldStripQuotes should always be forwarded by setArg to processValue, except at this line. In this invocation, shouldStripQuotes should always be true.

I'm not a fan of the solution I have here

I try to keep code purely functional (whenever possible). In order to avoid reading global variables in setArg, I dynamically set the default value for shouldStripQuotes. I don't particularly like that pattern, so I'm down to change it.

}

}
}
} else if (arg.match(negatedBoolean) && configuration['boolean-negation']) {
Expand Down Expand Up @@ -516,15 +525,15 @@ export class YargsParser {
} else {
// value in --option=value is eaten as is
if (!isUndefined(argAfterEqualSign)) {
argsToSet.push(processValue(key, argAfterEqualSign))
argsToSet.push(processValue(key, argAfterEqualSign, true))
}
for (let ii = i + 1; ii < args.length; ii++) {
if ((!configuration['greedy-arrays'] && argsToSet.length > 0) ||
(nargsCount && typeof nargsCount === 'number' && argsToSet.length >= nargsCount)) break
next = args[ii]
if (/^-/.test(next) && !negative.test(next) && !isUnknownOptionAsArg(next)) break
i = ii
argsToSet.push(processValue(key, next))
argsToSet.push(processValue(key, next, shouldStripQuotes))
}
}

Expand All @@ -548,7 +557,7 @@ export class YargsParser {
addNewAlias(key, alias)
}

const value = processValue(key, val)
const value = processValue(key, val, shouldStripQuotes)
const splitKey = key.split('.')
setKey(argv, splitKey, value)

Expand Down Expand Up @@ -605,13 +614,10 @@ export class YargsParser {
}
}

function processValue (key: string, val: any) {
function processValue (key: string, val: any, shouldStripQuotes: boolean) {
// strings may be quoted, clean this up as we assign values.
if (typeof val === 'string' &&
(val[0] === "'" || val[0] === '"') &&
val[val.length - 1] === val[0]
) {
val = val.substring(1, val.length - 1)
if (shouldStripQuotes) {
val = stripQuotes(val)
}

// handle parsing boolean arguments --foo=true --bar false.
Expand Down Expand Up @@ -1116,3 +1122,13 @@ function sanitizeKey (key: string): string {
if (key === '__proto__') return '___proto___'
return key
}

function stripQuotes(val: string): string {
return (
typeof val === 'string' &&
(val[0] === "'" || val[0] === '"') &&
val[val.length - 1] === val[0]
)
? val = val.substring(1, val.length - 1)
: val
}
20 changes: 18 additions & 2 deletions test/yargs-parser.cjs
Expand Up @@ -3569,7 +3569,7 @@ describe('yargs-parser', function () {
args.foo.should.equal('hello world')
args.bar.should.equal('goodnight\'moon')
const args2 = parser(['--foo', '"hello world"', '--bar="goodnight\'moon"'])
args2.foo.should.equal('hello world')
args2.foo.should.equal('"hello world"')
args2.bar.should.equal('goodnight\'moon')
})

Expand All @@ -3578,7 +3578,7 @@ describe('yargs-parser', function () {
args.foo.should.equal('hello world')
args.bar.should.equal('goodnight"moon')
const args2 = parser(['--foo', "'hello world'", "--bar='goodnight\"moon'"])
args2.foo.should.equal('hello world')
args2.foo.should.equal("'hello world'")
args2.bar.should.equal('goodnight"moon')
})

Expand All @@ -3587,6 +3587,22 @@ describe('yargs-parser', function () {
args.foo.should.equal('-hello world')
args.bar.should.equal('--goodnight moon')
})

it('respects inner quotes (string)', function () {
const args = parser('cmd --foo ""Hello"" --bar ""World"" --baz="":)""')
console.log('string', {args})
args.foo.should.equal('"Hello"')
args.bar.should.equal('"World"')
args.baz.should.equal('":)"')
})

it('respects inner quotes (array)', function () {
const args = parser(['cmd', '--foo', '"Hello"', '--bar', '"World"', '--baz="":)""'])
console.log('array', {args})
args.foo.should.equal('"Hello"')
args.bar.should.equal('"World"')
args.baz.should.equal('":)"')
})
})

// see: https://github.com/yargs/yargs-parser/issues/144
Expand Down