Bug 26658

Summary: Gabble asserts if the UI sends data before the FT channel is OPEN
Product: Telepathy Reporter: Youness Alaoui <youness.alaoui>
Component: gabbleAssignee: Youness Alaoui <youness.alaoui>
Status: CLOSED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: youness.alaoui
Version: git master   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/kakaroto/telepathy-gabble.git;a=shortlog;h=refs/heads/misc
Whiteboard:
i915 platform: i915 features:
Attachments: unit test for sending data before the peer accepts the FT

Description Youness Alaoui 2010-02-19 11:33:23 UTC
Hi,

I noticed there's a bug in gabble where it would be using a variable before it's assigned a value (self->priv->bytestream) in ft-channel.c
This could happen if the UI connects and starts sending data over the UI<->gabble socket after it calls ProvideFile and before the FT channel's state becomes OPEN.

The transport_handler (called when we receive data on the socket between gabble and UI), calls gabble_bytestream_iface_send and uses the self->priv->bytestream. That variable is only set when we call set_bytestream which is called in the callback of gabble_bytestream_factory_negotiate_stream which itself is only called after the remote accepts our FT. So as long as the peer doesn't accept our stream, bytestream is NULL.

Attached is a unit test to be added in the tests/twisted/file-transfer directory (and adding it to the Makefile.am). This will send the data, then accept the file and continue.. It will eventually timeout (if it doesn't throw a 'Broken Pipe' exception during the send) and you can look at the log to see this :
GLib-GObject-WARNING **: 2010-02-19 14:28:22.982356: invalid (NULL) pointer instance
aborting...

I will soon provide a patch for this as it seems easy to fix.
Comment 1 Youness Alaoui 2010-02-19 11:34:16 UTC
Created attachment 33434 [details]
unit test for sending data before the peer accepts the FT

attachment failed, reattaching.
Comment 2 Youness Alaoui 2010-02-19 12:23:02 UTC
I've fixed the bug and added the new unit test to the test suite (with a small change compared to the attached version).
You can find it here : 
http://git.collabora.co.uk/?p=user/kakaroto/telepathy-gabble.git;a=shortlog;h=refs/heads/misc

Waiting for review/merge.
Comment 3 Youness Alaoui 2010-02-19 12:30:40 UTC
reopening bug until it gets reviewed/merged.
Comment 4 Youness Alaoui 2010-02-19 12:35:03 UTC
setting url and reassigning to self
Comment 5 Youness Alaoui 2010-02-19 16:49:44 UTC
Merged into masted! Thanks sjoerd for the review
Comment 6 Youness Alaoui 2010-02-19 16:49:55 UTC
Closing bug

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.