Bug 112965 - Prefer 'KURL(ParsedURLString, String)' when dealing with known-good data.
Summary: Prefer 'KURL(ParsedURLString, String)' when dealing with known-good data.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike West
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-21 14:33 PDT by Mike West
Modified: 2013-04-05 03:04 PDT (History)
5 users (show)

See Also:

Attachments
Patch (5.07 KB, patch)
2013-03-21 14:39 PDT, Mike West 		no flags Details | Formatted Diff | Diff
Patch for landing (5.02 KB, patch)
2013-03-22 01:46 PDT, Mike West 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mike West 2013-03-21 14:33:47 PDT 
Prefer 'KURL(ParsedURLString, String)' when dealing with known-good data.
Comment 1 Mike West 2013-03-21 14:39:30 PDT 
Created attachment 194340 [details]
Patch
Comment 2 Alexey Proskuryakov 2013-03-21 20:00:11 PDT 
Comment on attachment 194340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194340&action=review

We still don't have a class that wraps a known good string without KURL size overhead?

> Source/WebCore/page/SecurityOrigin.cpp:532
> +    return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port)));

This isn't normalized to append a slash? Please watch KURL based debug testers.

> Source/WebCore/page/SecurityOrigin.cpp:540
> +    return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port)));

Ditto.
Comment 3 Mike West 2013-03-22 01:46:10 PDT 
(In reply to comment #2)
> (From update of attachment 194340 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194340&action=review
> 
> We still don't have a class that wraps a known good string without KURL size overhead?
> 
> > Source/WebCore/page/SecurityOrigin.cpp:532
> > +    return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port)));
> 
> This isn't normalized to append a slash? Please watch KURL based debug testers.
> 
> > Source/WebCore/page/SecurityOrigin.cpp:540
> > +    return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port)));
> 
> Ditto.

Good eye, thank you. I've added a trailing slash to both, which I think is indeed necessary. I'll CQ the patch, but keep my finger on the revert button if it chokes a bot.
Comment 4 Mike West 2013-03-22 01:46:59 PDT 
Created attachment 194471 [details]
Patch for landing
Comment 5 WebKit Review Bot 2013-03-22 02:23:13 PDT 
Comment on attachment 194471 [details]
Patch for landing

Clearing flags on attachment: 194471

Committed r146580: <http://trac.webkit.org/changeset/146580>
Comment 6 WebKit Review Bot 2013-03-22 02:23:16 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Alexey Proskuryakov 2013-04-04 14:33:52 PDT 
This has caused bug 113961.
Comment 8 Mike West 2013-04-05 03:04:58 PDT 
(In reply to comment #7)
> This has caused bug 113961.

Thank you for fixing the crash, and apologies that I missed that case. :/