Bug 93198

Created attachment 158824 [details] Patch

Comment on attachment 158824 [details] Patch Dropping the r?. This is nowhere near ready to land. :)

Hey Jochen! This is a very much WIP patch that I'd appreciate some help with. Can you recommend someone who knows things about JSC to help me evaluate an approach that they might accept? :)

Comment on attachment 158824 [details] Patch ggaren is your man View in context: https://bugs.webkit.org/attachment.cgi?id=158824&action=review > Source/JavaScriptCore/ChangeLog:3 > + When `eval` is blocked by CSP, the error message should make that clear. 'eval' :) > Source/JavaScriptCore/runtime/JSGlobalObject.h:300 > + void setEvalEnabled(bool enabled, const String& reason); I'd pick a more descriptive name instead of reason. Something that indicates that this will be included in the error message if eval is disabled and something tries to eval > Source/WebCore/page/ContentSecurityPolicy.cpp:643 > + String m_allowEvalReason; it's more a disallow reason, isn't it?

(In reply to comment #3) > Hey Jochen! This is a very much WIP patch that I'd appreciate some help with. Can you recommend someone who knows things about JSC to help me evaluate an approach that they might accept? :) (In reply to comment #4) > (From update of attachment 158824 [details]) > ggaren is your man Thanks Jochen! Hi Geoffrey! :) Would you mind helping me work out a mechanism for passing information down into JSC to improve the experience for developers who unexpectedly run into Content Security Policy's ability to disable 'eval' and its friends? I'd like on the one hand to pass in a more detailed string to be used as the error message, and on the other to generate a console warning (as we're seeing reports from developers using libraries that swallow the exception in a try/catch block). To make that possible, V8 has a callback mechanism (see http://code.google.com/searchframe#OAMlx_jo-ck/src/v8/src/runtime.cc&exact_package=chromium&q=allow_code_gen_callback&type=cs&l=9406) that I'd like to implement in JSC as well. The current patch is an attempt at the former. I'd like to talk about the latter. WDYT?

Comment on attachment 158824 [details] Patch Attachment 158824 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13511598

Comment on attachment 158824 [details] Patch Attachment 158824 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13512509

> I'd like on the one hand to pass in a more detailed string to be used as the error message Current patch looks good for that. I agree that you should use something more descriptive than "reason", which denotes an error message. > and on the other to generate a console warning The Web Inspector already gets callbacks when exceptions are thrown. Does that mechanism work for you? Does it need to be expanded in some way?

(In reply to comment #8) > > I'd like on the one hand to pass in a more detailed string to be used as the error message > > Current patch looks good for that. I agree that you should use something more descriptive than "reason", which denotes an error message. Ok. Then I'm closer than I expected. :) I'll split this bug into two: one for JSC and one for V8. The latter's going to be a bit more work. *cough* > > and on the other to generate a console warning > > The Web Inspector already gets callbacks when exceptions are thrown. Does that mechanism work for you? Does it need to be expanded in some way? The goal is to provide developers who use libraries that swallow exceptions some idea of what's going on. Many libraries parse JSON with 'eval', wrap it in try/catch, and assume that an exception means "Invalid JSON!" It would be nice if we generated a console warning in addition to throwing an exception at the point where 'eval' is called. I'll take a look at the throwError implementation to see if there's a simple way of hooking into the preexisting Web Inspector code. Thanks for the tip.

Both V8 and JSC landed. Closing this out.