Bug 89468

crash log for WebKitTestRunner (pid 24658): STDOUT: <empty> STDERR: ASSERTION FAILED: url.isEmpty() || isSchemeFirstChar(url[0]) Source/WebCore/platform/KURL.cpp(315) : void WebCore::checkEncodedString(const WTF::String&) _ZN7WebCore4KURL5parseERKN3WTF6StringE+0x29 _ZN7WebCore4KURLC1ENS_18ParsedURLStringTagERKN3WTF6StringE+0x33

Created attachment 149544 [details] Patch

Comment on attachment 149544 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149544&action=review > Source/WebKit2/Shared/qt/WebCoreArgumentCodersQt.cpp:58 > - request.setURL(KURL(WebCore::ParsedURLString, url)); > + if (url.isNull()) > + request.setURL(KURL()); > + else > + request.setURL(KURL(WebCore::ParsedURLString, url)); It's not introduced by this patch, but it's not right to use this constructor form in argument decoders. For a message sent from WebProcess to UI Process, we cannot trust the content - it can well be malicious. ParsedURLString constructor can only be used when we know that the string came from KURL::string(), and the object was valid.

Created attachment 149725 [details] Patch

Comment on attachment 149725 [details] Patch Yeah, this seems to be in line with what the other ports are using. However at the same time this is an example of unnecessary code duplication between ports that could easily be cleaned up. Mac and Win implementations using CFNetwork appear to just encode and decode the entire underlying dictionary. Gtk, Efl and Qt ports are either just storing the URL or (in the case of the Gtk port) also more meta-data. I think it would be really nice to _share_ a non-CFNetwork based implementation that saves/restores more properties of ResourceRequestBase and leaves room for a platform specific properties in the ResourceRequest sub-class. Then issues like these are less likely to happen because there's more code coverage through the other ports.

(In reply to comment #5) > (From update of attachment 149725 [details]) > Yeah, this seems to be in line with what the other ports are using. > > However at the same time this is an example of unnecessary code duplication between ports that could easily be cleaned up. Mac and Win implementations using CFNetwork appear to just encode and decode the entire underlying dictionary. Gtk, Efl and Qt ports are either just storing the URL or (in the case of the Gtk port) also more meta-data. > > I think it would be really nice to _share_ a non-CFNetwork based implementation that saves/restores more properties of ResourceRequestBase and leaves room for a platform specific properties in the ResourceRequest sub-class. > > Then issues like these are less likely to happen because there's more code coverage through the other ports. Ok, than I'm going to land this for now and later on I will look into the idea of sharing this across ports.

Comment on attachment 149725 [details] Patch Clearing flags on attachment: 149725 Committed r121416: <http://trac.webkit.org/changeset/121416>

All reviewed patches have been landed. Closing bug.