Bug 30262 - [Chromium] KURLGoogle's protocolIs barfs on input containing hyphens
Summary: [Chromium] KURLGoogle's protocolIs barfs on input containing hyphens
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-09 14:54 PDT by Darin Fisher (:fishd, Google)
Modified: 2009-10-10 06:54 PDT (History)
2 users (show)

See Also:


Attachments
v1 patch (1.16 KB, patch)
2009-10-09 14:57 PDT, Darin Fisher (:fishd, Google)
eric: review-
Details | Formatted Diff | Diff
v2 patch (1.40 KB, patch)
2009-10-09 20:36 PDT, Darin Fisher (:fishd, Google)
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2009-10-09 14:54:50 PDT
[Chromium] KURLGoogle's protocolIs barfs on input containing hyphens
Comment 1 Darin Fisher (:fishd, Google) 2009-10-09 14:57:48 PDT
Created attachment 40965 [details]
v1 patch
Comment 2 Eric Seidel (no email) 2009-10-09 16:16:30 PDT
Comment on attachment 40965 [details]
v1 patch

Needs test case or explanation of why there is none in the ChangeLog.
Comment 3 Eric Seidel (no email) 2009-10-09 16:16:51 PDT
At least, I assume this is tesatable via a layout test?
Comment 4 Darin Fisher (:fishd, Google) 2009-10-09 19:48:54 PDT
(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);
Comment 5 Darin Fisher (:fishd, Google) 2009-10-09 20:36:04 PDT
Created attachment 40983 [details]
v2 patch

Now with a better solution and a better ChangeLog entry.
Comment 6 Darin Fisher (:fishd, Google) 2009-10-09 20:38:23 PDT
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.
Comment 7 Darin Fisher (:fishd, Google) 2009-10-09 20:54:39 PDT
FYI, unit test is here:  http://codereview.chromium.org/261058
Comment 8 Eric Seidel (no email) 2009-10-09 22:04:09 PDT
Comment on attachment 40983 [details]
v2 patch

LGTM.  Thanks.
Comment 9 Darin Fisher (:fishd, Google) 2009-10-10 06:54:31 PDT
Landed as http://trac.webkit.org/changeset/49416