Bug 41297

Summary: [V8] Catch toString() exceptions in all DB-related code in V8 bindings
Product: WebKit Reporter: Dumitru Daniliuc <dumi>
Component: New BugsAssignee: Dumitru Daniliuc <dumi>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch abarth: review+, dumi: commit-queue-

Dumitru Daniliuc
Reported 2010-06-28 13:01:59 PDT
V8Binding::v8NonStringValueToWebCoreString() suppresses the exceptions thrown by ToString() and returns an empty String. It shouldn't do that.
Attachments
patch (11.42 KB, patch)
2010-06-28 23:26 PDT, Dumitru Daniliuc 		abarth: review+
dumi: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dumitru Daniliuc
Comment 1 2010-06-28 23:26:46 PDT
Created attachment 59991 [details] patch
Adam Barth
Comment 2 2010-06-28 23:50:42 PDT
Comment on attachment 59991 [details] patch Looks good. WebCore/bindings/v8/custom/V8BindingMacros.h:35 + var = value; \ It's usually a good idea to put parenthesis around (value). Otherwise you might get strange order-of-operations issues. WebCore/bindings/v8/custom/V8BindingMacros.h:44 + v8::Handle<v8::String> v8String = value->ToString(); \ It's usually a good idea to put parenthesis around (value). Otherwise you might get strange order-of-operations issues.
Dumitru Daniliuc
Comment 3 2010-06-29 00:31:08 PDT
(In reply to comment #2) > (From update of attachment 59991 [details]) > Looks good. > > WebCore/bindings/v8/custom/V8BindingMacros.h:35 > + var = value; \ > It's usually a good idea to put parenthesis around (value). Otherwise you might get strange order-of-operations issues. > > WebCore/bindings/v8/custom/V8BindingMacros.h:44 > + v8::Handle<v8::String> v8String = value->ToString(); \ > It's usually a good idea to put parenthesis around (value). Otherwise you might get strange order-of-operations issues. good point! fixed.
Dumitru Daniliuc
Comment 4 2010-06-29 00:40:06 PDT
Landed: r62094.
WebKit Review Bot
Comment 5 2010-06-29 01:00:21 PDT
http://trac.webkit.org/changeset/62094 might have broken Qt Windows 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/62093 http://trac.webkit.org/changeset/62094 http://trac.webkit.org/changeset/62095
Note You need to log in before you can comment on or make changes to this bug.