https://codereview.appspot.com/6827077/diff/1/build.xml File build.xml (right):
https://codereview.appspot.com/6827077/diff/1/build.xml#newcode990 build.xml:990: <input file="${third_party}/js/escodegen/escodegen.js" jslint="false"/> On 2012/11/12 19:04:24, kpreid2 wrote:
Since these third-party files support the 'exports' pattern, we should
use it in
order to avoid crufting up the global environment. Place files before
and after
this group which create an 'exports' object (saving the old variable),
let them
load, then transfer the exports into the 'ses' module and restore the
original
'exports' if any.
Done. https://codereview.appspot.com/6827077/diff/1/build.xml#newcode1016 build.xml:1016: <input file="${src.caja}/ses/mitigateGotchas.js"/> On 2012/11/12 19:04:24, kpreid2 wrote:
Ditto.
Done. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js File src/com/google/caja/ses/mitigateGotchas.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode18 src/com/google/caja/ses/mitigateGotchas.js:18: * outside the TCB. On 2012/11/12 18:16:08, MarkM wrote:
Please put in a "see" link to http://code.google.com/p/google-caja/wiki/SES#Source-SES_vs_Target-SES
. How do
the mitigations in this CL relate to the mitigations explained there?
Can we put
in a TODO for the mitigations explained there but not yet implemented
here?
Do we know of any other needed mitigations besides those explained
there? Done. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode20 src/com/google/caja/ses/mitigateGotchas.js:20: * Note the parse tree manipulate in this file uses the SpiderMonkey On 2012/11/12 19:16:24, metaweta wrote:
That sentence doesn't parse.
Done. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode48 src/com/google/caja/ses/mitigateGotchas.js:48: * window.x = window.x, window.y = 2, window.z = window.z Like strict mode, if a variable is used on the RHS without being declared. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/mitigateGotchas.js#newcode81 src/com/google/caja/ses/mitigateGotchas.js:81: * (function() { try { typeof x } catch (e) { return "undefined" } })() Yeah - I'll document this as a gotcha for now -- these cases lexically inlined cases are rarer than an expression (like a call expression) being the argument to typeof. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js File src/com/google/caja/ses/startSES.js (right): https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js#newcode633 src/com/google/caja/ses/startSES.js:633: * analogous {@code compileProgram} function that accepts a Added a TODO. I'd like to maintain the kludge until we have some empirical data to suggest the size and slowdown from introducing a parser isn't exorbitant. https://codereview.appspot.com/6827077/diff/1/src/com/google/caja/ses/startSES.js#newcode646 src/com/google/caja/ses/startSES.js:646: exprSrc = mitigateGotchas(exprSrc); Good catch. Fixed. https://codereview.appspot.com/6827077/diff/1/tests/com/google/caja/plugin/es53-test-gotchas-guest.html File tests/com/google/caja/plugin/es53-test-gotchas-guest.html (right): https://codereview.appspot.com/6827077/diff/1/tests/com/google/caja/plugin/es53-test-gotchas-guest.html#newcode17 tests/com/google/caja/plugin/es53-test-gotchas-guest.html:17: <div class="testcontainer" id="testTopLevel" Enabled testWindowObject in domado-dom tests. I'd like to keep these gotcha tests that test the mitigation between es5 and es53 together. https://codereview.appspot.com/6827077/diff/1/tests/com/google/caja/plugin/es53-test-gotchas-guest.html#newcode36 tests/com/google/caja/plugin/es53-test-gotchas-guest.html:36: debugger; On 2012/11/12 19:04:24, kpreid2 wrote:
cruft
Done. https://codereview.appspot.com/6827077/diff/1/tests/com/google/caja/plugin/es53-test-gotchas-guest.html#newcode53 tests/com/google/caja/plugin/es53-test-gotchas-guest.html:53: jsunitRegisterIf(inES5Mode, On 2012/11/12 19:04:24, kpreid2 wrote:
no testcontainer to match this test
Done. https://codereview.appspot.com/6827077/
