WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(6.49 KB, patch)
2011-04-01 00:17 PDT
,
Yury Semikhatsky
pfeldman
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 87765
[details]
Patch
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
Created
attachment 87829
[details]
Patch
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
Committed
r82664
: <
http://trac.webkit.org/changeset/82664
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug