RocketChat / RocketChat/Rocket.Chat
Message_QuoteChainLimit has off-by-one error
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 46.1k
- Forks
- 13.9k
- Avg merge
- 3d 3h
- Merged PRs (30d)
- 130
Description
Description:
Commit 5afe5ceb72888f0a0d51544d89fe1b4567ff5e85 ("Chore: Migrate oembed to ts (#25622)", released in RC v5.0.0) introduced a regression in the recursive processing of Message_QuoteChainLimit. The change creates an off-by-one error, cutting off the quote chain at the wrong point.
So, for example, if Message_QuoteChainLimit is at its default value of 2 and I quote a message that itself quotes a chain of other messages, RC v4.x would show:
- my new message, quoting...
- an older message, quoting...
- a still older message
- an older message, quoting...
However, under RC v5.x, without changing any configuration, the same action would show:
- my new message, quoting...
- an older message, quoting...
- a still older message, quoting...
- a very old message
- a still older message, quoting...
- an older message, quoting...
Additionally, Message_QuoteChainLimit has a bug in that setting it to 1 (or 0) isn't honored. The implementation always truncates at no less than 2.
As an aside, the documentation for this setting does not clearly define what constitutes a Quote Chain.
I wasn't certain whether "an older message" (in the above example) counted as part of the quote chain or not; that is, is the first quoted message not considered "chained" (excluding it from the count)? Or is the entire chain counted as a group (including it in the count)? I turned to the documentation for clarification, but https://docs.rocket.chat/guides/administration/admin-panel/settings/message says merely:
Maximum Number of Chained Quotes: Lets you set the maximum number of Chained quotes.
Steps to reproduce:
- Go to /admin/settings/Message and set Maximum Number of Chained Quotes to 2
- Enter a chat room and post a message.
- Click on the "Curly Quotes" Quote icon for the newly created message.
- Type in a message and post it.
- Repeat steps 3-4 several times, each time clicking [Quote] on the most recently created message.
Expected behavior:
The fourth message should not include the original message in the quote chain. Instead, message number 2 should be the oldest post in the chain.
Actual behavior:
The fourth message includes all 3 prior messages in the quote chain, including the original message, even though we have exceeded the limit of 2.
Server Setup Information:
- Version of Rocket.Chat Server: 5.1.2
- Operating System: Linux
- Deployment Method: tar
- Number of Running Instances: 5
- DB Replicaset Oplog:
- NodeJS Version: v14.19.3
- MongoDB Version: 4.2.15 / wiredTiger (oplog Enabled)
Client Setup Information
- Desktop App or Browser Version: Chrome Version 106.0.5249.119 (Official Build) (arm64)
- Operating System: MacOS
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the TypeScript implementation of Message_QuoteChainLimit from the Maximum Number of Chained Quotes setting and reproduce the issue using the listed quote steps. Verify the default limit of 2 and values of 1 and 0 against the expected chain length, then add regression coverage showing that the oldest quoted message is excluded when the limit is reached.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100