New snapshot.
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js
File src/com/google/caja/plugin/domado.js (right):
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js#newcode574
src/com/google/caja/plugin/domado.js:574: var ProxyHandler =
domitaModules.ProxyHandler;
On 2013/04/15 19:46:12, MarkM wrote:
Please place this declaration/initialization after the following
conditional,
since it is only relevant if the condition falls through.
I don't like putting “imports” anywhere but the top, but OK.
(Really, we should eliminate domitaModules entirely — it is vestigial —
but not as part of this CL.)
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js#newcode581
src/com/google/caja/plugin/domado.js:581: if (/^weakmap:/.test(name)) {
weakMapMagicName = name; }
On 2013/04/15 19:46:12, MarkM wrote:
This seems totally bizarre. I don't understand the purpose of this or
why it is
safe.
I am uncertain what part you are referring to.
- This proxy is designed to capture the magic property name so that
other proxies can handle it correctly.
- The regexp test ensures that we don't fail by capturing some other,
not WeakMap related, implicit property access triggered by the test
code.
- If you are concerned about the magic property being visible here, that
is not indicative of a vulnerability because we do not whitelist Proxy
for guest code, because of this issue (as noted in WeakMap.js comments).
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js#newcode591
src/com/google/caja/plugin/domado.js:591: w.get(proxy);
On 2013/04/15 20:36:53, ihab.awad wrote:
The trick that is happening here -- using a secretive channel to get
at the otherwise secret WeakMap property name -- is very subtle
and might benefit from some explanation.
Done.
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js
File src/com/google/caja/ses/WeakMap.js (right):
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js#newcode462
src/com/google/caja/ses/WeakMap.js:462: if (typeof HostWeakMap ===
'function') (function() {
On 2013/04/15 20:36:53, ihab.awad wrote:
Please use curlies after condition, as a stylistic thing to avoid
dangling
statement problems....
The function thingy is intended to substitute for the block entirely;
note the indentation would make it difficult to accidentally add a
dangling statement.
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js#newcode500
src/com/google/caja/ses/WeakMap.js:500: WeakMap = DoubleWeakMap;
On 2013/04/15 19:46:12, MarkM wrote:
after:
WeakMap.prototype.constructor = WeakMap;
Otherwise, WeakMap.prototype.constructor would be something
OurWeakMap.
Done. Added a test too.
https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js#newcode500
src/com/google/caja/ses/WeakMap.js:500: WeakMap = DoubleWeakMap;
On 2013/04/15 20:36:53, ihab.awad wrote:
Is this clever wonderfulness actually exercised by any tests in our
codebase at
the moment?
Done.
https://codereview.appspot.com/8612048/
--
---
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.