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

Sending receipts for messages fetched from the archive #1896

Closed
deleolajide opened this issue Feb 27, 2020 · 14 comments · Fixed by #1897
Closed

Sending receipts for messages fetched from the archive #1896

deleolajide opened this issue Feb 27, 2020 · 14 comments · Fixed by #1897
Labels
Milestone

Comments

@deleolajide
Copy link
Member

deleolajide commented Feb 27, 2020

Describe the bug
Every time you re-open a chatbox (one-to-one chat) and recieve a message from the MAM archive, Converse is generating one more receipt for this messages. The consequence is that the archive gets filled with receipts until only receipts are sent back instead of actual messages.

To Reproduce
Steps to reproduce the behavior:

  1. Go to conversejs.org and login.
  2. Open a chatbox to another user
  3. Type in a few messages.
  4. Open and close the chatbox several times.

Expected behavior
The chat history of the messages just typed. Instead, you will see the chat messages diminishing until the chat history is empty. At this point, only receipts are being sent.

@deleolajide
Copy link
Member Author

On investigating this, I think the IF statement for sendReceiptStanza needs to check if the original stanza isMAMMessage.

if (requests_receipt && !is_me && !u.isCarbonMessage(stanza)) {

@deleolajide deleolajide changed the title Converse.js is sending receipts for messages it fetches from the archive Sending receipts for messages fetched from the archive Feb 27, 2020
@jcbrand
Copy link
Member

jcbrand commented Feb 28, 2020

Thanks @deleolajide, looks like you're right: https://xmpp.org/extensions/xep-0184.html#when-archive

jcbrand pushed a commit that referenced this issue Feb 28, 2020
* fix issue #1896

* Added issue to CHANGES.md
@jcbrand jcbrand added this to the 7.0.0 milestone Feb 28, 2020
jcbrand pushed a commit that referenced this issue Feb 28, 2020
* fix issue #1896

* Added issue to CHANGES.md
@anyuta1166
Copy link

@jcbrand It seems that this issue is not fixed.

I observe the same behaviour on self-hosted conversejs build which is built from the current master.

@jcbrand
Copy link
Member

jcbrand commented Jun 8, 2020

Thanks @vampik, I think I've fixed the issue now, could you please double check?

@anyuta1166
Copy link

@jcbrand this seemed to work until I unchecked "This is a trusted device". Now the archive continues to be filled on every login...

@jcbrand
Copy link
Member

jcbrand commented Jun 11, 2020

@jcbrand this seemed to work until I unchecked "This is a trusted device". Now the archive continues to be filled on every login...

I don't see anything in the code to indicate that trusted mode is related to sending receipts to MAM messages.

@anyuta1166
Copy link

I don't see anything in the code to indicate that trusted mode is related to sending receipts to MAM messages.

I think in trusted mode the history is cached. If trusted mode is off than the history is not cached and is loaded from the server on every login, causing sending receipts on every login.

@jcbrand
Copy link
Member

jcbrand commented Jun 12, 2020

The whole point of this bugfix is to not send receipts for MAM messages, so your issue should be fixed by it.

@anyuta1166
Copy link

But every time I log in with "This is a trusted device" unchecked, the messages are fetched from the archive and receipts for them are sent. That's what I'm talking about.

@jcbrand
Copy link
Member

jcbrand commented Jun 12, 2020

Did you actually test this fix? b511f1d

It should prevent receipts from being sent when messages are fetched from the archive.

@anyuta1166
Copy link

Did you actually test this fix? b511f1d

Yes, this is my latest build:

# git log --oneline
83da03d6e (HEAD -> master, origin/master, origin/HEAD) [Security] Bump websocket-extensions from 0.1.3 to 0.1.4
a2d33ce3b Bump uglify-js from 3.8.1 to 3.9.1
b511f1d95 Updates #1896: Use right flag to checked if message is archived

@jcbrand jcbrand reopened this Jun 12, 2020
@anyuta1166
Copy link

Any ETA on fixing this? I need a web client very much, but it is unusable until this is fixed.

@anyuta1166
Copy link

anyuta1166 commented Aug 13, 2020

Hello. Any news on fixing the critical bug? I could help with testing any time.

jcbrand added a commit that referenced this issue Aug 13, 2020
Also, rename attribute from `is_receipt_request` to `is_valid_receipt_request` to avoid confusion.
@jcbrand
Copy link
Member

jcbrand commented Aug 13, 2020

@anyuta1166: I think it's fixed now, can you please check?

@jcbrand jcbrand added the bug label Aug 13, 2020
@jcbrand jcbrand closed this as completed Sep 10, 2020
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.

3 participants