https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js
File src/com/google/caja/ses/startSES.js (right):
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode319
src/com/google/caja/ses/startSES.js:319: * program with the same
semantics as the original but with some of
Uninformed reader asks "Some? Why not all?"
Please explain the category of gotchas which are handled here and refer
to where the other ones are handled. I suspect (not having finished
review) that the answer is that this mitigates gotchas via source-code
transformations whereas the rest are by scope-object modifications. If
so, please say so.
Also, 'mitigateSomeGotchas' is an underinformative name. Perhaps
'mitigateSrcGotchas'?
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode647
src/com/google/caja/ses/startSES.js:647: if
(options.wrapImportedFunction &&
This seems like a bad idea to me, for two reasons:
1) the 'imported function' will be unexpectedly !== to 'itself', as well
as possibly having accessor properties misbehave. (That is, this code is
like an incoherent membrane.)
2) This is putting additional code, which allocates new objects, in a
very hot path. (Now that I think about it, this path (using an accessor
to alias rather than copying the property) may be hotter than it needs
to be; I'll look into that separately.)
Is there a reason why we do not perform this mitigation by
source-to-source transform of 'foo()', when foo is not shadowed or
perhaps even if it is, into '(0,foo)()'? This may not help the jQuery
case but it will the normal 'untrusted-code-in-the-wild' case.
If we must do this, then please minimize unnecessary work by moving the
definition of wrappedResult into a function called inside the
conditional and turn the lookups on 'options' into lexical variables.
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode668
src/com/google/caja/ses/startSES.js:668: // wrappedResult rather
than the original.
This doesn't seem to be a desirable thing?
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode681
src/com/google/caja/ses/startSES.js:681: // without parsing or proxies,
that isn't possible.
Please add a note that mitigateGotchas' rewrite takes care of that under
normal circumstances.
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode810
src/com/google/caja/ses/startSES.js:810: * cases.
Explicitly state that this is a less-correct-but-faster alternative to
rewriteTypeOf.
https://codereview.appspot.com/9945043/
--
---
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.
