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.

Sign up for GitHub

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

Jump to bottom

Refactor and test custom marshalling logic #15113

Merged
merged 5 commits into from
May 17, 2017
Merged

Refactor and test custom marshalling logic #15113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
13f4acb
Add customMarshalObjectList to PatchedInterpreter instance
pcardune May 11, 2017
e7f0c1b
Rename PatchedInterpreter to CustomMarshalingInterpreter
pcardune May 11, 2017
a02c7f4
Break out custom marshaling into separate helper object
pcardune May 12, 2017
05230a9
Start writing tests for CustomMarshalingInterpreter functionality
pcardune May 15, 2017
195acf7
More tests and refactoring to increase overall coverage and document …
pcardune May 16, 2017
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
264 changes: 53 additions & 211 deletions apps/src/JSInterpreter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ var codegen = require('./codegen');
var ObservableEventDEPRECATED = require('./ObservableEventDEPRECATED');
var utils = require('./utils');
var acorn = require('@code-dot-org/js-interpreter/acorn');
import PatchedInterpreter from './lib/tools/jsinterpreter/PatchedInterpreter';
import CustomMarshalingInterpreter from './lib/tools/jsinterpreter/CustomMarshalingInterpreter';
import CustomMarshaler from './lib/tools/jsinterpreter/CustomMarshaler';
import {getStore} from './redux';

