Bug 22750 – [gtk] webkit up to r39121 crash on image urls like http:///sitename.com

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Alexander Butenko]

attached a saved html on which i get repeatable crash with midori-git and webkit-svn. 

File is a wget'ed output of http://cgi.ebay.com/IBM-Lenovo-ThinkPad-X61-C2D-12-1-120GB-3G-HSPDA_W0QQitemZ270313235609QQcmdZViewItemQQptZLaptops_Nov05?hash=item270313235609&_trksid=p3286.c0.m14&_trkparms=72%3A1234%7C66%3A2%7C65%3A12%7C39%3A1%7C240%3A1318%7C301%3A1%7C293%3A1%7C294%3A50

webkit is built with ./configure --with-http-backend=soup --prefix=/usr/ --disable-svg --disable-geolocation --disable-video --enable-optimizations

[Alexander Butenko]

Created attachment 25870 [details]
test case

[Alexander Butenko]

Created attachment 25959 [details]
core from webkit-debug

[Alexander Butenko]

Created attachment 25960 [details]
gdb bt

[Alexander Butenko]

Created attachment 25961 [details]
gdb run

[Alexander Butenko]

ok. stripped down html source and found where is an issue.

Now, im getting stable crash on a simple html page like:
<img src=http:///google.com>

Three slashes in image url is a cause of a crash. 

[Alexander Butenko]

Created attachment 26234 [details]
new simplified test

[Alexander Butenko]

Created attachment 26238 [details]
patch which fixes a crash

[Holger Freyther]

Comment on attachment 26238 [details]
patch which fixes a crash

Please see http://webkit.org/coding/contributing.html and attach a new version with a ChangeLog and an applyable patch.



> --- ResourceHandleSoup.cpp	2008-12-24 13:16:59.000000000 -0400
> +++ ResourceHandleSoup.cpp.new	2008-12-24 13:18:07.000000000 -0400
> @@ -335,7 +335,8 @@
>  
>      if (equalIgnoringCase(protocol, "data"))
>          return startData(urlString);
> -    else if (equalIgnoringCase(protocol, "http") || equalIgnoringCase(protocol, "https"))
> +    else if ( (equalIgnoringCase(protocol, "http") || equalIgnoringCase(protocol, "https") ) &&
> +                SOUP_URI_VALID_FOR_HTTP(soup_uri_new(urlString.utf8().data())))
>          return startHttp(urlString);


why did you decide to do this check here and not within startHttp? Is there anyone else calling startHttp?

[Alexander Butenko]

the point of the check here is that once url will not be valid we will fallback to last 'else' to call didFail() without a code duplication. 

I will update patch shortly to include changelog.

[Alexander Butenko]

Created attachment 26243 [details]
updated patch

[Holger Freyther]

(In reply to comment #10)
> Created an attachment (id=26243) [review]

Okay, I'm convinced that this is the right patch. There is one minor style issue thiugh. Would you mind uploading a new patch without the extra whitespace in the if? WebKit.org has a rather strict CodingStyle.

[Alexander Butenko]

Created attachment 26269 [details]
coding style fix

[Alexander Butenko]

Created attachment 26270 [details]
typo

[Holger Freyther]

Comment on attachment 26270 [details]
typo

thanks.

[Holger Freyther]

Landed in r39528.