Comment 206 for bug 296867

Revision history for this message
In , Simon McVittie (smcv) wrote :

Implementation in Gabble:

+ /* FIXME: There should be no sender for a notification, but setting handle to
+ * 0 makes empathy crash atm. */
+ tp_message_mixin_take_received (G_OBJECT (self),
+ tp_cm_message_new_text (base_conn,
+ tp_base_channel_get_target_handle (base_chan),
+ TP_CHANNEL_TEXT_MESSAGE_TYPE_NOTICE, text));

Is this a message from the OTR library, something like "*** Verified peer fingerprint: <email address hidden> ***"?

I think using the target handle for this is OK semantically.

However, I suspect remote users can spoof this by sending their own NOTICE. Messages coming from the OTR library should have a distinctive message header that an OTR-literate UI can take as evidence that they were locally-generated.

Ideally, that distinctive message header should be a machine-readable version of the message, so OTR-literate UIs (Empathy) can discard the untranslated version from Gabble and display something translated. We've always had a policy of putting UI strings and their translations in the UIs, not the CMs.

+ return g_variant_new ("(s@ay)", display_fp,
+ g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, fp_raw, 20,
...
+ guchar our_fp_raw[20];

The magic number 20 makes me nervous. Isn't there a constant for "length of a raw OTR fingerprint in bytes" in libotr?

If there really isn't, #define'ing our own would be better than nothing.

+static void
+otr_inject_message (void *opdata,
+ const gchar *accountname,
+ const gchar *protocol,
+ const gchar *recipient,
+ const gchar *message)
+{
+ inject_message (opdata, message);
+}

Is @message text/plain or text/html? Telepathy can only do text/plain at the moment, so if it's text/html, we need to strip tags, then unescape entities (&stuff;).

+static gint
+otr_max_message_size (void *opdata,
+ ConnContext *context)
+{
+ return 0;
+}

We should probably give some guess at what's generally interoperable.

+ msg = otrl_proto_default_query_msg (get_self_id (self), OTRL_POLICY_DEFAULT);

Do we need to update what otr_policy() would return here, too?

+ bus_name = g_strconcat (tp_base_connection_get_bus_name (base_conn),
+ ".OTR", NULL);

I suppose this isn't *so* bad, but the spec should tell the API user where to find this name.

+ content = wocky_node_get_content_from_child (node, "body");
+
+ err = otrl_message_sending (userstate, ui_ops_p, self,
+ get_self_id (self), "xmpp", get_target_id (self),
+ priv->instag, content, NULL, &new_content,
+ OTRL_FRAGMENT_SEND_ALL_BUT_LAST, NULL,
+ NULL, NULL);

Does otrl_message_sending() expect @content to be text/plain or text/html? If it expects text/html, we need to escape special characters with g_markup_escape_text().

Similarly, is @new_content text/plain or text/html? If text/html, we need to strip tags and unescape entities.

+gchar *
+gabble_im_channel_otr_receiving (GabbleIMChannel *self,
+ const gchar *content)

Same here.