Last Comment Bug 655921 - Do CSP checks fast and early in Function function
: Do CSP checks fast and early in Function function
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]
Depends on: 655192 655907
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-09 18:56:55 PDT
Created attachment 531225 [details] [diff] [review]

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 User image 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 User image Blake Kaplan (:mrbkap) 2011-05-10 09:12:46 PDT
Comment on attachment 531225 [details] [diff] [review]

I was just going to suggest changing the function name when I saw comment 1!
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-18 14:30:08 PDT

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

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