Skip to content

Commit

Permalink
Fix issues with PR serverless#5631 causing base64 errors
Browse files Browse the repository at this point in the history
- Some of values were not converted back from base64, which resulted in
  spurious errors with base64 strings.
  (This was caused by nested arrays, like commands, not being converted
   back, combined with the wrong assumption that commands always occur
   before options. It is not always true, as the added test case
   demonstrates.)
- Some other values were converted *from* base64, but they weren't
  base64 in the first place, which resulted in junk binary data.
  (This was due to the invalid assumption that only odd-numbered values
   need to be converted.)
  • Loading branch information
Jerzy Kozera committed Nov 15, 2018
1 parent 319e3e5 commit efa6963
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 12 deletions.
35 changes: 23 additions & 12 deletions lib/classes/CLI.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,32 +34,43 @@ class CLI {
inputArray = process.argv.slice(2);
}

const toBase64Helper = (value, index) => {
if ((index % 2) !== 0) {
return new Buffer(value.toString()).toString('base64');
const base64Encode = (valueStr) =>
new Buffer(valueStr).toString('base64');

const toBase64Helper = (value) => {
const valueStr = value.toString();
if (valueStr.startsWith('-')) {
if (valueStr.indexOf('=') !== -1) {
// do not encode argument names, since those are parsed by
// minimist, and thus need to be there unconverted:
const splitted = valueStr.split('=', 2);
// splitted[1] values, however, need to be encoded, since we
// decode them later back to utf8
const encodedValue = base64Encode(splitted[1]);
return `${splitted[0]}=${encodedValue}`;
}
// do not encode plain flags, for the same reason as above
return valueStr;
}
return value;
return base64Encode(valueStr);
};

const decodedArgsHelper = (arg) => {
if (_.isString(arg)) {
return new Buffer(arg, 'base64').toString();
} else if (_.isArray(arg)) {
return _.map(arg, decodedArgsHelper);
}
return arg;
};

// get all the commands
const valuesToIgnore = _.takeWhile(inputArray, (item) => !item.startsWith('-'));
// get all the options
const valuesToConvert = _.difference(inputArray, valuesToIgnore);
// encode all the options values to base64
const valuesToB64 = _.map(valuesToConvert, toBase64Helper);
// concat commands with values on base64
const valuesToParse = _.concat(valuesToIgnore, valuesToB64);
const valuesToParse = _.map(inputArray, toBase64Helper);

// parse the options with minimist
const argvToParse = minimist(valuesToParse);

// decode all values to utf8 strings
// decode all values back to utf8 strings
const argv = _.mapValues(argvToParse, decodedArgsHelper);

const commands = [].concat(argv._);
Expand Down
23 changes: 23 additions & 0 deletions lib/classes/CLI.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,29 @@ describe('CLI', () => {
expect(inputToBeProcessed).to.deep.equal(expectedObject);
});

it('should not pass base64 values as options', () => {
cli = new CLI(serverless, ['--service=foo', 'dynamodb', 'install']);
const inputToBeProcessed = cli.processInput();

/* It used to fail with the following diff, failing to convert base64 back,
and unconverting non-base64 values into binary:
{
"commands": [
- "ZHluYW1vZGI="
+ "dynamodb"
"install"
]
"options": {
- "service": "~�"
+ "service": "foo"
}
}
*/
const expectedObject = { commands: ['dynamodb', 'install'], options: { service: 'foo' } };

expect(inputToBeProcessed).to.deep.equal(expectedObject);
});

it('should return commands and options when both are given', () => {
cli = new CLI(serverless, ['deploy', 'functions', '-f', 'function1']);
const inputToBeProcessed = cli.processInput();
Expand Down

0 comments on commit efa6963

Please sign in to comment.