Last Comment Bug 655921 - Do CSP checks fast and early in Function function
: Do CSP checks fast and early in Function function
Status: RESOLVED FIXED
fixed-in-tracemonkey
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla6
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 655192 655907
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-09 18:56 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-05-23 14:13 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (3.10 KB, patch)
2011-05-09 18:56 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
mrbkap: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-09 18:56:55 PDT
Created attachment 531225 [details] [diff] [review]
Patch

Right now Function creates the function it'll return (minus its script), then it does CSP checks to see whether to reject the attempt to create dynamic code.  And it does this by unconditionally calling the security callback, when it really should use GlobalObject::isEvalAllowed to take advantage of its cache of the eval-prohibited setting.  It should check for prohibition first, then proceed to creating an object to return, and it should do so using the is-prohibited fast path.

This builds atop bug 655192 and bug 655907 for no particular reason beyond adding a new explicit-global NewFunction overload, which is the direction we need to start heading for all APIs that can create an object, in order to eventually fully fix bug 631135.  I trust the minimal reliance on those patches (for the obviously-named GlobalObject::getOrCreateFunctionPrototype) won't present a difficulty.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-09 19:00:11 PDT
It was tempting to rename isEvalAllowed, but that made the overlap with the other two bugs a bit more than I wanted, before those bugs land.  That can be a followup or something, probably touches enough code to deserve it.
Comment 2 Blake Kaplan (:mrbkap) 2011-05-10 09:12:46 PDT
Comment on attachment 531225 [details] [diff] [review]
Patch

I was just going to suggest changing the function name when I saw comment 1!
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-18 14:30:08 PDT
http://hg.mozilla.org/tracemonkey/rev/6408576c8e08

Comment 1 is now bug 658069; I'll try to get to it shortly.
Comment 4 Chris Leary [:cdleary] (not checking bugmail) 2011-05-23 14:13:51 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/6408576c8e08

Note You need to log in before you can comment on or make changes to this bug.