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 703297 - remote socket closure during chunked write causes warning
remote socket closure during chunked write causes warning
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: HTTP Transport
2.42.x
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2013-06-29 04:07 UTC by Olivier Crête
Modified: 2013-08-25 21:08 UTC
See Also:
GNOME target: ---
GNOME version: 3.7/3.8


Attachments
soup-body-output-stream: Ignore write errors when closing the socket (1.40 KB, patch)
2013-06-29 04:07 UTC, Olivier Crête
needs-work Details | Review

Description Olivier Crête 2013-06-29 04:07:32 UTC
Created attachment 248041 [details] [review]
soup-body-output-stream: Ignore write errors when closing the  socket

The bug is the following:

1. write in chunked mode
2. have the other side disconnect in the middle of writing a chunk
3. the write will fail
4. libsoup will try to close the iostream
5. inside the close_fn() it tries to call write_chunk() with buffer=NULL, this
is not good

solution:
1. ignore the buffer if it's NULL
2. Ignore any chunk writing errors in close, we're getting rid of the socket anyway I guess

With patch


Stack trace
  • #0 g_logv
    at gmessages.c line 981
  • #1 g_log
    at gmessages.c line 1010
  • #2 g_return_if_fail_warning
    at gmessages.c line 1019
  • #3 g_output_stream_write
    at goutputstream.c line 195
  • #4 soup_body_output_stream_write_chunked
    at soup-body-output-stream.c line 175
  • #5 soup_body_output_stream_close_fn
    at soup-body-output-stream.c line 235
  • #6 _g_output_stream_close_internal
    at goutputstream.c line 555
  • #7 g_output_stream_close
    at goutputstream.c line 626
  • #8 g_output_stream_dispose
    at goutputstream.c line 116
  • #9 g_filter_output_stream_dispose
    at gfilteroutputstream.c line 180
  • #10 g_object_unref
    at gobject.c line 2987
  • #11 soup_message_io_cleanup
    at soup-message-io.c line 112
  • #12 soup_message_io_finished
    at soup-message-io.c line 163
  • #13 io_run
    at soup-message-io.c line 939
  • #14 g_main_dispatch
    at gmain.c line 3054
  • #15 g_main_context_dispatch
    at gmain.c line 3630
  • #16 g_main_context_iterate
    at gmain.c line 3701
  • #17 g_main_loop_run
    at gmain.c line 3895
  • #18 run
    at gmsd.c line 199
  • #19 run_foreground
    at gmsd.c line 544
  • #20 main
    at gmsd.c line 657

Comment 1 Dan Winship 2013-06-29 16:40:28 UTC
Comment on attachment 248041 [details] [review]
soup-body-output-stream: Ignore write errors when closing the  socket

This isn't quite right. A chunked stream is terminated by a 0-length chunk, and that's what the call to soup_body_output_stream_write_chunked() in soup_body_output_stream_close_fn() is for. However, it was implicitly assuming that chunked_state was CHUNK_SIZE (in which case @buffer never gets looked at), which is what it would be in any non-error state.

So what we want is to write the final chunk, with error-checking, if everything is OK, but skip that if not. I was going to say we could do that by checking if chunked_state was CHUNK_SIZE, but that's actually not quite right, because the write could return G_IO_ERROR_WOULD_BLOCK while it's in TRAILERS or DONE. Not that the code actually deals with that case correctly anyway. Ugh.

OK, quick fix is to only do the final chunk write if state is CHUNK_SIZE when we call close_fn(). Does that fix it for you?
Comment 2 Olivier Crête 2013-06-29 17:20:37 UTC
That would work I guess, that said, I think you should also ignore errors in calls to write_chunked() inside close_fn()
Comment 3 Dan Winship 2013-08-25 21:08:59 UTC
fixed in master (and eventually 2.43.91)