Bug 88109

Summary: WebFrame::_stringByEvaluatingJavaScriptFromString methods don't handle nil string
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: WebKit Misc.Assignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ggaren: review-
Patch with fix ggaren: review+

Michael Saboff
Reported 2012-06-01 10:47:25 PDT
The various flavors of WebFrame::_stringByEvaluatingJavaScriptFromString don't properly handle nil string. Instead such nil strings are passed down to the JavaScript engine where it may be a crash. An if (!string) return @"": should be added as appropriate.
Attachments
Patch (3.40 KB, patch)
2012-06-01 11:52 PDT, Michael Saboff
ggaren: review-
Patch with fix (3.41 KB, patch)
2012-06-01 13:34 PDT, Michael Saboff
ggaren: review+
Michael Saboff
Comment 1 2012-06-01 11:52:01 PDT
Simon Fraser (smfr)
Comment 2 2012-06-01 12:29:16 PDT
Comment on attachment 145346 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145346&action=review > Tools/TestWebKitAPI/Tests/mac/StringByEvaluatingJavaScriptFromString.mm:60 > + EXPECT_EQ(nil, result); Don't you expect an empty string, not a nil string?
Geoffrey Garen
Comment 3 2012-06-01 12:37:24 PDT
Comment on attachment 145346 [details] Patch Expectation should be empty string. Does nil pass?
Michael Saboff
Comment 4 2012-06-01 13:34:11 PDT
Created attachment 145369 [details] Patch with fix (In reply to comment #3) > (From update of attachment 145346 [details]) > Expectation should be empty string. Does nil pass? I was thinking about this as I was out at lunch, realizing that the test was wrong. The test was "passing", but shouldn't. I was assuming that run-test-webkit-api built the tests before running. Therefore I wasn't testing the new code. Built and tested this patch. It works as expected.
Geoffrey Garen
Comment 5 2012-06-01 14:14:24 PDT
Comment on attachment 145369 [details] Patch with fix r=me
Michael Saboff
Comment 6 2012-06-01 14:26:29 PDT
Radar WebKit Bug Importer
Comment 7 2012-06-07 17:07:16 PDT
Note You need to log in before you can comment on or make changes to this bug.