[Bug 32612] Remove Tubes code

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Jul 18 20:18:04 CEST 2012


https://bugs.freedesktop.org/show_bug.cgi?id=32612

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-07-18 11:18:04 PDT ---
"private-tubes-factory: only create Tube channels for tube offers"

+ guint tube_id;
...
+ tube_id = g_ascii_strtoull (tmp, NULL, 10);
...
+ if (tube_id == 0 || tube_id >= G_MAXINT)
+ DEBUG ("tube ID is not numeric or > 2**32: %s", tmp);

tube_id should be a guint64 really, and the comparison should be with
G_MAXUINT32 (unless you really mean G_MAXINT32, and 2**31 in the debug
message). This is probably not your fault, you probably moved the buggy code
from elsewhere.

The debug message is misleading, since 0 is also disallowed. "tube ID is
non-numeric or out of range"?

+ DEBUG ("tube ID already in use; do not open the offered tube and close "
+ "the existing tube if it's to the same contact");

Not a merge blocker and presumably not your fault, but these semantics are
crazy. We should have a separate tube ID "namespace" per peer, and store tubes
in the hash table by (handle, id) tuples or something.

+private_tubes_factory_tube_close_cb (
...
+ if (!tube_msg_checks (self, msg, node, NULL, &tube_id))
+ return FALSE;

Er, this function allows Alice to close tubes between us and Bob, if she can
guess or brute-force the tube ID. Pre-existing bug?

+new_channel_from_stanza (GabblePrivateTubesFactory *self,

Deserves a Returns: (transfer none) pseudo-doc-comment, I think - its name
sounds as though it might return a ref.

+ /* this has already been checked */
+ handle = tp_handle_lookup (contact_repo,
+ wocky_stanza_get_from (stanza), NULL, NULL);

Has it really? Validity has been checked, but existence of a handle hasn't.
extract_tube_information doesn't seem to ensure the handle if the
initiator_handle "out" parameter is NULL, as it is here.

I think you should use tp_handle_ensure, then assert that the handle is nonzero
(which, you're right, Wocky should already have checked).

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.



More information about the telepathy-bugs mailing list