Bug 1392602 - Stop freezing everything in DevTools loader. r=jdescottes draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 28 Aug 2017 11:43:43 +0200
changeset 658605 bf44d123cbb341349513c59a7150ea429d456335
parent 658604 919a92469821f45a76043e537956e8b82828c248
child 658606 ad13cf51f448aa6a3db02152e483a4840e28c45d
push id77824
push userbmo:poirot.alex@gmail.com
push dateMon, 04 Sep 2017 12:19:59 +0000
reviewersjdescottes
bugs1392602
milestone57.0a1
Bug 1392602 - Stop freezing everything in DevTools loader. r=jdescottes MozReview-Commit-ID: ANdgiYjfV6N
devtools/shared/base-loader.js
--- a/devtools/shared/base-loader.js
+++ b/devtools/shared/base-loader.js
@@ -40,73 +40,37 @@ const COMPONENT_ERROR = '`Components` is
   'Shortcuts: \n' +
   '    Cc = Components' + '.classes \n' +
   '    Ci = Components' + '.interfaces \n' +
   '    Cu = Components' + '.utils \n' +
   '    CC = Components' + '.Constructor \n' +
   'Example: \n' +
   '    let { Cc, Ci } = require(\'chrome\');\n';
 
-// Workaround for bug 674195. Freezing objects from other compartments fail,
-// so we use `Object.freeze` from the same component instead.
-function freeze(object) {
-  if (prototypeOf(object) === null) {
-      Object.freeze(object);
-  }
-  else {
-    prototypeOf(prototypeOf(object.isPrototypeOf)).
-      constructor. // `Object` from the owner compartment.
-      freeze(object);
-  }
-  return object;
-}
-
 // Returns map of given `object`-s own property descriptors.
-const descriptor = iced(function descriptor(object) {
+const descriptor = function descriptor(object) {
   let value = {};
   for (let name of getOwnIdentifiers(object))
     value[name] = getOwnPropertyDescriptor(object, name)
   return value;
-});
-
-// Freeze important built-ins so they can't be used by untrusted code as a
-// message passing channel.
-freeze(Object);
-freeze(Object.prototype);
-freeze(Function);
-freeze(Function.prototype);
-freeze(Array);
-freeze(Array.prototype);
-freeze(String);
-freeze(String.prototype);
-
-// This function takes `f` function sets it's `prototype` to undefined and
-// freezes it. We need to do this kind of deep freeze with all the exposed
-// functions so that untrusted code won't be able to use them a message
-// passing channel.
-function iced(f) {
-  if (!Object.isFrozen(f)) {
-    f.prototype = undefined;
-  }
-  return freeze(f);
-}
+};
 
 // Defines own properties of given `properties` object on the given
 // target object overriding any existing property with a conflicting name.
 // Returns `target` object. Note we only export this function because it's
 // useful during loader bootstrap when other util modules can't be used &
 // thats only case where this export should be used.
