30262 2009-10-09 14:54:50 -0700 [Chromium] KURLGoogle's protocolIs barfs on input containing hyphens 2009-10-10 06:54:31 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fishd fishd brettw eric oldest_to_newest 153767 0 fishd 2009-10-09 14:54:50 -0700 [Chromium] KURLGoogle's protocolIs barfs on input containing hyphens 153769 1 40965 fishd 2009-10-09 14:57:48 -0700 Created attachment 40965 v1 patch 153795 2 40965 eric 2009-10-09 16:16:30 -0700 Comment on attachment 40965 v1 patch Needs test case or explanation of why there is none in the ChangeLog. 153796 3 eric 2009-10-09 16:16:51 -0700 At least, I assume this is tesatable via a layout test? 153835 4 fishd 2009-10-09 19:48:54 -0700 (In reply to comment #3) > At least, I assume this is tesatable via a layout test? This assertion was getting hit via a Chrome UI test when I switched some code in Chromium's webkit/glue from using GURL to KURL. I think the assertion is plainly bogus. This is a case where a unit test would be superior to a layout test IMO since you'd be able to be certain that you are executing this code path. Also, I realized that this change would probably be better: - ASSERT(isASCIILower(*str)); + ASSERT(toASCIILower(*str) == *str); 153841 5 40983 fishd 2009-10-09 20:36:04 -0700 Created attachment 40983 v2 patch Now with a better solution and a better ChangeLog entry. 153842 6 fishd 2009-10-09 20:38:23 -0700 Unfortunately, I could not find a way to cause this code path to be reached with input that contains a hyphen. It is a code path that can only be reached in Chrome due to the way it uses WebCore. I'll add something to GKURL_unittest.cpp, which I plan to one day upstream to svn.webkit.org. 153844 7 fishd 2009-10-09 20:54:39 -0700 FYI, unit test is here: http://codereview.chromium.org/261058 153847 8 40983 eric 2009-10-09 22:04:09 -0700 Comment on attachment 40983 v2 patch LGTM. Thanks. 153872 9 fishd 2009-10-10 06:54:31 -0700 Landed as http://trac.webkit.org/changeset/49416 40965 2009-10-09 14:57:48 -0700 2009-10-09 20:36:04 -0700 v1 patch kurl_1.txt text/plain 1192 fishd SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gV2ViQ29yZS9DaGFuZ2VM b2cJKHJldmlzaW9uIDQ5NDA0KQorKysgV2ViQ29yZS9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkK QEAgLTEsMyArMSwxMyBAQAorMjAwOS0xMC0wOSAgRGFyaW4gRmlzaGVyICA8ZGFyaW5AY2hyb21p dW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFtDaHJvbWl1bV0gS1VSTEdvb2dsZSdzIHByb3RvY29sSXMgYmFyZnMgb24gaW5wdXQgY29udGFp bmluZyBoeXBoZW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zMDI2MgorCisgICAgICAgICogcGxhdGZvcm0vS1VSTEdvb2dsZS5jcHA6CisgICAgICAg IChXZWJDb3JlOjpsb3dlckNhc2VFcXVhbHNBU0NJSSk6IG9ubHkgYXNzZXJ0IGlzQVNDSUlMb3dl ciBpZiBpc0FTQ0lJQWxwaGEKKwogMjAwOS0xMC0wOSAgSmVucyBBbGZrZSAgPHNuZWpAY2hyb21p dW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpJbmRleDogV2ViQ29y ZS9wbGF0Zm9ybS9LVVJMR29vZ2xlLmNwcA0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIFdlYkNvcmUvcGxhdGZv cm0vS1VSTEdvb2dsZS5jcHAJKHJldmlzaW9uIDQ5NDA0KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9L VVJMR29vZ2xlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTA2LDcgKzEwNiw3IEBAIHN0YXRpYyBp bmxpbmUgYm9vbCBpc1VuaWNvZGVFbmNvZGluZyhjb24KIHN0YXRpYyBib29sIGxvd2VyQ2FzZUVx dWFsc0FTQ0lJKGNvbnN0IGNoYXIqIGJlZ2luLCBjb25zdCBjaGFyKiBlbmQsIGNvbnN0IGNoYXIq IHN0cikKIHsKICAgICB3aGlsZSAoYmVnaW4gIT0gZW5kICYmICpzdHIpIHsKLSAgICAgICAgQVNT RVJUKGlzQVNDSUlMb3dlcigqc3RyKSk7CisgICAgICAgIEFTU0VSVCghaXNBU0NJSUFscGhhKCpz dHIpIHx8IGlzQVNDSUlMb3dlcigqc3RyKSk7CiAgICAgICAgIGlmICh0b0FTQ0lJTG93ZXIoKmJl Z2luKyspICE9ICpzdHIrKykKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKICAgICB9Cg== 40983 2009-10-09 20:36:04 -0700 2009-10-09 22:04:09 -0700 v2 patch kurl_2.txt text/plain 1437 fishd SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gV2ViQ29yZS9DaGFuZ2VM b2cJKHJldmlzaW9uIDQ5NDEzKQorKysgV2ViQ29yZS9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkK QEAgLTEsMyArMSwxOCBAQAorMjAwOS0xMC0wOSAgRGFyaW4gRmlzaGVyICA8ZGFyaW5AY2hyb21p dW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFtDaHJvbWl1bV0gS1VSTEdvb2dsZSdzIHByb3RvY29sSXMgYmFyZnMgb24gaW5wdXQgY29udGFp bmluZyBoeXBoZW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zMDI2MgorCisgICAgICAgIFRoaXMgaXMgbm90IHRlc3RhYmxlIGZyb20gV2ViS2l0IHNp bmNlIFdlYktpdCBuZXZlciB1c2VzIHRoZSBtZW1iZXIKKyAgICAgICAgZnVuY3Rpb24gZm9ybSBv ZiBwcm90b2NvbElzIHdpdGggaW5wdXQgdGhhdCBjYW4gYmUgY29udHJvbGxlZCBmcm9tIGEKKyAg ICAgICAgd2ViIHBhZ2UuICBJdCBhbHdheXMgcGFzc2VzIHN0cmluZyBsaXRlcmFscyBsYWNraW5n IGh5cGhlbnMuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9LVVJMR29vZ2xlLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6Omxvd2VyQ2FzZUVxdWFsc0FTQ0lJKTogT25seSBhc3NlcnQgaWYgdG9BU0NJSUxv d2VyIHdvdWxkCisgICAgICAgIHRyYW5zZm9ybSB0aGUgaW5wdXQgY2hhcmFjdGVyLgorCiAyMDA5 LTEwLTA5ICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgRGFyaW4gQWRsZXIuCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL0tVUkxHb29nbGUuY3Bw DQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09DQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9LVVJMR29vZ2xlLmNwcAkocmV2aXNp b24gNDk0MTMpCisrKyBXZWJDb3JlL3BsYXRmb3JtL0tVUkxHb29nbGUuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xMDYsNyArMTA2LDcgQEAgc3RhdGljIGlubGluZSBib29sIGlzVW5pY29kZUVuY29k aW5nKGNvbgogc3RhdGljIGJvb2wgbG93ZXJDYXNlRXF1YWxzQVNDSUkoY29uc3QgY2hhciogYmVn aW4sIGNvbnN0IGNoYXIqIGVuZCwgY29uc3QgY2hhciogc3RyKQogewogICAgIHdoaWxlIChiZWdp biAhPSBlbmQgJiYgKnN0cikgewotICAgICAgICBBU1NFUlQoaXNBU0NJSUxvd2VyKCpzdHIpKTsK KyAgICAgICAgQVNTRVJUKHRvQVNDSUlMb3dlcigqc3RyKSA9PSAqc3RyKTsKICAgICAgICAgaWYg KHRvQVNDSUlMb3dlcigqYmVnaW4rKykgIT0gKnN0cisrKQogICAgICAgICAgICAgcmV0dXJuIGZh bHNlOwogICAgIH0K