| Summary: | MUC bytestreams may not check if data is successfully sent? | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Will Thompson <will> |
| Component: | gabble | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED MOVED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | git master | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | libreoffice-tubes | ||
| i915 platform: | i915 features: | ||
This branch <http://cgit.collabora.com/git/user/wjt/telepathy-gabble/log/?h=47999-handle-ibb-errors> makes the bytestream close up if a stanza gets dropped. There's no test for this in the branch, because I know Jonny has been gutting the tube tests of references to the old API and I don't want to conflict. I tested this by making the test always send back errors for IBB IQs: @@ -664,8 +664,7 @@ class BytestreamIBB(Bytestream): assert ibb_data['sid'] == self.stream_id # ack the IQ - result = make_result_iq(self.stream, ibb_event.stanza) - result.send() + send_error_reply(self.stream, ibb_event.stanza) if len(binary) >= size or size == 0: received = True Before this branch, tubes/accept-private-stream-tube.py (which tells Gabble to send some data down the bytestream, waits for it to arrive, sends back a reply over XMPP and sucks the reply out of Gabble) passes even with this change. With this branch, this change breaks this and other tests, as you would expect. This branch doesn't address MUCs. I could get behind this. r+ Merged. http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=4d9133a523d95207699b3ede02b26e7e2c272fdd Retitling for MUCs maybe having the same issue (which I still haven't really checked.) -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-gabble/issues/222. |
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.
During the LibreOffice hackfest, Michael and Eike asked me how reliable tubes are. Specifically, might we ever drop packets? I said I thought that if a message failed to send for whatever reason (maybe rate-limiting by the server) the tube would die, which is imperfect but at least means that as long as the tube is open it's reliable. I was wrong, at least for in-band bytestreams. In bytestream-ibb.c's send_data() function, an IQ is constructed with the data to send, and sent (irrelevant bookkeeping elided): iq = wocky_stanza_build (WOCKY_STANZA_TYPE_IQ, WOCKY_STANZA_SUB_TYPE_SET, NULL, priv->peer_jid, '(', "data", '$', encoded, ':', NS_IBB, '@', "sid", priv->stream_id, '@', "seq", seq, ')', NULL); ret = _gabble_connection_send_with_reply (priv->conn, iq, iq_acked_cb, G_OBJECT (self), NULL, &error); if (!ret) { gabble_bytestream_iface_close (GABBLE_BYTESTREAM_IFACE (self), NULL); _gabble_connection_send_with_reply() succeeds unless you're literally not connected any more, so the error-checking here is perhaps a little overly keen. But iq_acked_cb doesn't look at the reply to see if it's got type='result' or type='error'. If the IQ is sent successfully but we get an error reply back, we blindly carry on sending subsequent chunks of data. Badness. (If sending the IQ fails (that is, if we disconnect before we get a reply), the callback is never called. It looks like that's okay in this case.) I think bytestream-socks5 is okay. bytestream-muc.c uses <message/> stanzas, doesn't put an id='' on them, and doesn't check for errors or for the echo as far as I can tell, so may have similar issues if the MUC rejects some of our messages (rate-limiting doesn't seem unlikely…).