import { setIsDebuggerPaused } from './redux/runState';
Expand Down Expand Up @@ -35,9 +36,11 @@ export default class JSInterpreter {
this.studioApp = studioApp;
this.shouldRunAtMaxSpeed = shouldRunAtMaxSpeed || function () { return true; };
this.maxInterpreterStepsPerTick = maxInterpreterStepsPerTick || 10000;
this.customMarshalGlobalProperties = customMarshalGlobalProperties || {};
this.customMarshalBlockedProperties = customMarshalBlockedProperties || [];
this.customMarshalObjectList = customMarshalObjectList || [];
this.customMarshaler = new CustomMarshaler({
globalProperties: customMarshalGlobalProperties,
blockedProperties: customMarshalBlockedProperties,
objectList: customMarshalObjectList,
});

// Publicly-exposed events that anyone with access to the JSInterpreter can
// observe and respond to.
Expand Down Expand Up @@ -75,14 +78,10 @@ export default class JSInterpreter {

addCustomMarshalObject(config) {
// TODO (pcardune): validate config format.
this.customMarshalObjectList.push(config);
this.customMarshaler.objectList.push(config);
}
}

JSInterpreter.baseHasProperty = PatchedInterpreter.prototype.hasProperty;
JSInterpreter.baseGetProperty = PatchedInterpreter.prototype.getProperty;
JSInterpreter.baseSetProperty = PatchedInterpreter.prototype.setProperty;

/**
* Initialize the JSInterpreter, parsing the provided code and preparing to
* execute it one step at a time.
Expand Down Expand Up @@ -149,47 +148,46 @@ JSInterpreter.prototype.parse = function (options) {
// Return value will be stored as this.interpreter inside the supplied
// initFunc() (other code in initFunc() depends on this.interpreter, so
// we can't wait until the constructor returns)
new PatchedInterpreter('', (interpreter, scope) => {
// Store Interpreter on JSInterpreter
this.interpreter = interpreter;
// Store globalScope on JSInterpreter
this.globalScope = scope;
codegen.customMarshalObjectList_DEPRECATED = this.customMarshalObjectList;
// Override Interpreter's get/has/set Property functions with JSInterpreter
interpreter.getProperty = this.getProperty.bind(this, interpreter);
interpreter.hasProperty = this.hasProperty.bind(this, interpreter);
interpreter.setProperty = this.setProperty.bind(this, interpreter);
codegen.initJSInterpreter(
interpreter,
options.blocks,
options.blockFilter,
scope,
options.globalFunctions);

if (options.initGlobals) {
options.initGlobals();
}
new CustomMarshalingInterpreter(
'',
this.customMarshaler,
(interpreter, scope) => {
// Store Interpreter on JSInterpreter
this.interpreter = interpreter;
// Store globalScope on JSInterpreter
this.globalScope = scope;
codegen.initJSInterpreter(
interpreter,
options.blocks,
options.blockFilter,
scope,
options.globalFunctions);

if (options.initGlobals) {
options.initGlobals();
}

// Only allow five levels of depth when marshalling the return value
// since we will occasionally return DOM Event objects which contain
// properties that recurse over and over...
var wrapper = codegen.makeNativeMemberFunction({
interpreter: interpreter,
nativeFunc: this.nativeGetCallback.bind(this),
maxDepth: 5
});
interpreter.setProperty(scope,
'getCallback',
interpreter.createNativeFunction(wrapper));

wrapper = codegen.makeNativeMemberFunction({
interpreter: interpreter,
nativeFunc: this.nativeSetCallbackRetVal.bind(this),
});
interpreter.setProperty(scope,
'setCallbackRetVal',
interpreter.createNativeFunction(wrapper));
});
// Only allow five levels of depth when marshalling the return value
// since we will occasionally return DOM Event objects which contain
// properties that recurse over and over...
var wrapper = codegen.makeNativeMemberFunction({
interpreter: interpreter,
nativeFunc: this.nativeGetCallback.bind(this),
maxDepth: 5
});
interpreter.setProperty(scope,
'getCallback',
interpreter.createNativeFunction(wrapper));

wrapper = codegen.makeNativeMemberFunction({
interpreter: interpreter,
nativeFunc: this.nativeSetCallbackRetVal.bind(this),
});
interpreter.setProperty(scope,
'setCallbackRetVal',
interpreter.createNativeFunction(wrapper));
}
);
// We initialize with an empty program so that all of our global functions
// can be injected before the user code is processed (thus allowing user
// code to override globals of the same names)
Expand Down Expand Up @@ -764,151 +762,6 @@ JSInterpreter.prototype.createPrimitive = function (data) {
}
};

/**
* Helper to determine if we should prevent custom marshalling from occurring
* in a situation where we normally would use it. Allows us to block from a
* specific list of properties and a hardcoded list of instance types that are
* not safe to return into the interpreter sandbox.
*
* @param {string} name Name of property.
* @param {!Object} obj Data object.
* @param {Object} nativeParent Native parent object (if parented).
* @return {boolean} true if property access should be blocked.
*/
JSInterpreter.prototype.shouldBlockCustomMarshalling_ = function (name, obj,
nativeParent) {
if (-1 !== this.customMarshalBlockedProperties.indexOf(name)) {
return true;
}
var value = obj.isCustomMarshal ? obj.data[name] : nativeParent[name];
if (value instanceof Node || value instanceof Window) {
return true;
}
return false;
};

/**
* Wrapper to Interpreter's getProperty (extended for custom marshaling)
*
* Fetch a property value from a data object.
* @param {!Object} interpeter Interpreter instance.
* @param {!Object} obj Data object.
* @param {*} name Name of property.
* @return {!Object} Property value (may be UNDEFINED).
*/
JSInterpreter.prototype.getProperty = function (
interpreter,
obj,
name) {
name = name.toString();
var nativeParent;
var customMarshalValue;
if (obj.isCustomMarshal) {
if (this.shouldBlockCustomMarshalling_(name, obj)) {
return interpreter.UNDEFINED;
} else {
customMarshalValue = obj.data[name];
}
} else {
var hasProperty = false;
if (!obj.isPrimitive) {
hasProperty = JSInterpreter.baseHasProperty.call(interpreter, obj, name);
}
if (!hasProperty &&
obj === this.globalScope &&
!!(nativeParent = this.customMarshalGlobalProperties[name]) &&
!this.shouldBlockCustomMarshalling_(name, obj, nativeParent)) {
customMarshalValue = nativeParent[name];
} else {
return JSInterpreter.baseGetProperty.call(interpreter, obj, name);
}
}
var type = typeof customMarshalValue;
if (type === 'number' || type === 'boolean' || type === 'string' ||
type === 'undefined' || customMarshalValue === null) {
return interpreter.createPrimitive(customMarshalValue);
} else {
return codegen.marshalNativeToInterpreter(interpreter,
customMarshalValue,
obj.data);
}
};

/**
* Wrapper to Interpreter's hasProperty (extended for custom marshaling)
*
* Does the named property exist on a data object.
* @param {!Object} interpeter Interpreter instance.
* @param {!Object} obj Data object.
* @param {*} name Name of property.
* @return {boolean} True if property exists.
*/
JSInterpreter.prototype.hasProperty = function (
interpreter,
obj,
name) {
name = name.toString();
var nativeParent;
if (obj.isCustomMarshal) {
if (this.shouldBlockCustomMarshalling_(name, obj)) {
return false;
} else {
return name in obj.data;
}
} else {
var hasProperty = JSInterpreter.baseHasProperty.call(interpreter, obj, name);
if (!hasProperty &&
obj === this.globalScope &&
!!(nativeParent = this.customMarshalGlobalProperties[name]) &&
!this.shouldBlockCustomMarshalling_(name, obj, nativeParent)) {
return true;
} else {
return hasProperty;
}
}
};

/**
* Wrapper to Interpreter's setProperty (extended for custom marshaling)
*
* Set a property value on a data object.
* @param {!Object} interpeter Interpreter instance.
* @param {!Object} obj Data object.
* @param {*} name Name of property.
* @param {*} value New property value.
* @param {boolean} opt_fixed Unchangeable property if true.
* @param {boolean} opt_nonenum Non-enumerable property if true.
*/
JSInterpreter.prototype.setProperty = function (
interpreter,
obj,
name,
value,
opt_fixed,
opt_nonenum) {
name = name.toString();
var nativeParent;
if (obj.isCustomMarshal) {
if (!this.shouldBlockCustomMarshalling_(name, obj)) {
obj.data[name] = codegen.marshalInterpreterToNative(interpreter, value);
}
} else {
var hasProperty = false;
if (!obj.isPrimitive) {
hasProperty = JSInterpreter.baseHasProperty.call(interpreter, obj, name);
}
if (!hasProperty &&
obj === this.globalScope &&
!!(nativeParent = this.customMarshalGlobalProperties[name]) &&
!this.shouldBlockCustomMarshalling_(name, obj, nativeParent)) {
nativeParent[name] = codegen.marshalInterpreterToNative(interpreter, value);
} else {
return JSInterpreter.baseSetProperty.call(
interpreter, obj, name, value, opt_fixed, opt_nonenum);
}
}
};

/**
* Selects code in droplet/ace editor.
*
Expand Down Expand Up @@ -1014,11 +867,11 @@ JSInterpreter.prototype.createGlobalProperty = function (name, value, parent) {

// Bypass setProperty since we've hooked it and it will not create the
// property if it is in customMarshalGlobalProperties
JSInterpreter.baseSetProperty.call(
this.interpreter,
this.globalScope,
name,
interpreterVal);
this.interpreter.setPropertyWithoutCustomMarshaling(
this.globalScope,
name,
interpreterVal
);
};

/**
Expand Down Expand Up @@ -1158,7 +1011,7 @@ JSInterpreter.prototype.getCurrentState = function () {
*/
JSInterpreter.prototype.evalInCurrentScope = function (expression) {
var currentScope = this.interpreter.getScope();
var evalInterpreter = new PatchedInterpreter(expression);
var evalInterpreter = new CustomMarshalingInterpreter(expression, this.customMarshaler);
// Set scope to the current scope of the running program
// NOTE: we are being a little tricky here (we are re-running
// part of the Interpreter constructor with a different interpreter's
Expand All @@ -1176,17 +1029,6 @@ JSInterpreter.prototype.evalInCurrentScope = function (expression) {
evalInterpreter[prop] = this.interpreter[prop];
}, this);

// Patch getProperty, hasProperty, and setProperty to enable custom marshalling
evalInterpreter.getProperty = this.getProperty.bind(
this,
evalInterpreter);
evalInterpreter.hasProperty = this.hasProperty.bind(
this,
evalInterpreter);
evalInterpreter.setProperty = this.setProperty.bind(
this,
evalInterpreter);

// run() may throw if there's a problem in the expression
evalInterpreter.run();
return evalInterpreter.value;
Expand Down