-const override = iced(function override(target, source) {
+const override = function override(target, source) {
   let properties = descriptor(target);
 
   for (let name of getOwnIdentifiers(source || {}))
     properties[name] = getOwnPropertyDescriptor(source, name);
 
   return Object.defineProperties({}, properties);
-});
+};
 
 function sourceURI(uri) { return String(uri).split(" -> ").pop(); }
 
 function isntLoaderFrame(frame) { return frame.fileName !== __URI__ }
 
 function parseURI(uri) { return String(uri).split(" -> ").pop(); }
 
 function parseStack(stack) {
@@ -187,17 +151,17 @@ function join(base, ...paths) {
 //    wants X-ray vision with respect to objects inside the sandbox. Defaults
 //    to `true`.
 // - `sandbox`: A sandbox to share JS compartment with. If omitted new
 //    compartment will be created.
 // - `metadata`: A metadata object associated with the sandbox. It should
 //    be JSON-serializable.
 // For more details see:
 // https://developer.mozilla.org/en/Components.utils.Sandbox
-const Sandbox = iced(function Sandbox(options) {
+const Sandbox = function Sandbox(options) {
   // Normalize options and rename to match `Cu.Sandbox` expectations.
   options = {
     // Do not expose `Components` if you really need them (bad idea!) you
     // still can expose via prototype.
     wantComponents: false,
     sandboxName: options.name,
     principal: 'principal' in options ? options.principal : systemPrincipal,
     wantXrays: 'wantXrays' in options ? options.wantXrays : true,
@@ -220,40 +184,40 @@ const Sandbox = iced(function Sandbox(op
   // ones from it's prototype. We override delete such `sandbox` properties
   // to avoid shadowing.
   delete sandbox.Iterator;
   delete sandbox.Components;
   delete sandbox.importFunction;
   delete sandbox.debug;
 
   return sandbox;
-});
+}
 
 // Evaluates code from the given `uri` into given `sandbox`. If
 // `options.source` is passed, then that code is evaluated instead.
 // Optionally following options may be given:
 // - `options.encoding`: Source encoding, defaults to 'UTF-8'.
 // - `options.line`: Line number to start count from for stack traces.
 //    Defaults to 1.
 // - `options.version`: Version of JS used, defaults to '1.8'.
-const evaluate = iced(function evaluate(sandbox, uri, options) {
+const evaluate = function evaluate(sandbox, uri, options) {
   let { source, line, version, encoding } = override({
     encoding: 'UTF-8',
     line: 1,
     version: '1.8',
     source: null
   }, options);
 
   return source ? Cu.evalInSandbox(source, sandbox, version, uri, line)
                 : loadSubScript(uri, sandbox, encoding);
-});
+};
 
 // Populates `exports` of the given CommonJS `module` object, in the context
 // of the given `loader` by evaluating code associated with it.
-const load = iced(function load(loader, module) {
+const load = function load(loader, module) {
   let { sandboxes, globals, loadModuleHook } = loader;
   let require = Require(loader, module);
 
   // We expose set of properties defined by `CommonJS` specification via
   // prototype of the sandbox. Also globals are deeper in the prototype
   // chain so that each module has access to them as well.
   let descriptors = descriptor({
     require: require,
@@ -359,30 +323,30 @@ const load = iced(function load(loader, 
 
   // Only freeze the exports object if we created it ourselves. Modules
   // which completely replace the exports object and still want it
   // frozen need to freeze it themselves.
   if (module.exports === originalExports)
     Object.freeze(module.exports);
 
   return module;
-});
+}
 
 // Utility function to normalize module `uri`s so they have `.js` extension.
 function normalizeExt(uri) {
   return isJSURI(uri) ? uri :
          isJSONURI(uri) ? uri :
          isJSMURI(uri) ? uri :
          uri + '.js';
 }
 
 // Utility function to join paths. In common case `base` is a
 // `requirer.uri` but in some cases it may be `baseURI`. In order to
 // avoid complexity we require `baseURI` with a trailing `/`.
-const resolve = iced(function resolve(id, base) {
+const resolve = function resolve(id, base) {
   if (!isRelative(id))
     return id;
 
   let baseDir = dirname(base);
 
   let resolved;
   if (baseDir.includes(":"))
     resolved = join(baseDir, id);
@@ -390,17 +354,17 @@ const resolve = iced(function resolve(id
     resolved = normalize(`${baseDir}/${id}`);
 
   // Joining and normalizing removes the './' from relative files.
   // We need to ensure the resolution still has the root
   if (base.startsWith('./'))
     resolved = './' + resolved;
 
   return resolved;
-});
+}
 
 function addTrailingSlash(path) {
   return path.replace(/\/*$/, "/");
 }
 
 function compileMapping(paths) {
   // Make mapping array that is sorted from longest path to shortest path.
   let mapping = Object.keys(paths)
@@ -441,23 +405,23 @@ function compileMapping(paths) {
 
   // This will replace the longest matching path mapping at the start of
   // the ID string with its mapped value.
   return id => {
     return id.replace(pattern, (m0, m1) => paths[m1]);
   };
 }
 
-const resolveURI = iced(function resolveURI(id, mapping) {
+const resolveURI = function resolveURI(id, mapping) {
   // Do not resolve if already a resource URI
   if (isAbsoluteURI(id))
     return normalizeExt(id);
 
   return normalizeExt(mapping(id))
-});
+}
 
 /**
  * Defines lazy getters on the given object, which lazily require the
  * given module the first time they are accessed, and then resolve that
  * module's exported properties.
  *
  * @param {object} obj
  *        The target object on which to define the lazy getters.
@@ -503,17 +467,17 @@ function lazyRequireModule(obj, moduleId
   defineLazyGetter(obj, prop, () => this.require(moduleId));
 }
 
 
 // Creates version of `require` that will be exposed to the given `module`
 // in the context of the given `loader`. Each module gets own limited copy
 // of `require` that is allowed to load only a modules that are associated
 // with it during link time.
-const Require = iced(function Require(loader, requirer) {
+const Require = function Require(loader, requirer) {
   let {
     modules, mapping, mappingCache, resolve: loaderResolve, load,
     manifest, rootURI, isNative, requireHook
   } = loader;
 
   function require(id) {
     if (!id) // Throw if `id` is not passed.
       throw Error('You must provide a module name when calling require() from '
@@ -532,30 +496,28 @@ const Require = iced(function Require(lo
     let module = null;
     // If module is already cached by loader then just use it.
     if (uri in modules) {
       module = modules[uri];
     }
     else if (isJSMURI(uri)) {
       module = modules[uri] = Module(requirement, uri);
       module.exports = Cu.import(uri, {});
-      freeze(module);
     }
     else if (isJSONURI(uri)) {
       let data;
 
       // First attempt to load and parse json uri
       // ex: `test.json`
       // If that doesn't exist, check for `test.json.js`
       // for node parity
       try {
         data = JSON.parse(readURI(uri));
         module = modules[uri] = Module(requirement, uri);
         module.exports = data;
-        freeze(module);
       }
       catch (err) {
         // If error thrown from JSON parsing, throw that, do not
         // attempt to find .json.js file
         if (err && /JSON\.parse/.test(err.message))
           throw err;
         uri = uri + '.js';
       }
@@ -633,43 +595,43 @@ const Require = iced(function Require(lo
   // This is like webpack's require.context.  It returns a new require
   // function that prepends the prefix to any requests.
   require.context = prefix => {
     return id => {
       return require(prefix + id);
     };
   };
 
-  return iced(require);
-});
+  return require;
+}
 
 // Makes module object that is made available to CommonJS modules when they
 // are evaluated, along with `exports` and `require`.
-const Module = iced(function Module(id, uri) {
+const Module = function Module(id, uri) {
   return Object.create(null, {
     id: { enumerable: true, value: id },
     exports: { enumerable: true, writable: true, value: Object.create(null),
                configurable: true },
     uri: { value: uri }
   });
-});
+}
 
 // Takes `loader`, and unload `reason` string and notifies all observers that
 // they should cleanup after them-self.
-const unload = iced(function unload(loader, reason) {
+const unload = function unload(loader, reason) {
   // subject is a unique object created per loader instance.
   // This allows any code to cleanup on loader unload regardless of how
   // it was loaded. To handle unload for specific loader subject may be
   // asserted against loader.destructor or require('@loader/unload')
   // Note: We don not destroy loader's module cache or sandboxes map as
   // some modules may do cleanup in subsequent turns of event loop. Destroying
   // cache may cause module identity problems in such cases.
   let subject = { wrappedJSObject: loader.destructor };
   notifyObservers(subject, 'sdk:loader:destroy', reason);
-});
+};
 
 // Function makes new loader that can be used to load CommonJS modules
 // described by a given `options.manifest`. Loader takes following options:
 // - `globals`: Optional map of globals, that all module scopes will inherit
 //   from. Map is also exposed under `globals` property of the returned loader
 //   so it can be extended further later. Defaults to `{}`.
 // - `modules` Optional map of built-in module exports mapped by module id.
 //   These modules will incorporated into module cache. Each module will be
@@ -726,17 +688,17 @@ function Loader(options) {
     manifest.jetpack.overrides = {};
   }
 
   // We create an identity object that will be dispatched on an unload
   // event as subject. This way unload listeners will be able to assert
   // which loader is unloaded. Please note that we intentionally don't
   // use `loader` as subject to prevent a loader access leakage through
   // observer notifications.
-  let destructor = freeze(Object.create(null));
+  let destructor = Object.create(null);
 
   let mapping = compileMapping(paths);
 
   // Define pseudo modules.
   modules = override({
     '@loader/unload': destructor,
     '@loader/options': options,
     'chrome': { Cc: Cc, Ci: Ci, Cu: Cu, Cr: Cr, Cm: Cm,
@@ -758,17 +720,17 @@ function Loader(options) {
     // allow them to be loaded lazily.
     Object.defineProperty(module, "exports", {
       enumerable: true,
       get: function() {
         return builtinModuleExports[id];
       }
     });
 
-    modules[uri] = freeze(module);
+    modules[uri] = module;
   }
 
   // Create the unique sandbox we will be using for all modules,
   // so that we prevent creating a new comportment per module.
   // The side effect is that all modules will share the same
   // global objects.
   let sharedGlobalSandbox = Sandbox({
     name: options.sandboxName || "Addon-SDK",
@@ -813,17 +775,17 @@ function Loader(options) {
     // Whether the modules loaded should be ignored by the debugger
     invisibleToDebugger: { enumerable: false,
                            value: options.invisibleToDebugger || false },
     load: { enumerable: false, value: options.load || load },
     requireHook: { enumerable: false, value: options.requireHook },
     loadModuleHook: { enumerable: false, value: options.loadModuleHook },
   };
 
-  return freeze(Object.create(null, returnObj));
+  return Object.create(null, returnObj);
 };
 
 var isSystemURI = uri => /^resource:\/\/(gre|devtools|testing-common)\//.test(uri);
 
 var isJSONURI = uri => uri.endsWith('.json');
 var isJSMURI = uri => uri.endsWith('.jsm');
 var isJSURI = uri => uri.endsWith('.js');
 var isAbsoluteURI = uri => /^(resource|chrome|file|jar):/.test(uri);