OTRv4 builds silently drop all jabber messages

[dx <dx@…>]

The messages don't show at all in the jabber xmlconsole (it idles and shows empty TX: regularly), and other protocols (at least AIM) don't seem to be affected

Downgrading to builds older than 1001 or uninstalling the otr plugin "fixes" the issue.

OTR policy doesn't seem to affect this behavior.

Sometimes it's not *all* jabber messages but every message after the first one.

[pesco]

What is the jabber xmlconsole?

Is this about outgoing or incoming messages?

Are you using multiple clients (or bitlbee instances) on the same account?

[dx]

Oh hey, didn't expect you to appear. I was just submitting this to warn other users that try the test builds.

I haven't reproduced this issue personally yet - i'm just stating the conclusions we've reached after debugging this with #bitlbee users (four people complained about this so far)

Replying to pesco:

What is the jabber xmlconsole?

The jabber xmlconsole is a special "contact" meant for debugging, that shows the XML messages that are sent/received in the connection to the server. See this link for more details on how to enable it ​http://wiki.bitlbee.org/Debugging

What I said about the xmlconsole in the description was just a way to say that OTR is eating the messages at some point, they don't reach the server.

Is this about outgoing or incoming messages?

Outgoing messages only.

Are you using multiple clients (or bitlbee instances) on the same account?

Has been reproduced with no other xmpp clients.

[anonymous]

Thanks! Please drop any further hints you might uncover as to what is swallowing the messages in this ticket.

[dx]

Okay, first meaningful result of my debugging:

The OTRv4 plugin always eats all messages, but if it has any modifications to do to the messages (such as adding the magic whitespace pattern, or encrypting it), it calls inject_message to actually send it to the contact.

This means that setting otr_policy to never means not getting any message sent, while the default value, opportunistic means getting only the first message sent

The only way to communicate while having the OTRv4 plugin installed is to encrypt all messages

Now i'm pretty sure it's not jabber specific. It should apply to all supported protocols (everything except twitter). That AIM contact mentioned in the ticket description could have some otr plugin installed in their client (should confirm that later by asking the user who reported that)

(I'm currently looking at #1110 too - my OTRv4 bitlbee crashes on every 'second message')

[Flexo <bitlbee@…>]

This patch fixes the eaten messages when otr isn't active (for whatever reason) but doesn't fix the double-message segfault.

I don't have an end-to-end OTR setup at the moment so I haven't been able to test with OTR active, and I've run out of time for today.

I would also like to hear from the original author whether why OTRL_FRAGMENT_SEND_ALL was chosen rather than OTRL_FRAGMENT_SEND_SKIP, which I have changed it to, in case there are subtle effects I haven't noticed.

[Flexo <bitlbee@…>]

Fix for eaten messages when OTR is inactive.

[pesco]

Replying to Flexo <bitlbee@…>:

I would also like to hear from the original author whether why OTRL_FRAGMENT_SEND_ALL was chosen rather than OTRL_FRAGMENT_SEND_SKIP, which I have changed it to, in case there are subtle effects I haven't noticed.

Well, the reason is that SEND_ALL is what we want. It does (or is supposed to do) what it says: send all OTR messages that arise after (possible) fragmentation. SEND_SKIP actually disables fragmentation, so it's a regression. An alternative is SEND_ALL_BUT_LAST, but I'm afraid that will just hide the real bug, so I'd at least like to nail down the cause.

There is a subtle mistake in your patch. In the error case, the original message must not be sent, i.e. the function should return NULL.

I did some poking the other night but couldn't quite find the cause for the disappearing messages before I ran out of time. Will try to get a better look tomorrow.

[wilmer]

Glad to see your name, pesco! (I assume you won't be at FOSDEM this weekend?)

I've already committed Flexo's patch since we were already in a broken situation anyway and it appeared to get us into a slightly less broken state.

For now I've had to disable otr package builds on code.bitlbee.org because it's too buggy. :-(

[pesco]

Replying to wilmer:

Glad to see your name, pesco! (I assume you won't be at FOSDEM this weekend?)

That is correct... :/

I've already committed Flexo's patch since we were already in a broken situation anyway and it appeared to get us into a slightly less broken state.

For now I've had to disable otr package builds on code.bitlbee.org because it's too buggy. :-(

Understood. Sorry about this mess, and thanks again to people for testing as somehow the issue didn't manifest clearly in my cases.

I think some of it is that libotr has gotten to a point of complexity that the reference use in pidgin-otr is no longer a sufficient test case. There are parts that feel flaky, maybe because they are not used by the major plugin implementation.

[pesco]

When using OTRL_FRAGMENT_SEND_ALL, libotr handles outgoing unmodified plaintext messages incorrectly (or at least confusingly).

NB: "Unmodified" above means, in particular, the case where no whitespace tag is added to the message. This can be either because it is disabled (OTR policy "manual") or because our buddy "answered" our tag with a non-OTR message ("rejected" the tag). That is the reason why sometimes the first message will go out and later ones are dropped: The ones that get through are those with a whitespace tag.

With libotr 3 it was the caller's responsibility to send any messages or fragments after otrl_message_sending. There are three cases:

1. otrl_message_sending returns an error. We should send nothing.
2. otrl_message_sending returns success and sets its messagep argument. We should send *messagep via otrl_fragment_and_send.
3. otrl_message_sending returns success and sets messagep to NULL. We should send the original message.

The OTRL_FRAGMENT_SEND_ALL path of libotr 4.0.0 omits case 3 (cf. ​http://sourceforge.net/p/otr/libotr/ci/4.0.0/tree/src/message.c#l441 ):

    /* Fragment and send according to policy */
    if (!err && messagep && *messagep) {
        if (context) {
            err = fragment_and_send(ops, NULL, context, *messagep,
                    fragPolicy, messagep);
        }
    }
    return err;

If *messagep is NULL, as in the unmodified plaintext case, nothing is sent.

I suppose we could still handle this case by checking *messagep in the caller but it is not clear to me whether this is the intended usage. I have instead opted to do the same thing that pidgin-otr does and use SEND_ALL_BUT_LAST where it is clear that the caller must handle *messagep.

So implemented in r1007 of ​http://code.khjk.org/bitlbee/.

[dx]

Wilmer just merged r1007 in the main branch. Thanks pesco!