Skip to content

Commit 4b34bd6

Browse files
authored
fix: import Slack files as attachments instead of raw URLs (RocketChat#41285)
1 parent 0faaaeb commit 4b34bd6

9 files changed

Lines changed: 774 additions & 35 deletions

File tree

.changeset/deep-ways-poke.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@rocket.chat/core-typings': patch
3+
'@rocket.chat/models': patch
4+
'@rocket.chat/meteor': patch
5+
---
6+
7+
Fixes the Slack importer storing shared files as raw URLs in the message body. Imported file messages now stay hidden until "Download Pending Files" button fetches them, then display as native attachments with image previews. Failed downloads (e.g. invalidated export links) are no longer silently saved as the file's content — they are counted as errors and can be retried.

apps/meteor/server/lib/import/classes/converters/MessageConverter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export class MessageConverter extends RecordConverter<IImportMessageRecord> {
104104
mentions,
105105
channels,
106106
_importFile: data._importFile,
107+
_hidden: data._hidden,
107108
url: data.url,
108109
attachments: data.attachments,
109110
bot: data.bot,

apps/meteor/server/lib/import/pending-files/PendingFileImporter.ts

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import https from 'node:https';
33

44
import { api } from '@rocket.chat/core-services';
55
import type { IImport, MessageAttachment, IUpload, IImporterShortSelection } from '@rocket.chat/core-typings';
6-
import { Messages } from '@rocket.chat/models';
6+
import { Messages, Users } from '@rocket.chat/models';
77
import { Random } from '@rocket.chat/random';
88

99
import { Importer, ProgressStep } from '..';
10+
import { parseFileIntoMessageAttachments } from '../../../meteor-methods/messages/sendFileMessage';
1011
import { FileUpload } from '../../media/file-upload';
1112
import type { ConverterOptions } from '../classes/ImportDataConverter';
1213
import type { ImporterProgress } from '../classes/ImporterProgress';
@@ -79,6 +80,12 @@ export class PendingFileImporter extends Importer {
7980
currentSize -= details.size;
8081
};
8182

83+
const failFile = async (details: { size: number }) => {
84+
await this.addCountError(1);
85+
count--;
86+
currentSize -= details.size;
87+
};
88+
8289
const logError = this.logger.error.bind(this.logger);
8390

8491
try {
@@ -107,7 +114,6 @@ export class PendingFileImporter extends Importer {
107114
rid: message.rid,
108115
};
109116

110-
const requestModule = /https/i.test(url) ? https : http;
111117
const fileStore = FileUpload.getStore('Uploads');
112118

113119
nextSize = details.size;
@@ -116,41 +122,25 @@ export class PendingFileImporter extends Importer {
116122
currentSize += nextSize;
117123
downloadedFileIds.push(_importFile.id);
118124

119-
requestModule.get(url, (res) => {
120-
const contentType = res.headers['content-type'];
121-
if (!details.type && contentType) {
122-
details.type = contentType;
123-
}
125+
void this.downloadFile(url, details)
126+
.then(async (rawData) => {
127+
// Bypass the fileStore filters
128+
const file = await fileStore._doInsert(details, rawData);
124129

125-
const rawData: Uint8Array[] = [];
126-
res.on('data', (chunk) => {
127-
rawData.push(chunk);
130+
const rocketChatUrl = FileUpload.getPath(`${file._id}/${encodeURI(file.name || '')}`);
131+
const user = await Users.findOneById(message.u._id);
132+
const attachment = user
133+
? (await parseFileIntoMessageAttachments(file, message.rid, user)).attachments[0]
134+
: this.getMessageAttachment(file, rocketChatUrl);
128135

129-
// Update progress more often on large files
130-
this.reportProgress();
131-
});
132-
res.on('error', async (err) => {
136+
await Messages.setImportFileRocketChatAttachment(_importFile.id, rocketChatUrl, attachment);
133137
await completeFile(details);
134-
logError({ err });
138+
importedRoomIds.add(message.rid);
139+
})
140+
.catch(async (err) => {
141+
logError({ msg: 'Failed to download pending file', url, err });
142+
await failFile(details);
135143
});
136-
137-
res.on('end', async () => {
138-
try {
139-
// Bypass the fileStore filters
140-
const file = await fileStore._doInsert(details, Buffer.concat(rawData));
141-
142-
const url = FileUpload.getPath(`${file._id}/${encodeURI(file.name || '')}`);
143-
const attachment = this.getMessageAttachment(file, url);
144-
145-
await Messages.setImportFileRocketChatAttachment(_importFile.id, url, attachment);
146-
await completeFile(details);
147-
importedRoomIds.add(message.rid);
148-
} catch (err) {
149-
await completeFile(details);
150-
logError({ err });
151-
}
152-
});
153-
});
154144
} catch (err) {
155145
this.logger.error({ err });
156146
}
@@ -172,6 +162,39 @@ export class PendingFileImporter extends Importer {
172162
return this.getProgress();
173163
}
174164

165+
private downloadFile(url: string, details: { type?: string }): Promise<Buffer> {
166+
return new Promise((resolve, reject) => {
167+
const requestModule = /https/i.test(url) ? https : http;
168+
169+
requestModule
170+
.get(url, (res) => {
171+
res.on('error', reject);
172+
173+
if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) {
174+
// Error responses (e.g. a redirect to a login page) would otherwise be saved as the file's content.
175+
res.resume(); // discard the body and free the socket
176+
reject(new Error(`Unexpected response status ${res.statusCode}`));
177+
return;
178+
}
179+
180+
const contentType = res.headers['content-type'];
181+
if (!details.type && contentType) {
182+
details.type = contentType;
183+
}
184+
185+
const rawData: Uint8Array[] = [];
186+
res.on('data', (chunk) => {
187+
rawData.push(chunk);
188+
189+
// Update progress more often on large files
190+
this.reportProgress();
191+
});
192+
res.on('end', () => resolve(Buffer.concat(rawData)));
193+
})
194+
.on('error', reject);
195+
});
196+
}
197+
175198
getMessageAttachment(file: IUpload, url: string): MessageAttachment {
176199
if (file.type) {
177200
if (/^image\/.+/.test(file.type)) {

apps/meteor/server/lib/import/slack/SlackImporter.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,8 @@ export class SlackImporter extends Importer {
496496
_id: fileId,
497497
rid: newMessage.rid,
498498
ts: newMessage.ts,
499-
msg: message.file.url_private_download || '',
499+
msg: '',
500+
_hidden: true,
500501
_importFile: this.convertSlackFileToPendingFile(message.file),
501502
u: {
502503
_id: newMessage.u._id,
@@ -568,7 +569,8 @@ export class SlackImporter extends Importer {
568569
_id: fileId,
569570
rid: slackChannelId,
570571
ts: newMessage.ts,
571-
msg: file.url_private_download || '',
572+
msg: '',
573+
_hidden: true,
572574
_importFile: this.convertSlackFileToPendingFile(file),
573575
u: {
574576
_id: this._replaceSlackUserId(message.user),

0 commit comments

Comments
 (0)