[issue #87] Kill vm.runInNewContext with fire

Change default condition evaluation from vm.runInNewContext to
eval-style functions.  Use of "naked" variables (pid === 123, etc) no
longer works, making this.<name> variable qualification necessary.
This commit is contained in:
Patrick Mooney 2014-07-31 19:56:03 -05:00
parent 12eda6036f
commit 042e2c0fc9
2 changed files with 105 additions and 45 deletions

View file

@ -276,9 +276,16 @@ function filterRecord(rec, opts)
return false;
}
if (opts.conditions) {
for (var i = 0; i < opts.conditions.length; i++) {
var pass = opts.conditions[i].runInNewContext(rec);
if (opts.condFuncs) {
var recCopy = objCopy(rec);
for (var i = 0; i < opts.condFuncs.length; i++) {
var pass = opts.condFuncs[i].call(recCopy);
if (!pass)
return false;
}
} else if (opts.condVm) {
for (var i = 0; i < opts.condVm.length; i++) {
var pass = opts.condVm[i].runInNewContext(rec);
if (!pass)
return false;
}
@ -348,6 +355,33 @@ function emitNextRecord(opts, stylize)
}
}
/**
* Return a function for the given JS code that returns.
*
* If no 'return' in the given javascript snippet, then assume we are a single
* statement and wrap in 'return (...)'. This is for convenience for short
* '-c ...' snippets.
*/
function funcWithReturnFromSnippet(js) {
// auto-"return"
if (js.indexOf('return') === -1) {
if (js.substring(js.length - 1) === ';') {
js = js.substring(0, js.length - 1);
}
js = 'return (' + js + ')';
}
// Expose level definitions to condition func context
var varDefs = [];
Object.keys(upperNameFromLevel).forEach(function (lvl) {
varDefs.push(format('var %s = %d;',
upperNameFromLevel[lvl], lvl));
});
varDefs = varDefs.join('\n') + '\n';
return (new Function(varDefs + js));
}
/**
* Parse the command-line options and arguments into an object.
*
@ -400,6 +434,7 @@ function parseArgv(argv) {
}
args = newArgs;
// Expose level definitions to condition vm context
var condDefines = [];
Object.keys(upperNameFromLevel).forEach(function (lvl) {
condDefines.push(
@ -495,18 +530,21 @@ function parseArgv(argv) {
case '-c':
case '--condition':
var condition = args.shift();
parsed.conditions = parsed.conditions || [];
var scriptName = 'bunyan-condition-'+parsed.conditions.length;
if (Boolean(process.env.JSON_EXEC &&
process.env.JSON_EXEC === 'vm')) {
parsed.condVm = parsed.condVm || [];
var scriptName = 'bunyan-condition-'+parsed.condVm.length;
var code = condDefines + condition;
var script;
try {
var script = vm.createScript(code, scriptName);
} catch (compileErr) {
script = vm.createScript(code, scriptName);
} catch (complErr) {
throw new Error(format('illegal CONDITION code: %s\n'
+ ' CONDITION script:\n'
+ '%s\n'
+ ' Error:\n'
+ '%s',
compileErr, indent(code), indent(compileErr.stack)));
complErr, indent(code), indent(complErr.stack)));
}
// Ensure this is a reasonably safe CONDITION.
@ -527,8 +565,11 @@ function parseArgv(argv) {
indent(condErr.stack)
));
}
parsed.conditions.push(script);
parsed.condVm.push(script);
} else {
parsed.condFuncs = parsed.condFuncs || [];
parsed.condFuncs.push(funcWithReturnFromSnippet(condition));
}
break;
default: // arguments
if (!endOfOptions && arg.length > 0 && arg[0] === '-') {

View file

@ -280,7 +280,7 @@ test('--level 40', function (t) {
});
});
test('--condition "level === 10 && pid === 123"', function (t) {
test('--condition "this.level === 10 && this.pid === 123"', function (t) {
var expect = [
'# levels\n',
/* JSSTYLED */
@ -292,14 +292,14 @@ test('--condition "level === 10 && pid === 123"', function (t) {
'not a JSON line\n',
'{"hi": "there"}\n'
].join('');
var cmd = _('%s -c "level === 10 && pid === 123" %s/corpus/all.log',
BUNYAN, __dirname);
var cmd = _('%s -c "this.level === 10 && this.pid === 123"'
+ ' %s/corpus/all.log', BUNYAN, __dirname);
exec(cmd, function (err, stdout, stderr) {
t.ifError(err);
t.equal(stdout, expect);
var cmd = _(
'%s --condition "level === 10 && pid === 123" %s/corpus/all.log',
BUNYAN, __dirname);
'%s --condition "this.level === 10 && this.pid === 123"'
+ ' %s/corpus/all.log', BUNYAN, __dirname);
exec(cmd, function (err, stdout, stderr) {
t.ifError(err);
t.equal(stdout, expect);
@ -308,13 +308,34 @@ test('--condition "level === 10 && pid === 123"', function (t) {
});
});
test('--condition "this.level === TRACE', function (t) {
var expect = [
'# levels\n',
/* JSSTYLED */
'[2012-02-08T22:56:50.856Z] TRACE: myservice/123 on example.com: My message\n',
'\n',
'# extra fields\n',
'\n',
'# bogus\n',
'not a JSON line\n',
'{"hi": "there"}\n'
].join('');
var cmd = _('%s -c "this.level === TRACE" %s/corpus/all.log',
BUNYAN, __dirname);
exec(cmd, function (err, stdout, stderr) {
t.ifError(err);
t.equal(stdout, expect);
t.done();
});
});
// multiple
// not sure if this is a bug or a feature. let's call it a feature!
test('multiple --conditions', function (t) {
var expect = [
'# levels\n',
/* JSSTYLED */
'[2012-02-08T22:56:53.856Z] WARN: myservice/1 on example.com: My message\n',
'[2012-02-08T22:56:53.856Z] WARN: myservice/123 on example.com: My message\n',
'\n',
'# extra fields\n',
'\n',
@ -322,10 +343,8 @@ test('multiple --conditions', function (t) {
'not a JSON line\n',
'{"hi": "there"}\n'
].join('');
exec(_('%s %s/corpus/all.log ' +
'-c "if (level === 40) pid = 1; true" ' +
'-c "pid === 1"', BUNYAN, __dirname),
function (err, stdout, stderr) {
exec(_('%s %s/corpus/all.log -c "this.level === 40" -c "this.pid === 123"',
BUNYAN, __dirname), function (err, stdout, stderr) {
t.ifError(err);
t.equal(stdout, expect);
t.end();