Summary: | Do not append trailing slash to urls with no path for KURL::parse | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Treat <manyoso> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, darin, staikos, zimmermann | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Adam Treat
2009-01-28 20:16:25 PST
Created attachment 27136 [details]
Do not append trailing slash in parse
Remove the append of the trailing slash.
The other option is to remove or edit the assert to take into account the trailing slash I suppose. Where is the single argument constructor called from? If it's called with a string that is not already an output of KURL::parse algorithm, then it is likely that an arbitrary string can be passed via this code path, so a two-argument constructor should be used. This assertion usually fires on seemingly innocuous things like this one, but uncovers real bugs at call sites nonetheless. Comment on attachment 27136 [details]
Do not append trailing slash in parse
Marking r- for now, because it's more likely to be a bug at call site.
It is being called from KURLQt.cpp for the QUrl conversion constructor. Perhaps I should add a special new constructor to KURL for usage by PLATFORM(QT) that will forgo the parsing entirely since QUrl already accomplishes this? For posterity: as discussed on IRC, I think that if any constructor were to expect QUrl output to be properly encoded, then KURL::parse should be replaced with an appropriate QUrl method to avoid surprises during re-parsing. For now, it's probably best to just use two-argument KURL constructor with a null base. Created attachment 27148 [details]
Makes KURLQt.cpp use two-arg ctor
After discussion with ap it was determined that KURLQt should use the two-arg ctor for implicit conversion of QUrl to KURL.
Comment on attachment 27148 [details]
Makes KURLQt.cpp use two-arg ctor
r=me
|