Revision: 5486
Author:   erights
Date:     Sat Jul 13 08:35:24 2013
Log:      Minor uncontributed improvements
https://codereview.appspot.com/10075043

Several little things
* changed many double quoted strings to single quotes
* StringMap can now be used without ses
* optional source position for evalators have been moved into the options
object
* removed "skip" from whitelist and whitelist mechanism since we don't currently
need it

[email protected]

http://code.google.com/p/google-caja/source/detail?r=5486

Modified:
 /trunk/src/com/google/caja/ses/StringMap.js
 /trunk/src/com/google/caja/ses/WeakMap.js
 /trunk/src/com/google/caja/ses/atLeastFreeVarNames.js
 /trunk/src/com/google/caja/ses/compileExprLater.js
 /trunk/src/com/google/caja/ses/debug.js
 /trunk/src/com/google/caja/ses/explicit.html
 /trunk/src/com/google/caja/ses/makeSimpleAMDLoader.js
 /trunk/src/com/google/caja/ses/repairES5.js
 /trunk/src/com/google/caja/ses/startSES.js
 /trunk/src/com/google/caja/ses/whitelist.js

=======================================
--- /trunk/src/com/google/caja/ses/StringMap.js Thu Apr 25 14:25:03 2013
+++ /trunk/src/com/google/caja/ses/StringMap.js Sat Jul 13 08:35:24 2013
@@ -24,7 +24,7 @@
 var StringMap;

 (function() {
-   "use strict";
+   'use strict';

    var create = Object.create;
    var freeze = Object.freeze;
@@ -41,9 +41,12 @@
    }

    var createNull;
-   if (((ses.es5ProblemReports || {}).FREEZING_BREAKS_PROTOTYPES || {})
-       .beforeFailure) {
- // Object.create(null) is broken; fall back to ES3-style implementation
+
+   if (typeof ses === 'undefined' ||
+       !ses.ok() ||
+       ses.es5ProblemReports.FREEZING_BREAKS_PROTOTYPES.beforeFailure) {
+
+ // Object.create(null) may be broken; fall back to ES3-style implementation
      // (safe because we suffix keys anyway).
      createNull = function() { return {}; };
    } else {
=======================================
--- /trunk/src/com/google/caja/ses/WeakMap.js   Tue Jun 18 13:18:13 2013
+++ /trunk/src/com/google/caja/ses/WeakMap.js   Sat Jul 13 08:35:24 2013
@@ -22,9 +22,13 @@
  * implementation where the {@code WeakMap} specification does not
  * quite conform, run <code>repairES5.js</code> first.
  *
+ * <p> Even though WeakMapModule is not global, the linter thinks it
+ * is, which is why it is in the overrides list below.
+ *
  * @author Mark S. Miller
  * @requires crypto, ArrayBuffer, Uint8Array, navigator
  * @overrides WeakMap, ses, Proxy
+ * @overrides WeakMapModule
  */

 /**
=======================================
--- /trunk/src/com/google/caja/ses/atLeastFreeVarNames.js Tue Jul 2 11:57:39 2013 +++ /trunk/src/com/google/caja/ses/atLeastFreeVarNames.js Sat Jul 13 08:35:24 2013
@@ -106,13 +106,13 @@
         return '\\u' + ('0000' + u.charCodeAt(0).toString(16)).slice(-4);
       });
     return { programSrc: programSrc };
-  }
+  };

   /**
    * Return a regexp that can be used repeatedly to scan for the next
* identifier. It works correctly in concert with ses.limitSrcCharset above.
    *
-   * If this regexp is changed compileExprLater.js should be checked for
+   * If this regexp is changed, compileExprLater.js should be checked for
    * correct escaping of freeNames.
    */
   function SHOULD_MATCH_IDENTIFIER() {
@@ -125,11 +125,11 @@

   ses.atLeastFreeVarNames = function atLeastFreeVarNames(programSrc) {
     programSrc = ''+programSrc;
-    var result = ses.limitSrcCharset(programSrc);
-    if (!('programSrc' in result)) {
-      throw new EvalError(result.error);
+    var limited = ses.limitSrcCharset(programSrc);
+    if (!('programSrc' in limited)) {
+      throw new EvalError(limited.error);
     } else {
-      programSrc = result.programSrc;
+      programSrc = limited.programSrc;
     }
     // Now that we've temporarily limited our attention to ascii...
     var regexp = SHOULD_MATCH_IDENTIFIER();
=======================================
--- /trunk/src/com/google/caja/ses/compileExprLater.js Wed Jun 5 20:35:14 2013 +++ /trunk/src/com/google/caja/ses/compileExprLater.js Sat Jul 13 08:35:24 2013
@@ -53,15 +53,13 @@
     * be one that sends the expression back up the server to be
     * cajoled.
     */
-   function compileExprLaterFallback(exprSrc,
-                                     opt_mitigateOpts,
-                                     opt_sourcePosition) {
+   function compileExprLaterFallback(exprSrc, opt_mitigateOpts) {
      // Coercing an object to a string may observably run code, so do
      // this now rather than in any later turn.
      exprSrc = ''+exprSrc;

      return Q(cajaVM).send('compileExpr',
-                           exprSrc, opt_mitigateOpts, opt_sourcePosition);
+                           exprSrc, opt_mitigateOpts);
    }

    if (typeof document === 'undefined') {
@@ -87,14 +85,14 @@
    /**
     * Implements an eventual compileExpr using injected script tags
     */
- function compileLaterInScript(exprSrc, opt_mitigateOpts, opt_sourceUrl) {
+   function compileLaterInScript(exprSrc, opt_mitigateOpts) {

      var result = Q.defer();

      // The portion of the pattern in compileExpr which is appropriate
      // here as well.
      var options = ses.resolveOptions(opt_mitigateOpts);
-     var wrapperSrc = ses.securableWrapperSrc(exprSrc, opt_sourceUrl);
+     var wrapperSrc = ses.securableWrapperSrc(exprSrc);
      var freeNames = ses.atLeastFreeVarNames(exprSrc);

      var head = document.getElementsByTagName("head")[0];
@@ -110,10 +108,10 @@
          '["' + freeNames.join('", "') + '"], ' +
          JSON.stringify(options) + ')));';

-     if (opt_sourceUrl) {
+     if (options.sourceUrl === void 0) {
        // See http://code.google.com/p/google-caja/wiki/SES#typeof_variable
        if (typeof global.URI !== 'undefined' && URI.parse) {
-         var parsed = URI.parse(String(opt_sourceUrl));
+         var parsed = URI.parse(String(options.sourceUrl));
          parsed = null === parsed ? null : parsed.toString();

// Note we could try to encode these characters or search specifically
=======================================
--- /trunk/src/com/google/caja/ses/debug.js     Tue Apr  9 19:24:18 2013
+++ /trunk/src/com/google/caja/ses/debug.js     Sat Jul 13 08:35:24 2013
@@ -95,7 +95,7 @@
            ssts.set(err, sst);
          }
          // Technically redundant, but prepareStackTrace is supposed
-         // to return a value, so this make it clearer that this value
+         // to return a value, so this makes it clearer that this value
          // is undefined (void 0).
          return void 0;
        };
=======================================
--- /trunk/src/com/google/caja/ses/explicit.html        Wed Jun  5 20:35:14 2013
+++ /trunk/src/com/google/caja/ses/explicit.html        Sat Jul 13 08:35:24 2013
@@ -64,7 +64,6 @@
 <script src="startSES.js"></script>
 <script src="ejectorsGuardsTrademarks.js"></script>
 <script src="hookupSESPlus.js"></script>
-<script src="../plugin/uri.js"></script>

 <script>
   var amdLoaderTest = gebi('amdLoaderTest');
=======================================
--- /trunk/src/com/google/caja/ses/makeSimpleAMDLoader.js Wed Jun 5 20:35:14 2013 +++ /trunk/src/com/google/caja/ses/makeSimpleAMDLoader.js Sat Jul 13 08:35:24 2013
@@ -125,7 +125,7 @@
          cajaVM.def(imports);

          var compiledExprP = compileExprLater(
-           '(function(){' + src + '})()', void 0, id);
+           '(function(){' + src + '})()', { sourceUrl: id });
          return Q(compiledExprP).then(function(compiledExpr) {

            compiledExpr(imports);
=======================================
--- /trunk/src/com/google/caja/ses/repairES5.js Thu Jul 11 18:45:27 2013
+++ /trunk/src/com/google/caja/ses/repairES5.js Sat Jul 13 08:35:24 2013
@@ -3469,7 +3469,7 @@
       urls: [],
       sections: ['15.2.3.4'],
       tests: ['15.2.3.4-0-1']
-    },
+    }
   ];

   /**
@@ -3689,7 +3689,8 @@
       preSeverity: severities.SAFE_SPEC_VIOLATION,
       canRepair: true,
       urls: ['http://code.google.com/p/v8/issues/detail?id=2273',
-          
'https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/forEach'],
+             'https://developer.mozilla.org/en-US/docs/JavaScript/' +
+               'Reference/Global_Objects/Array/forEach'],
       sections: ['15.4.4.18'],
       tests: []
     },
@@ -4110,7 +4111,8 @@
       preSeverity: severities.UNSAFE_SPEC_VIOLATION,
       canRepair: false,  // repair is useful but inadequate
       urls: ['https://bugzilla.mozilla.org/show_bug.cgi?id=784892',
-             'https://bugzilla.mozilla.org/show_bug.cgi?id=674195'],
+             'https://bugzilla.mozilla.org/show_bug.cgi?id=674195',
+             'https://bugzilla.mozilla.org/show_bug.cgi?id=789897'],
       sections: [],
       tests: []
     },
@@ -4174,7 +4176,7 @@
       preSeverity: severities.SAFE_SPEC_VIOLATION,
       canRepair: false,
       urls: ['https://code.google.com/p/v8/issues/detail?id=1310',
-             
'https://mail.mozilla.org/pipermail/es-discuss/2013-March/029177.html'],
+        
'https://mail.mozilla.org/pipermail/es-discuss/2013-March/029177.html'],
sections: [], // Not spelled out in spec, according to Brendan Eich (see
                      // es-discuss link)
tests: [] // TODO(kpreid): add to test262 once we have a section to cite
=======================================
--- /trunk/src/com/google/caja/ses/startSES.js  Thu Jul 11 18:45:27 2013
+++ /trunk/src/com/google/caja/ses/startSES.js  Sat Jul 13 08:35:24 2013
@@ -165,7 +165,7 @@
  *        {@code global} is the global object of <i>this</i>
  *        frame. The code should be made to work for cross-frame use.
  * @param whitelist ::Record(Permit) where Permit = true | "*" |
- *        "skip" | Record(Permit).  Describes the subset of naming
+ *        Record(Permit).  Describes the subset of naming
  *        paths starting from {@code sharedImports} that should be
  *        accessible. The <i>accessible primordials</i> are all values
  *        found by navigating these paths starting from {@code
@@ -288,6 +288,14 @@
    * options and their effects.
    */
   function resolveOptions(opt_mitigateOpts) {
+    if (typeof opt_mitigateOpts === 'string') {
+      // TODO: transient deprecated adaptor only, since there used to
+      // be an opt_sourceUrl parameter in many of the parameter
+      // positions now accepting an opt_mitigateOpts. Once we are
+      // confident that we no longer have live clients that count on
+      // the  old behavior, remove this kludge.
+      opt_mitigateOpts = { sourceUrl: opt_mitigateOpts };
+    }
     function resolve(opt, defaultOption) {
       return (opt_mitigateOpts && opt in opt_mitigateOpts) ?
         opt_mitigateOpts[opt] : defaultOption;
@@ -296,6 +304,7 @@
     if (opt_mitigateOpts === undefined || opt_mitigateOpts === null) {
       options.maskReferenceError = true;
       options.parseFunctionBody = true;
+      options.sourceUrl = void 0;

       options.rewriteTopLevelVars = true;
       options.rewriteTopLevelFuncs = true;
@@ -305,6 +314,7 @@
     } else {
       options.maskReferenceError = resolve('maskReferenceError', true);
       options.parseFunctionBody = resolve('parseFunctionBody', false);
+      options.sourceUrl = resolve('sourceUrl', void 0);

       options.rewriteTopLevelVars = resolve('rewriteTopLevelVars', true);
       options.rewriteTopLevelFuncs = resolve('rewriteTopLevelFuncs', true);
@@ -695,13 +705,8 @@
      *     accurate wrt the original source text, and except for the
      *     first line, all the column numbers are accurate too.
      * </ul>
-     *
-     * <p>TODO(erights): Find out if any platforms have any way to
-     * associate a file name and line number with eval'ed text, so
-     * that we can do something useful with the optional {@code
-     * opt_sourcePosition} to better support debugging.
      */
-    function securableWrapperSrc(exprSrc, opt_sourcePosition) {
+    function securableWrapperSrc(exprSrc) {
       verifyStrictExpression(exprSrc);

       return '(function() { ' +
@@ -799,7 +804,7 @@
      * {@code with} together with RegExp matching to intercept free
      * variable access without parsing.
      */
-    function compileExpr(src, opt_mitigateOpts, opt_sourcePosition) {
+    function compileExpr(src, opt_mitigateOpts) {
       // Force src to be parsed as an expr
       var exprSrc = '(' + src + '\n)';

@@ -811,7 +816,7 @@
       if (exprSrc[exprSrc.length - 1] === ';') {
         exprSrc = exprSrc.substr(0, exprSrc.length - 1);
       }
-      var wrapperSrc = securableWrapperSrc(exprSrc, opt_sourcePosition);
+      var wrapperSrc = securableWrapperSrc(exprSrc);
       var wrapper = unsafeEval(wrapperSrc);
       var freeNames = atLeastFreeVarNames(exprSrc);
       var result = makeCompiledExpr(wrapper, freeNames, options);
@@ -829,13 +834,13 @@
      * endowments are the only source of eval-time abilities for the
      * expr to cause effects.
      */
-    function confine(exprSrc, opt_endowments, opt_sourcePosition) {
+    function confine(exprSrc, opt_endowments, opt_mitigateOpts) {
       var imports = makeImports();
       if (opt_endowments) {
         copyToImports(imports, opt_endowments);
       }
       def(imports);
-      return compileExpr(exprSrc, opt_sourcePosition)(imports);
+      return compileExpr(exprSrc, opt_mitigateOpts)(imports);
     }


@@ -900,7 +905,7 @@
      * {@code getRequirements} above would also have to extract these
      * from the text to be compiled.
      */
-    function compileModule(modSrc, opt_mitigateOpts, opt_sourcePosition) {
+    function compileModule(modSrc, opt_mitigateOpts) {
// Note the EOL after modSrc to prevent trailing line comment in modSrc
       // eliding the rest of the wrapper.
       var options = resolveOptions(opt_mitigateOpts);
@@ -912,7 +917,7 @@
           mitigateSrcGotchas(modSrc, options) +
           '\n}).call(this)';
       // Follow the pattern in compileExpr
-      var wrapperSrc = securableWrapperSrc(exprSrc, opt_sourcePosition);
+      var wrapperSrc = securableWrapperSrc(exprSrc);
       var wrapper = unsafeEval(wrapperSrc);
       var freeNames = atLeastFreeVarNames(exprSrc);
       var moduleMaker = makeCompiledExpr(wrapper, freeNames, options);
@@ -1364,8 +1369,8 @@
    * cleaned.
    *
    * We initialize the whiteTable only so that {@code getPermit} can
-   * process "*" and "skip" inheritance using the whitelist, by
-   * walking actual superclass chains.
+   * process "*" inheritance using the whitelist, by walking actual
+   * superclass chains.
    */
   var whiteTable = WeakMap();
   function register(value, permit) {
@@ -1379,10 +1384,8 @@
     }
     whiteTable.set(value, permit);
     keys(permit).forEach(function(name) {
-      if (permit[name] !== 'skip') {
-        var sub = value[name];
-        register(sub, permit[name]);
-      }
+      var sub = value[name];
+      register(sub, permit[name]);
     });
   }
   register(sharedImports, whitelist);
@@ -1392,7 +1395,7 @@
    * {@code base} object, and if so, with what Permit?
    *
    * <p>If it should be permitted, return the Permit (where Permit =
-   * true | "*" | "skip" | Record(Permit)), all of which are
+   * true | "*" | Record(Permit)), all of which are
    * truthy. If it should not be permitted, return false.
    */
   function getPermit(base, name) {
@@ -1406,7 +1409,7 @@
       permit = whiteTable.get(base);
       if (permit && hop.call(permit, name)) {
         var result = permit[name];
-        if (result === '*' || result === 'skip') {
+        if (result === '*') {
           return result;
         } else {
           return false;
@@ -1557,13 +1560,8 @@
       var path = prefix + (prefix ? '.' : '') + name;
       var p = getPermit(value, name);
       if (p) {
-        if (p === 'skip') {
-          reportProperty(ses.severities.SAFE,
-                         'Skipped', path);
-        } else {
-          var sub = value[name];
-          clean(sub, path);
-        }
+        var sub = value[name];
+        clean(sub, path);
       } else {
         cleanProperty(value, name, path);
       }
@@ -1572,11 +1570,10 @@
   clean(sharedImports, '');

// es5ProblemReports has a 'dynamic' set of keys, and the whitelist mechanism
-  // does not support this (it only has 'skip', which is intended as a
- // workaround and logged as such), so as a kludge we insert it after cleaning
+  // does not support this, so as a kludge we insert it after cleaning
// and before defending. TODO(kpreid): Figure out a proper workaround. Perhaps
   // add another type of whitelisting (say a wildcard property name, or
-  // 'recursively JSON', or a non-warning 'skip')?
+  // 'recursively JSON')?
   cajaVM.es5ProblemReports = ses.es5ProblemReports;

   // This protection is now gathered here, so that a future version
=======================================
--- /trunk/src/com/google/caja/ses/whitelist.js Wed May  1 11:29:48 2013
+++ /trunk/src/com/google/caja/ses/whitelist.js Sat Jul 13 08:35:24 2013
@@ -60,21 +60,8 @@
  *     of these is tamed as if with true, so that the value of the
  *     property is further tamed according to what other objects it
  *     inherits from.
- * <li>"skip", in which case this property on this object is simply
- *     whitelisted, as is this property as inherited by all objects
- *     that inherit from this object, but we avoid taming the value
- *     associated with that property. For example, as of this writing
- *     {@code "Function.prototype.caller"} leads to "skip" because
- *     some current browser bugs prevent us from removing or even
- *     traversing this property on some platforms of interest.
  * </ul>
  *
- * The "skip" markings are workarounds for browser bugs or other
- * temporary problems. For each of these, there should be an
- * explanatory comment explaining why or a bug citation. Ideally, we
- * can retire all "skip" entries by the time SES is ready for secure
- * production use.
- *
  * The members of the whitelist are either
  * <ul>
  * <li>(uncommented) defined by the ES5.1 normative standard text,
@@ -91,7 +78,6 @@
  *     page are <b>not harmless</b> and so must not be whitelisted.
  * <li>(ES-Harmony proposal) accepted as "proposal" status for
  *     EcmaScript-Harmony.
- * <li>(Marked as "skip") See above.
  * </ul>
  *
  * <p>With the above encoding, there are some sensible whitelists we
@@ -266,9 +252,7 @@
         filter: t,
         reduce: t,
         reduceRight: t,
-        length: 'skip'               // can't be redefined on Mozilla
-        // See https://bugzilla.mozilla.org/show_bug.cgi?id=591059
-        // and https://bugzilla.mozilla.org/show_bug.cgi?id=598996
+        length: t
       },
       isArray: t
     },

--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to