Skip to content

Commit

Permalink
fix: __proto__ will now be replaced with ___proto___ in parse (#1591)
Browse files Browse the repository at this point in the history
  • Loading branch information
bcoe committed Mar 16, 2020
1 parent 2fed2a7 commit 2474c38
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 2 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -29,7 +29,7 @@
"string-width": "^4.2.0",
"which-module": "^2.0.0",
"y18n": "^4.0.0",
"yargs-parser": "^18.1.0"
"yargs-parser": "^18.1.1"
},
"devDependencies": {
"c8": "^7.0.0",
Expand Down
31 changes: 31 additions & 0 deletions test/yargs.js
Expand Up @@ -2500,4 +2500,35 @@ describe('yargs dsl tests', () => {
})
})
})

// See: https://github.com/nodejs/node/issues/31951
describe('should not pollute the prototype', () => {
it('does not pollute, when .parse() is called', function () {
yargs.parse(['-f.__proto__.foo', '99', '-x.y.__proto__.bar', '100', '--__proto__', '200'])
Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line
expect({}.foo).to.equal(undefined)
expect({}.bar).to.equal(undefined)
})

it('does not pollute, when .argv is called', function () {
yargs(['-f.__proto__.foo', '99', '-x.y.__proto__.bar', '100', '--__proto__', '200']).argv // eslint-disable-line
Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line
expect({}.foo).to.equal(undefined)
expect({}.bar).to.equal(undefined)
})

// TODO(bcoe): due to replacement of __proto__ with ___proto___ parser
// hints are not properly applied, we should move to an alternate approach
// in the future:
it('does not pollute, when options are set', function () {
yargs
.option('__proto__', {
describe: 'pollute pollute',
nargs: 33
})
.default('__proto__', { hello: 'world' })
.parse(['--foo'])
Object.keys({}.__proto__).length.should.equal(0) // eslint-disable-line
})
})
})
11 changes: 10 additions & 1 deletion yargs.js
Expand Up @@ -259,6 +259,7 @@ function Yargs (processArgs, cwd, parentRequire) {
function populateParserHintArray (type, keys, value) {
keys = [].concat(keys)
keys.forEach((key) => {
key = sanitizeKey(key)
options[type].push(key)
})
}
Expand Down Expand Up @@ -314,8 +315,8 @@ function Yargs (processArgs, cwd, parentRequire) {

function populateParserHintObject (builder, isArray, type, key, value) {
if (Array.isArray(key)) {
const temp = Object.create(null)
// an array of keys with one value ['x', 'y', 'z'], function parse () {}
const temp = {}
key.forEach((k) => {
temp[k] = value
})
Expand All @@ -326,6 +327,7 @@ function Yargs (processArgs, cwd, parentRequire) {
builder(k, key[k])
})
} else {
key = sanitizeKey(key)
// a single key value pair 'x', parse() {}
if (isArray) {
options[type][key] = (options[type][key] || []).concat(value)
Expand All @@ -335,6 +337,13 @@ function Yargs (processArgs, cwd, parentRequire) {
}
}

// TODO(bcoe): in future major versions move more objects towards
// Object.create(null):
function sanitizeKey (key) {
if (key === '__proto__') return '___proto___'
return key
}

function deleteFromParserHintObject (optionKey) {
// delete from all parsing hints:
// boolean, array, key, alias, etc.
Expand Down

0 comments on commit 2474c38

Please sign in to comment.