89468 2012-06-19 07:33:10 -0700 [Qt] KURL assert at fast/loader/opaque-base-url.html 2012-06-28 00:47:11 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED LayoutTestFailure, Qt, QtTriaged P2 Normal --- 79668 1 kbalazs kbalazs ap cmarcelo menard webkit.review.bot zoltan oldest_to_newest 652435 0 kbalazs 2012-06-19 07:33:10 -0700 Local debug test session, desktop Ubuntu 11.10, WK2 652436 1 kbalazs 2012-06-19 07:33:28 -0700 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 657648 2 149544 kbalazs 2012-06-26 09:31:36 -0700 Created attachment 149544 Patch 658310 3 149544 ap 2012-06-26 21:04:46 -0700 Comment on attachment 149544 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. 658526 4 149725 kbalazs 2012-06-27 04:27:45 -0700 Created attachment 149725 Patch 659201 5 149725 hausmann 2012-06-27 22:28:53 -0700 Comment on attachment 149725 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. 659303 6 kbalazs 2012-06-28 00:45:02 -0700 (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. 659304 7 149725 kbalazs 2012-06-28 00:47:03 -0700 Comment on attachment 149725 Patch Clearing flags on attachment: 149725 Committed r121416: <http://trac.webkit.org/changeset/121416> 659305 8 kbalazs 2012-06-28 00:47:11 -0700 All reviewed patches have been landed. Closing bug. 149544 2012-06-26 09:31:36 -0700 2012-06-27 04:27:14 -0700 Patch bug-89468-20120626183124.patch text/plain 2116 kbalazs U3VidmVyc2lvbiBSZXZpc2lvbjogMTIxMTg0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDhjOTc0YmRjMTVjNDgz YTFkNjE0YmM4OGUwY2UwZGEwNjU3NjRjNC4uMjJiNGY4M2RjNTUzZWIwZmJmNjNmN2QwMGQ1Zjk0 NmQ0NDM0MTFkZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA2LTI2ICBCYWxh enMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9yZz4KKworICAgICAgICBbUXRdIEtVUkwgYXNz ZXJ0IGF0IGZhc3QvbG9hZGVyL29wYXF1ZS1iYXNlLXVybC5odG1sCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTQ2OAorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEhhbmRsZSBpbnZhbGlkIFVSTCdzIGNvcnJl Y3RseS4KKyAgICAgICAgS1VSTChQYXJzZWRVUkxTdHJpbmdUYWcsIGNvbnN0IFN0cmluZyYpIGV4 cGVjdCB0aGUgc3RyaW5nCisgICAgICAgIGlzIHRoZSBvdXRwdXQgb2YgS1VSTDo6c3RyaW5nKCkg Y2FsbGVkIG9uIGEgdmFsaWQgb2JqZWN0LgorICAgICAgICAqIFNoYXJlZC9xdC9XZWJDb3JlQXJn dW1lbnRDb2RlcnNRdC5jcHA6CisgICAgICAgIChDb3JlSVBDOjo6OmVuY29kZSk6CisgICAgICAg IChDb3JlSVBDOjo6OmRlY29kZSk6CisKIDIwMTItMDYtMjUgIFlvdW5nVGFlY2sgU29uZyAgPHlv dW5ndGFlY2suc29uZ0BzYW1zdW5nLmNvbT4KIAogICAgICAgICBbV0syXVtRdF1bRUZMXSBNb3Zl IExheWVyQmFja2luZ1N0b3JlIGludG8gY29tbW9uIGRpcmVjdG9yeQpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYktpdDIvU2hhcmVkL3F0L1dlYkNvcmVBcmd1bWVudENvZGVyc1F0LmNwcCBiL1NvdXJj ZS9XZWJLaXQyL1NoYXJlZC9xdC9XZWJDb3JlQXJndW1lbnRDb2RlcnNRdC5jcHAKaW5kZXggN2E3 N2VmMzBmN2IxOTQ2M2M5OTM1ZjY2NGIyZjU2ZDkxZDlmZWFmYy4uM2E4NzNjYmZjYjI3ZTBlYzBi NTFhMDgwZThjMDE2ZWUyMTRmZjg0ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvU2hhcmVk L3F0L1dlYkNvcmVBcmd1bWVudENvZGVyc1F0LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9TaGFy ZWQvcXQvV2ViQ29yZUFyZ3VtZW50Q29kZXJzUXQuY3BwCkBAIC0zNiw3ICszNiwxMSBAQCBuYW1l c3BhY2UgQ29yZUlQQyB7CiAKIHZvaWQgQXJndW1lbnRDb2RlcjxSZXNvdXJjZVJlcXVlc3Q+Ojpl bmNvZGUoQXJndW1lbnRFbmNvZGVyKiBlbmNvZGVyLCBjb25zdCBSZXNvdXJjZVJlcXVlc3QmIHJl c291cmNlUmVxdWVzdCkKIHsKLSAgICBlbmNvZGVyLT5lbmNvZGUocmVzb3VyY2VSZXF1ZXN0LnVy bCgpLnN0cmluZygpKTsKKyAgICBjb25zdCBLVVJMJiB1cmwgPSByZXNvdXJjZVJlcXVlc3QudXJs KCk7CisgICAgaWYgKHVybC5pc1ZhbGlkKCkpCisgICAgICAgIGVuY29kZXItPmVuY29kZSh1cmwu c3RyaW5nKCkpOworICAgIGVsc2UKKyAgICAgICAgZW5jb2Rlci0+ZW5jb2RlKFN0cmluZygpKTsK IH0KIAogYm9vbCBBcmd1bWVudENvZGVyPFJlc291cmNlUmVxdWVzdD46OmRlY29kZShBcmd1bWVu dERlY29kZXIqIGRlY29kZXIsIFJlc291cmNlUmVxdWVzdCYgcmVzb3VyY2VSZXF1ZXN0KQpAQCAt NDgsNyArNTIsMTAgQEAgYm9vbCBBcmd1bWVudENvZGVyPFJlc291cmNlUmVxdWVzdD46OmRlY29k ZShBcmd1bWVudERlY29kZXIqIGRlY29kZXIsIFJlc291cmNlUmUKICAgICBTdHJpbmcgdXJsOwog ICAgIGlmICghZGVjb2Rlci0+ZGVjb2RlKHVybCkpCiAgICAgICAgIHJldHVybiBmYWxzZTsKLSAg ICByZXF1ZXN0LnNldFVSTChLVVJMKFdlYkNvcmU6OlBhcnNlZFVSTFN0cmluZywgdXJsKSk7Cisg ICAgaWYgKHVybC5pc051bGwoKSkKKyAgICAgICAgcmVxdWVzdC5zZXRVUkwoS1VSTCgpKTsKKyAg ICBlbHNlCisgICAgICAgIHJlcXVlc3Quc2V0VVJMKEtVUkwoV2ViQ29yZTo6UGFyc2VkVVJMU3Ry aW5nLCB1cmwpKTsKIAogICAgIHJlc291cmNlUmVxdWVzdCA9IHJlcXVlc3Q7CiAgICAgcmV0dXJu IHRydWU7Cg== 149725 2012-06-27 04:27:45 -0700 2012-06-28 00:47:03 -0700 Patch bug-89468-20120627132730.patch text/plain 1764 kbalazs U3VidmVyc2lvbiBSZXZpc2lvbjogMTIxMTg0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDhjOTc0YmRjMTVjNDgz YTFkNjE0YmM4OGUwY2UwZGEwNjU3NjRjNC4uYWFlZTM1Mzc0OTIzZjQxMzllZDkwN2FiZjMwYTMz ZWFlZjljN2I0YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA2LTI3ICBCYWxh enMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9yZz4KKworICAgICAgICBbUXRdIEtVUkwgYXNz ZXJ0IGF0IGZhc3QvbG9hZGVyL29wYXF1ZS1iYXNlLXVybC5odG1sCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTQ2OAorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIERvbid0IHVzZSB0aGUgS1VSTChQYXJzZWRV UkxTdHJpbmdUYWcsIGNvbnN0IFN0cmluZyYpIGNvbnN0cnVjdG9yLgorICAgICAgICBXZSBjYW5u b3QgYmUgc3VyZSB0aGF0IHRoZSB1cmwgaW4gZW5jb2RlIHdhcyB2YWxpZCBhbmQgZXZlbiBpZiBp dAorICAgICAgICB3YXMgdGhlIG1lc3NhZ2UgY291bGQgaGF2ZSBiZWVuIGNvcnJ1cHRlZC4KKyAg ICAgICAgKiBTaGFyZWQvcXQvV2ViQ29yZUFyZ3VtZW50Q29kZXJzUXQuY3BwOgorICAgICAgICAo Q29yZUlQQzo6OjplbmNvZGUpOgorICAgICAgICAoQ29yZUlQQzo6OjpkZWNvZGUpOgorCiAyMDEy LTA2LTI1ICBZb3VuZ1RhZWNrIFNvbmcgIDx5b3VuZ3RhZWNrLnNvbmdAc2Ftc3VuZy5jb20+CiAK ICAgICAgICAgW1dLMl1bUXRdW0VGTF0gTW92ZSBMYXllckJhY2tpbmdTdG9yZSBpbnRvIGNvbW1v biBkaXJlY3RvcnkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9xdC9XZWJDb3Jl QXJndW1lbnRDb2RlcnNRdC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9TaGFyZWQvcXQvV2ViQ29yZUFy Z3VtZW50Q29kZXJzUXQuY3BwCmluZGV4IDdhNzdlZjMwZjdiMTk0NjNjOTkzNWY2NjRiMmY1NmQ5 MWQ5ZmVhZmMuLmFkNTU3NGY4Zjg4YzM2OTE5OTYyZWUyNzNjOTNkODU0MDg5OTNmNGQgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9xdC9XZWJDb3JlQXJndW1lbnRDb2RlcnNRdC5j cHAKKysrIGIvU291cmNlL1dlYktpdDIvU2hhcmVkL3F0L1dlYkNvcmVBcmd1bWVudENvZGVyc1F0 LmNwcApAQCAtNDQsMTMgKzQ0LDEwIEBAIGJvb2wgQXJndW1lbnRDb2RlcjxSZXNvdXJjZVJlcXVl c3Q+OjpkZWNvZGUoQXJndW1lbnREZWNvZGVyKiBkZWNvZGVyLCBSZXNvdXJjZVJlCiAgICAgLy8g RklYTUU6IEFkZCAqbW9yZSogY29kaW5nIGltcGxlbWVudGF0aW9uIHdoZW4gd2Ugd2FudCB0byBp bXBsZW1lbnQgc29tZXRoaW5nIHRoYXQKICAgICAvLyBkZXBlbmRzIG9uIHRoaXMgbGlrZSB0aGUg cG9saWN5IGNsaWVudC4KIAotICAgIFJlc291cmNlUmVxdWVzdCByZXF1ZXN0OwogICAgIFN0cmlu ZyB1cmw7CiAgICAgaWYgKCFkZWNvZGVyLT5kZWNvZGUodXJsKSkKICAgICAgICAgcmV0dXJuIGZh bHNlOwotICAgIHJlcXVlc3Quc2V0VVJMKEtVUkwoV2ViQ29yZTo6UGFyc2VkVVJMU3RyaW5nLCB1 cmwpKTsKLQotICAgIHJlc291cmNlUmVxdWVzdCA9IHJlcXVlc3Q7CisgICAgcmVzb3VyY2VSZXF1 ZXN0LnNldFVSTChLVVJMKEtVUkwoKSwgdXJsKSk7CiAgICAgcmV0dXJuIHRydWU7CiB9CiAK