Skip to content

fix: quote attachments not rendering when client hostname differs from Site_Url #40107

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.

Sign up for GitHub

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

Closed
ricardogarim wants to merge 1 commit into from
Closed

fix: quote attachments not rendering when client hostname differs from Site_Url #40107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: detect quote permalinks by query param instead of hostname
  • Loading branch information
@ricardogarim
ricardogarim committed Apr 9, 2026
commit c24a7d787ef8656df864ea7d93c0c8ec8d931dc8
5 changes: 5 additions & 0 deletions .changeset/shy-pillows-repair.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixes quote attachments not rendering when client hostname differs from Site_Url
55 changes: 19 additions & 36 deletions apps/meteor/server/services/messages/hooks/BeforeSaveJumpToMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import QueryString from 'querystring';
import URL from 'url';

import type { MessageAttachment, IMessage, IUser, IOmnichannelRoom, IRoom } from '@rocket.chat/core-typings';
import type { MessageAttachment, MessageUrl, IMessage, IUser, IOmnichannelRoom, IRoom } from '@rocket.chat/core-typings';
import { isOmnichannelRoom, isQuoteAttachment } from '@rocket.chat/core-typings';

import { createQuoteAttachment } from '../../../../lib/createQuoteAttachment';

const getMessageIdFromUrl = (url: string): string | undefined => {
const { query } = URL.parse(url, true);
return typeof query?.msg === 'string' ? query.msg : undefined;
};

