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
On 2013/06/03 18:33:30, kpreid2 wrote:
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'?
Done.
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 &&
On 2013/06/03 18:33:30, kpreid2 wrote:
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.
Done. Added a "rewriteFunctionCalls" instead and noted, with a
TODO(jasvir), that this is currently unimplemented.
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.
On 2013/06/03 18:33:30, kpreid2 wrote:
This doesn't seem to be a desirable thing?
No. Deleted. Done.
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.
On 2013/06/03 18:33:30, kpreid2 wrote:
Please add a note that mitigateGotchas' rewrite takes care of that
under normal
circumstances.
Done.
https://codereview.appspot.com/9945043/diff/18001/src/com/google/caja/ses/startSES.js#newcode810
src/com/google/caja/ses/startSES.js:810: * cases.
On 2013/06/03 18:33:30, kpreid2 wrote:
Explicitly state that this is a less-correct-but-faster alternative to
rewriteTypeOf.
Done.
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.
