After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 663451 - SoupHTTPInputStream/SoupRequestHTTP API changes
SoupHTTPInputStream/SoupRequestHTTP API changes
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: API
unspecified
Other All
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2011-11-05 13:31 UTC by Dan Winship
Modified: 2011-12-02 09:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
SoupHTTPInputStream: change handling of error responses (5.69 KB, patch)
2011-11-05 13:31 UTC, Dan Winship
committed Details | Review
SoupRequestHTTP: don't complete send() until after sniffing (10.70 KB, patch)
2011-11-05 13:31 UTC, Dan Winship
committed Details | Review
configure: bump to 2.37.2 for SoupRequestHTTP changes (1.12 KB, patch)
2011-11-05 13:31 UTC, Dan Winship
none Details | Review

Description Dan Winship 2011-11-05 13:31:40 UTC
Filing this here because we'll want to synchronize the libsoup and
WebKit commits.

This changes the way SoupHTTPInputStream and SoupRequestHTTP work to
make things simpler for WebKit.
Comment 1 Dan Winship 2011-11-05 13:31:42 UTC
Created attachment 200749 [details] [review]
SoupHTTPInputStream: change handling of error responses

Old behavior:

  - If the (initial) response status code is 3xx/401/407, and the
    message is successfully restarted, then just throw away the
    initial response.

  - If the (final) response status code is 2xx, then stream the body.

  - Otherwise, return a GError from soup_http_input_stream_send*(),
    and don't stream the body.

This worked OK for gvfs, which never cared about the bodies of 4xx
responses, but was problematic for WebKitGTK, which ended up needing
three separate (but interwoven) codepaths to handle all the cases.
(And still ended up handling 407 wrong.)

New behavior:

  - If the (initial) response status code is 3xx/401/407, and the
    message is successfully restarted, then just throw away the
    initial response.

  - The body of the (final) response is always streamed to the caller.

  - A GError is only returned on transport errors, not on 4xx/5xx
    responses

(This is an API break, but SoupHTTPInputStream is not API-stable.)
Comment 2 Dan Winship 2011-11-05 13:31:45 UTC
Created attachment 200750 [details] [review]
SoupRequestHTTP: don't complete send() until after sniffing

If the session has a SoupContentSniffer (and it's not disabled for
this message), wait until content-sniffed is emitted before completing
send()/send_async(). Remove the faked content-sniffed emissions from
the cache codepaths, since they should no longer be needed.
Comment 3 Dan Winship 2011-11-05 13:31:48 UTC
Created attachment 200751 [details] [review]
configure: bump to 2.37.2 for SoupRequestHTTP changes
Comment 4 Sergio Villar 2011-11-07 18:19:42 UTC
(In reply to comment #1)
> Created an attachment (id=200749) [details] [review]
> SoupHTTPInputStream: change handling of error responses

Yeah, I have a local branch that does more or less the same but I never managed to get it fully working with WebKit tests. Next time I'll try to push it to bz sooner in order to avoid work duplication.

> This worked OK for gvfs, which never cared about the bodies of 4xx
> responses, but was problematic for WebKitGTK, which ended up needing
> three separate (but interwoven) codepaths to handle all the cases.
> (And still ended up handling 407 wrong.)

Indeed we were not getting message bodies.
 
>   - A GError is only returned on transport errors, not on 4xx/5xx
>     responses

That was definitely wrong and causing some weird errors in webkitgtk.
Comment 5 Sergio Villar 2011-11-07 18:37:19 UTC
And this is a bit unrelated, but I think we should get rid of the pause/unpause message stuff from the streams. I remember adding them (actually to get webkitgtk+ working), but I don't see any reason why we should keep them if the stream works as it should. 

On the contrary, I prefer to discard network data if the client decides not to provide a buffer than to loose a precious amount of time in paused state instead of being getting data from the network.
Comment 6 Dan Winship 2011-12-02 09:28:22 UTC
pushed to git, with 2.37.2.1 for the libsoup version number, since
2.37.2 already shipped