Bug 106789 – [SOUP] work around a glib bug exposed by the new redirect code

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Dan Winship]

qv https://bugzilla.gnome.org/show_bug.cgi?id=691489

Since it will presumably be a while before we depend on a new-enough glib to have the fix, work around this by using read_async() rather than skip_async().

(I'm not 100% sure that this completely fixes the bug there, because I had a hard time reproducing it reliably. So, testing welcome. Note that if you're testing with epiphany, you need to rm -rf ~/.cache/epiphany/ after it crashes.)

[Dan Winship]

Created attachment 182567 [details]
patch to use read_async instead of skip_async

(intentionally not setting cq? because I'd like people to test it and confirm that it does help first...)

[Martin Robinson]

Comment on attachment 182567 [details]
patch to use read_async instead of skip_async

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

Looks reasonable to me.

> Source/WebCore/ChangeLog:5
> +        https://bugzilla.gnome.org/show_bug.cgi?id=691489

I'm a little worried that this might confuse the tools which parse ChangeLogs, so probably better to put this into the description.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:671
> +            // This can switch to using skip_async after we depend on glib > 2.35.4

Probably good to put a link to the bug.

[Dan Winship]

Created attachment 182763 [details]
updated for comments

This removes the bgo URL form ChangeLog and adds it to ResourceHandleSoup.cpp.


If we're going to put out a point release to deal with bug 106903 we should probably get this in too.

[Dan Winship]

Comment on attachment 182763 [details]
updated for comments

ok, well, not sure anyone is actually testing this, but at least no one has said "i tested with it and it didn't fix the bug", so let's commit it

[WebKit Review Bot]

Comment on attachment 182763 [details]
updated for comments

Clearing flags on attachment: 182763

Committed r140338: <http://trac.webkit.org/changeset/140338>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Sergio Villar Senin]

Dan I think we need to revert it. The current libsoup stack does not handle the G_MAXSIZE value for count in read_async as it tries to create a buffer of that size.

[Sergio Villar Senin]

(In reply to comment #7)
> Dan I think we need to revert it. The current libsoup stack does not handle the G_MAXSIZE value for count in read_async as it tries to create a buffer of that size.

The problem is actually located in read_internal() in gconverterinputstream.c the first time it tries to read something from the underlying string.

[Dan Winship]

ugh, the G_MAXSSIZE should have changed to READ_BUFFER_SIZE... The code now would be a buffer overrun if someone returned a redirect response that was larger than READ_BUFFER_SIZE. (Which the test suite never does... actually, that might be a good test...)

[Alberto Garcia]

This has already been reverted in http://trac.webkit.org/changeset/151013

Since we depend on a glib version that doesn't have this bug we can close this.