Bug 30262 – [Chromium] KURLGoogle's protocolIs barfs on input containing hyphens

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Darin Fisher (:fishd, Google)]

[Chromium] KURLGoogle's protocolIs barfs on input containing hyphens

[Darin Fisher (:fishd, Google)]

Created attachment 40965 [details]
v1 patch

[Eric Seidel (no email)]

Comment on attachment 40965 [details]
v1 patch

Needs test case or explanation of why there is none in the ChangeLog.

[Eric Seidel (no email)]

At least, I assume this is tesatable via a layout test?

[Darin Fisher (:fishd, Google)]

(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);

[Darin Fisher (:fishd, Google)]

Created attachment 40983 [details]
v2 patch

Now with a better solution and a better ChangeLog entry.

[Darin Fisher (:fishd, Google)]

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.

[Darin Fisher (:fishd, Google)]

FYI, unit test is here:  http://codereview.chromium.org/261058

[Eric Seidel (no email)]

Comment on attachment 40983 [details]
v2 patch

LGTM.  Thanks.

[Darin Fisher (:fishd, Google)]

Landed as http://trac.webkit.org/changeset/49416