Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User messages double logged in MUC logs #1201

Closed
Shungy opened this issue Oct 6, 2019 · 5 comments · Fixed by #1209
Closed

User messages double logged in MUC logs #1201

Shungy opened this issue Oct 6, 2019 · 5 comments · Fixed by #1209
Assignees
Labels
Milestone

Comments

@Shungy
Copy link

Shungy commented Oct 6, 2019

  • Enable MUC logging
  • Write something
  • Check the log file
  • What you write is double-logged

v. 0.7.1

@jubalh jubalh self-assigned this Oct 6, 2019
@jubalh jubalh added this to the 0.8.0 milestone Oct 6, 2019
@jubalh jubalh added the bug label Oct 6, 2019
@jubalh
Copy link
Member

jubalh commented Oct 6, 2019

We log both outgoing MUC messages and incoming MUC messages.

We could only log the incoming ones (since we also get our own messages), but need to make sure that we trusted our OMEMO key in that case, and maybe other encryption specific things.

Or we could filter the incoming messages and only log the ones not coming from our Occupant JID.

@jubalh
Copy link
Member

jubalh commented Oct 15, 2019

With the current fix we also filter out messages sent from ourselves by other clients.

@jubalh
Copy link
Member

jubalh commented Oct 16, 2019

To solve this nicely we will need #1207 XEP-0359: Unique and Stable Stanza IDs.
Then we can filter incoming messages based on origin-id.

jubalh added a commit to jubalh/profanity that referenced this issue Oct 18, 2019
jubalh added a commit to jubalh/profanity that referenced this issue Oct 18, 2019
We implement </origin-id> from [XEP-0359](https://xmpp.org/extensions/xep-0359.html).
We already had this implemented for OMEMO. And now use it to check
whether MUC messages were sent from us
(profanity-im#1201).

We don't implement </stanza-id> yet, but probably need to do so for MAM.

Anyways let's flag this as implementing the XEP.

Fix profanity-im#1207
@jubalh jubalh mentioned this issue Oct 18, 2019
@jubalh
Copy link
Member

jubalh commented Oct 21, 2019

@Shungy should be fixed now!

@Shungy
Copy link
Author

Shungy commented Oct 21, 2019

Great! Thank you.

jubalh added a commit that referenced this issue Oct 31, 2019
For OMEMO we had a list with our sent messages.
It was used so that we don't decrypt our own messages in MUCs that come
in via reflection.

Recently for #1209 we
started to use origin-id and use an algorithm so we can detect our own
sent messages via checking origin-id.

Profanity uses the same id for the message ID and origin-id.

With 06f300a we added the
message_is_sent_by_us() function.

We implemented XEP-0359 this way to fix
#1201 so that we don't
log our own messages in MUCs twice.

We can now check whether the message was sent by us using this function
and can get rid of the list.

Probably we could also put many parts of the sv_ev_room_message()
function inside (else) part of `if (!(g_strcmp0(mynick,
message->jid->resourcepart) == 0 && message_is_sent_by_us(message))) {`.

Have to look more closely whether any of this needs to be run in case
the message actually comes from us.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants