RESOLVED FIXED 57557
[Crash] Web Inspector: inspected page crashes on attempt to log object with broken .toString
https://bugs.webkit.org/show_bug.cgi?id=57557
Summary [Crash] Web Inspector: inspected page crashes on attempt to log object with b...
Yury Semikhatsky
Reported 2011-03-31 09:55:16 PDT
Steps to reproduce: 1. Navigate to a page with the following content: <script> console.log({toString:{}}); </script> 2. Open inspector. 3. Reload the inspected page. Result: Page crashes. Original Chromium bug report: http://code.google.com/p/chromium/issues/detail?id=74411
Attachments
Patch (6.46 KB, patch)
2011-03-31 10:55 PDT, Yury Semikhatsky
no flags
Patch (6.49 KB, patch)
2011-04-01 00:17 PDT, Yury Semikhatsky
pfeldman: review+
Yury Semikhatsky
Comment 1 2011-03-31 10:26:24 PDT
Clearing [V8] prefix since it's reproducible with JSC as well.
Yury Semikhatsky
Comment 2 2011-03-31 10:55:25 PDT
Yury Semikhatsky
Comment 3 2011-03-31 10:57:11 PDT
A big thanks to Anton Muhin for his help in finding the real cause of this crash.
anton muhin
Comment 4 2011-03-31 11:36:02 PDT
Comment on attachment 87765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87765&action=review LGTM > LayoutTests/inspector/console/console-log-toString-object.html:20 > +Tests passing object with broken toString attribute into console.log won't crash nit: I am having some troubles parsing this sentence, maybe Test <that> passing object... And maybe you should generalize the case: passing an object which throws on string conversion or something of this kind > Source/WebCore/ChangeLog:8 > + If ScriptValue.toString causes a JavaScript exception the exception is cleared nit: exception<,> the exception (add a comma) > Source/WebCore/bindings/v8/ScriptValue.cpp:72 > + return v8StringToWebCoreString<String>(s, DoNotExternalize); Another option would be to teach V8Parameter not to rethrow an exception. The benefit would be your code would be shorter and others can use the same logic too. Up to you though,
Yury Semikhatsky
Comment 5 2011-04-01 00:17:37 PDT
Yury Semikhatsky
Comment 6 2011-04-01 00:18:48 PDT
(In reply to comment #4) > (From update of attachment 87765 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=87765&action=review > > LayoutTests/inspector/console/console-log-toString-object.html:20 > > +Tests passing object with broken toString attribute into console.log won't crash > > nit: I am having some troubles parsing this sentence, maybe Test <that> passing object... And maybe you should generalize the case: passing an object which throws on string conversion or something of this kind > Done. > > Source/WebCore/ChangeLog:8 > > + If ScriptValue.toString causes a JavaScript exception the exception is cleared > > nit: exception<,> the exception (add a comma) > Done. > > Source/WebCore/bindings/v8/ScriptValue.cpp:72 > > + return v8StringToWebCoreString<String>(s, DoNotExternalize); > > Another option would be to teach V8Parameter not to rethrow an exception. The benefit would be your code would be shorter and others can use the same logic too. Up to you though, I'd rather do this in a separate change.
Yury Semikhatsky
Comment 7 2011-04-01 01:59:47 PDT
Note You need to log in before you can comment on or make changes to this bug.