const recursiveRemoveAttachments = (attachments: MessageAttachment, deep = 1, quoteChainLimit: number): MessageAttachment => {
if (attachments && isQuoteAttachment(attachments)) {
if (deep < quoteChainLimit - 1) {
Expand Down Expand Up @@ -80,7 +84,6 @@ export class BeforeSaveJumpToMessage {
user: Pick<IUser, '_id' | 'username' | 'name' | 'language'>;
config: {
chainLimit: number;
siteUrl: string;
useRealName: boolean;
};
}): Promise<IMessage> {
Expand All @@ -92,27 +95,15 @@ export class BeforeSaveJumpToMessage {
return message;
}

const linkedMessages = message.urls
.filter((item) => item.url.includes(config.siteUrl))
.map((item) => {
const urlObj = URL.parse(item.url);

// if the URL doesn't have query params (doesn't reference message) skip
if (!urlObj.query) {
return;
}

const { msg: msgId } = QueryString.parse(urlObj.query);

if (typeof msgId !== 'string') {
return;
}

return { msgId, url: item.url };
})
.filter(Boolean);
const linkedMessages: { msgId: string; urlItem: MessageUrl }[] = [];
for (const urlItem of message.urls) {
const msgId = getMessageIdFromUrl(urlItem.url);
if (msgId) {
linkedMessages.push({ msgId, urlItem });
}
}

const msgs = await this.getMessages(linkedMessages.map((linkedMsg) => linkedMsg?.msgId) as string[]);
const msgs = await this.getMessages(linkedMessages.map((linkedMsg) => linkedMsg.msgId));

const validMessages = msgs.filter((msg) => validateAttachmentDeepness(msg, config.chainLimit));

Expand All @@ -138,17 +129,8 @@ export class BeforeSaveJumpToMessage {

const quotes = [];

for (const item of message.urls) {
if (!item.url.includes(config.siteUrl)) {
continue;
}

const linkedMessage = linkedMessages.find((msg) => msg?.url === item.url);
if (!linkedMessage) {
continue;
}

const messageFromUrl = validMessages.find((msg) => msg._id === linkedMessage.msgId);
for (const { msgId, urlItem } of linkedMessages) {
const messageFromUrl = validMessages.find((msg) => msg._id === msgId);
if (!messageFromUrl) {
continue;
}
Expand All @@ -157,9 +139,10 @@ export class BeforeSaveJumpToMessage {
continue;
}

item.ignoreParse = true;
// prevent OEmbed from also fetching this URL for a link preview
urlItem.ignoreParse = true;

quotes.push(createQuoteAttachment(messageFromUrl, item.url, useRealName, this.getUserAvatarURL(messageFromUrl.u.username)));
quotes.push(createQuoteAttachment(messageFromUrl, urlItem.url, useRealName, this.getUserAvatarURL(messageFromUrl.u.username)));
}

if (quotes.length > 0) {
Expand Down
1 change: 0 additions & 1 deletion apps/meteor/server/services/messages/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
user,
config: {
chainLimit: settings.get<number>('Message_QuoteChainLimit'),
siteUrl: settings.get<string>('Site_Url'),
useRealName: settings.get<boolean>('UI_Use_Real_Name'),
},
});
Expand Down
131 changes: 55 additions & 76 deletions apps/meteor/tests/unit/server/services/messages/hooks/BeforeSaveJumpToMessage.tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,13 @@ describe('Create attachments for message URLs', () => {
const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey'),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

return expect(message).to.not.have.property('urls');
});

it('should do nothing if URL is not from SiteUrl', async () => {
it('should do nothing if URL does not have a msg query parameter', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
getRooms: async () => [createRoom()],
Expand All @@ -89,18 +85,14 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
expect(message).to.not.have.property('attachments');
});

it('should do nothing if URL is from SiteUrl but not have a query string', async () => {
it('should do nothing if URL has a non-msg query parameter', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
getRooms: async () => [createRoom()],
Expand All @@ -112,24 +104,20 @@ describe('Create attachments for message URLs', () => {
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat',
url: 'https://open.rocket.chat/?token=value',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
expect(message).to.not.have.property('attachments');
});

it('should do nothing if URL is from SiteUrl but not have a msgId query string', async () => {
it('should reject query parameter pollution (msg[$gt]=)', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
getRooms: async () => [createRoom()],
Expand All @@ -141,24 +129,55 @@ describe('Create attachments for message URLs', () => {
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/?token=value',
url: 'https://open.rocket.chat/?msg[$gt]=',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
expect(message).to.not.have.property('attachments');
});

it('should do nothing if it do not find a msg from the URL', async () => {
it('should create quote attachment even when client hostname differs from server (e.g. localhost vs 127.0.0.1)', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
getRooms: async () => [createRoom()],
canAccessRoom: async () => true,
getUserAvatarURL: () => 'url',
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [
{
url: 'http://localhost:3000/channel/general?msg=linked',
meta: {},
},
],
}),
user: createUser(),
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);

const [url] = message.urls ?? [];
expect(url).to.include({
url: 'http://localhost:3000/channel/general?msg=linked',
ignoreParse: true,
});

expect(message).to.have.property('attachments').and.to.have.lengthOf(1);

const [attachment] = message.attachments ?? [];
expect(attachment).to.have.property('text', 'linked message');
});

it('should do nothing if it does not find a message from the URL', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [],
getRooms: async () => [createRoom()],
Expand All @@ -176,11 +195,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
Expand All @@ -205,18 +220,14 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
expect(message).to.not.have.property('attachments');
});

it('should do nothing if user dont have access to the room of the message from the URL', async () => {
it('should do nothing if user does not have access to the room of the message from the URL', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
getRooms: async () => [createRoom()],
Expand All @@ -234,11 +245,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').of.length(1);
Expand Down Expand Up @@ -272,11 +279,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);
Expand Down Expand Up @@ -317,11 +320,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('attachments').and.to.have.lengthOf(0);
Expand Down Expand Up @@ -349,11 +348,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('attachments').and.to.have.lengthOf(1);
Expand All @@ -379,11 +374,7 @@ describe('Create attachments for message URLs', () => {
],
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);
Expand Down Expand Up @@ -453,7 +444,6 @@ describe('Create attachments for message URLs', () => {
user: createUser(),
config: {
chainLimit: 3,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
});
Expand Down Expand Up @@ -536,7 +526,6 @@ describe('Create attachments for message URLs', () => {
user: createUser(),
config: {
chainLimit: 3,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
});
Expand Down Expand Up @@ -567,11 +556,7 @@ describe('Create attachments for message URLs', () => {
token: 'livechatToken',
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);
Expand Down Expand Up @@ -609,11 +594,7 @@ describe('Create attachments for message URLs', () => {
token: 'another-token',
}),
user: createUser(),
config: {
chainLimit: 10,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
config: { chainLimit: 10, useRealName: true },
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);
Expand Down Expand Up @@ -653,7 +634,6 @@ describe('Create attachments for message URLs', () => {
user: createUser(),
config: {
chainLimit: 1,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
});
Expand Down Expand Up @@ -708,7 +688,6 @@ describe('Create attachments for message URLs', () => {
user: createUser(),
config: {
chainLimit: 1,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
});
Expand Down
Loading