-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat: Send multiple files into a single message #32703
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
base: develop
Are you sure you want to change the base?
Changes from 235 commits
5a9f646
771314e
755c08b
f2bd552
39ff2a3
23eb009
e786f70
f4b60d5
1c0ddde
f5213b0
6f398b4
cdc0ff5
a1faa22
326a197
c078083
0e4d827
60a80f9
7ac5841
9f74219
9afeee6
97f743b
68f45dc
d7b92cc
21005c0
a0f6c83
de319d8
3ad5e63
72bc00d
fbd30c9
fe2b428
b3db9fc
3aa8c3d
2eb9b64
11aac8e
9ff1b9f
2370b14
47b592e
9722194
84bfcae
335e886
a5598f7
7ea18d7
822e2c5
d1b6922
b13bdd2
a8cff84
0fe3aee
7347447
65c7be7
c4fb104
12abfc5
ed463c0
8c91c8c
7e5081b
e45e376
d21dd7f
8508954
ea689a2
ce5b7dd
7a2324a
acdf3c1
ca89baf
1ccc5f8
724856b
f1ea0da
1d92433
8ce9034
4437083
5f22315
19df253
9d12bf6
71135f1
bbdd7a5
ec6a12f
beb5723
d9a7452
176d676
056bfed
5dbdfc6
d68c421
11e6002
d7e71b9
65e39c5
9fba829
945a224
c060422
3cc6b17
a89e0d7
08416f7
fcc474e
8b176c7
8db4e9d
007a5cc
94da580
c565f4e
7bf3b59
60b2262
e675268
22b981c
0a001b9
6340dbf
27464bb
a11bfc0
15237a2
2d7453e
eca7363
6f69c6c
56da938
dd2839a
7f34a38
b1796d4
872f0c6
da5dcf8
565abe1
4b98e47
6b8ab8e
c21b037
a003a4f
40189e2
7ab58a2
7c6d0e4
70ba3de
4c0be8e
be927dd
c206d67
931b595
bbfe281
7bea8d8
3fe2d3b
726e6ac
d8a38e3
df671b6
fbaa990
1635254
481f921
33b659d
29404ae
af68ef2
7f50d02
cbff55d
447c08a
8bac67f
769c798
afaf266
26e9097
788ee6c
4210798
c4a76fe
23e9407
e17fde6
720add4
4286095
e2b0cd8
8790dee
eb94859
43445e5
dfa3435
b3e2541
a96e873
7c8cfbb
822d1c1
573d5c6
21f11bf
6850713
230bc41
2934c06
cc55b16
a4f4f43
1f2cd49
6b6f901
00415a5
8aabc84
3bc60fc
c556588
91c5825
5688b08
9a24a2e
4e9d386
9077bfb
dce346c
615c301
e450f82
2dc7cc3
97f4868
84dc860
2dea937
b3bd39f
1dd6c3e
654ec75
5e81cf6
7b1567a
8314b0a
aa8a988
fefb435
c41e08a
d93c57e
5f7f95e
d218323
bc1f332
5d1151e
026bdc0
a100de2
d608af4
ec00572
4c7e72b
c93448c
b42cde1
815439c
068a511
b7c8931
189652b
cc3efee
eb9f404
52dc433
0a3b5aa
4c8bfdd
495eb6a
d60e256
f267a09
d644674
98ff478
a4a415a
ba6e788
9ac3505
15aecf0
88ec668
287e23e
f338103
cc47468
8927156
71dc9e7
53efa78
4c997e5
2519585
dbb1599
128bb15
5b860e2
8359678
fbc9888
7405f75
fa3bfd0
6bf27c8
5f1fbf7
0022943
66aacbd
33defd1
ba854f0
50bdc45
4a95578
cb9ebf9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| '@rocket.chat/model-typings': minor | ||
| '@rocket.chat/core-typings': minor | ||
| '@rocket.chat/models': minor | ||
| '@rocket.chat/i18n': minor | ||
| '@rocket.chat/meteor': minor | ||
| --- | ||
|
|
||
| Introduces the ability to upload multiple files and send it into a single message | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,12 @@ import type { | |
| AtLeast, | ||
| FilesAndAttachments, | ||
| IMessage, | ||
| FileProp, | ||
| } from '@rocket.chat/core-typings'; | ||
| import type { ServerMethods } from '@rocket.chat/ddp-client'; | ||
| import { Logger } from '@rocket.chat/logger'; | ||
| import { Rooms, Uploads, Users } from '@rocket.chat/models'; | ||
| import { wrapExceptions } from '@rocket.chat/tools'; | ||
| import { Match, check } from 'meteor/check'; | ||
| import { Meteor } from 'meteor/meteor'; | ||
|
|
||
|
|
@@ -22,13 +25,36 @@ import { FileUpload } from '../lib/FileUpload'; | |
|
|
||
| function validateFileRequiredFields(file: Partial<IUpload>): asserts file is AtLeast<IUpload, '_id' | 'name' | 'type' | 'size'> { | ||
| const requiredFields = ['_id', 'name', 'type', 'size']; | ||
| requiredFields.forEach((field) => { | ||
| for (const field of requiredFields) { | ||
| if (!Object.keys(file).includes(field)) { | ||
| throw new Meteor.Error('error-invalid-file', 'Invalid file'); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
Comment on lines
+26
to
37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation checks key presence but not value truthiness.
Consider checking that the value is actually present and meaningful: 🛡️ Proposed fix function validateFilesRequiredFields(files: Partial<IUpload>[]): asserts files is MinimalUploadData[] {
const requiredFields = ['_id', 'name', 'type', 'size'];
for (const file of files) {
- const fields = Object.keys(file);
-
for (const field of requiredFields) {
- if (!fields.includes(field)) {
+ if (!(field in file) || file[field as keyof IUpload] == null) {
throw new Meteor.Error('error-invalid-file', 'Invalid file');
}
}
}
}🤖 Prompt for AI Agents |
||
|
|
||
| const logger = new Logger('sendFileMessage'); | ||
|
|
||
| export const parseMultipleFilesIntoMessageAttachments = async ( | ||
| filesToConfirm: Partial<IUpload>[], | ||
| roomId: string, | ||
| user: IUser, | ||
| ): Promise<{ files: FileProp[]; attachments: MessageAttachment[] }> => { | ||
| const results = await Promise.all( | ||
| filesToConfirm.map((file) => | ||
| wrapExceptions(() => parseFileIntoMessageAttachments(file, roomId, user)).catch(async (error) => { | ||
| // Not an important error, it should not happen and if it happens wil affect the attachment preview in the message object only | ||
| logger.warn({ msg: 'Error processing file: ', file, error }); | ||
| return { files: [], attachments: [] }; | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| return { | ||
| files: results.flatMap(({ files }) => files), | ||
| attachments: results.flatMap(({ attachments }) => attachments), | ||
| }; | ||
| }; | ||
|
|
||
| export const parseFileIntoMessageAttachments = async ( | ||
rodrigok marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| file: Partial<IUpload>, | ||
| roomId: string, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import { Apps } from '@rocket.chat/apps'; | ||
| import { api, Message } from '@rocket.chat/core-services'; | ||
| import type { IMessage, IRoom } from '@rocket.chat/core-typings'; | ||
| import { Messages } from '@rocket.chat/models'; | ||
| import { isE2EEMessage, type IMessage, type IRoom, type IUpload, type IUploadToConfirm } from '@rocket.chat/core-typings'; | ||
| import { Messages, Uploads } from '@rocket.chat/models'; | ||
| import { Match, check } from 'meteor/check'; | ||
|
|
||
| import { parseUrlsInMessage } from './parseUrlsInMessage'; | ||
| import { isRelativeURL } from '../../../../lib/utils/isRelativeURL'; | ||
| import { isURL } from '../../../../lib/utils/isURL'; | ||
| import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; | ||
| import { FileUpload } from '../../../file-upload/server'; | ||
| import { parseMultipleFilesIntoMessageAttachments } from '../../../file-upload/server/methods/sendFileMessage'; | ||
| import { settings } from '../../../settings/server'; | ||
| import { afterSaveMessage } from '../lib/afterSaveMessage'; | ||
| import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener'; | ||
|
|
@@ -212,14 +213,56 @@ export function prepareMessageObject( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Update file names on the Uploads collection, as the names may have changed between the upload and the sending of the message | ||
| * For encrypted rooms, the full `content` of the file is updated as well, as the name is included there | ||
| **/ | ||
| const updateFileNames = async (filesToConfirm: IUploadToConfirm[], isE2E: boolean) => { | ||
| return Promise.all( | ||
| filesToConfirm.map(async (upload) => { | ||
| if (isE2E) { | ||
| // on encrypted files, the `upload.name` is an useless attribute, so it doesn't need to be updated | ||
| // the name will be loaded from the encrypted data on `upload.content` instead | ||
| if (upload.content) { | ||
| await Uploads.updateFileContentById(upload._id, upload.content); | ||
| } | ||
| } else if (upload.name) { | ||
| await Uploads.updateFileNameById(upload._id, upload.name); | ||
| } | ||
| }), | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Validates and sends the message object. | ||
| */ | ||
| export const sendMessage = async function (user: any, message: any, room: any, upsert = false, previewUrls?: string[]) { | ||
| export const sendMessage = async ( | ||
| user: any, | ||
| message: any, | ||
| room: any, | ||
| upsert = false, | ||
| previewUrls?: string[], | ||
| filesToConfirm?: IUploadToConfirm[], | ||
| ) => { | ||
| if (!user || !message || !room._id) { | ||
| return false; | ||
| } | ||
|
|
||
| const isE2E = isE2EEMessage(message); | ||
|
|
||
| if (filesToConfirm) { | ||
| await updateFileNames(filesToConfirm, isE2E); | ||
| } | ||
|
|
||
| const uploadIdsToConfirm = filesToConfirm?.map(({ _id }) => _id); | ||
|
|
||
| if (uploadIdsToConfirm !== undefined && !isE2E) { | ||
| const uploadsToConfirm: Partial<IUpload>[] = await Uploads.findByIds(uploadIdsToConfirm).toArray(); | ||
| const { files, attachments } = await parseMultipleFilesIntoMessageAttachments(uploadsToConfirm, message.rid, user); | ||
| message.files = files; | ||
| message.attachments = attachments; | ||
| } | ||
|
Comment on lines
+262
to
+273
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find Uploads model/collection implementations
fd -e ts -e js | xargs grep -l "findByIds\|confirmTemporaryFiles" | grep -E "(upload|file)" | head -20Repository: RocketChat/Rocket.Chat Length of output: 502 🏁 Script executed: #!/bin/bash
# Search for Uploads class/model definition
rg -n "class Uploads|export.*Uploads\s*=" --type=ts -A 3 | head -50Repository: RocketChat/Rocket.Chat Length of output: 1712 🏁 Script executed: #!/bin/bash
# Find where Uploads methods are defined
fd -e ts -e js -path "*/uploads*" | head -20Repository: RocketChat/Rocket.Chat Length of output: 234 🏁 Script executed: #!/bin/bash
# Read the Uploads model
wc -l packages/models/src/models/Uploads.tsRepository: RocketChat/Rocket.Chat Length of output: 107 🏁 Script executed: #!/bin/bash
# Find method implementations in Uploads model
cat packages/models/src/models/Uploads.tsRepository: RocketChat/Rocket.Chat Length of output: 2888 🏁 Script executed: #!/bin/bash
# Search for confirmTemporaryFiles implementation
rg -n "confirmTemporaryFiles" --type ts -A 20 | head -80Repository: RocketChat/Rocket.Chat Length of output: 3647 🏁 Script executed: #!/bin/bash
# Search for parseMultipleFilesIntoMessageAttachments
rg -n "parseMultipleFilesIntoMessageAttachments" --type ts -B 2 -A 15 | head -100Repository: RocketChat/Rocket.Chat Length of output: 7178 🏁 Script executed: #!/bin/bash
# Find updateFileComplete implementation
rg -n "updateFileComplete" --type ts -B 2 -A 15 | head -150Repository: RocketChat/Rocket.Chat Length of output: 6181 🏁 Script executed: #!/bin/bash
# Check what updateFileComplete does with userId
rg -n "updateFileComplete\s*\(" --type ts -B 2 -A 20 | head -200Repository: RocketChat/Rocket.Chat Length of output: 7676 🏁 Script executed: #!/bin/bash
# Look for the full BaseUploadModel to see all method implementations
cat packages/models/src/models/BaseUploadModel.tsRepository: RocketChat/Rocket.Chat Length of output: 3487 Unauthorized file attachment via unvalidated file ID references — security vulnerability.
Validate file ownership before processing: 🤖 Prompt for AI Agents |
||
|
|
||
| await validateMessage(message, room, user); | ||
| prepareMessageObject(message, room._id, user); | ||
|
|
||
|
|
@@ -292,6 +335,10 @@ export const sendMessage = async function (user: any, message: any, room: any, u | |
| // TODO: is there an opportunity to send returned data to notifyOnMessageChange? | ||
| await afterSaveMessage(message, room); | ||
|
|
||
| if (uploadIdsToConfirm !== undefined) { | ||
| await Uploads.confirmTemporaryFiles(uploadIdsToConfirm, user._id); | ||
pierre-lehnen-rc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| void notifyOnMessageChange({ id: message._id }); | ||
|
|
||
| void notifyOnRoomChangedById(message.rid); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.