Skip to content

Commit fc8e36d

Browse files
regression: custom sound record breaks when validation error on update (#40696)
1 parent 6fb88e1 commit fc8e36d

2 files changed

Lines changed: 63 additions & 7 deletions

File tree

apps/meteor/app/api/server/v1/custom-sounds.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,13 @@ const customSoundsEndpoints = API.v1
259259
: soundToUpdate.extension;
260260

261261
try {
262+
await insertOrUpdateSound({
263+
_id: fields._id,
264+
name: fields.name,
265+
extension: nextExtension,
266+
previousName: soundToUpdate.name,
267+
previousExtension: soundToUpdate.extension,
268+
});
262269
if (fileBuffer) {
263270
await uploadCustomSound(fileBuffer, computedMimeType, {
264271
_id: fields._id,
@@ -267,13 +274,6 @@ const customSoundsEndpoints = API.v1
267274
extension: nextExtension,
268275
});
269276
}
270-
await insertOrUpdateSound({
271-
_id: fields._id,
272-
name: fields.name,
273-
extension: nextExtension,
274-
previousName: soundToUpdate.name,
275-
previousExtension: soundToUpdate.extension,
276-
});
277277
return API.v1.success({});
278278
} catch (error) {
279279
SystemLogger.error({ error });

apps/meteor/tests/end-to-end/api/custom-sounds.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { randomUUID } from 'crypto';
2+
import fs from 'fs';
23
import path from 'path';
34

45
import type { Credentials } from '@rocket.chat/api-client';
@@ -361,6 +362,61 @@ describe('[CustomSounds]', () => {
361362
.expect(403);
362363
});
363364
});
365+
366+
describe('mutating file on failed validation', () => {
367+
let soundAId: string;
368+
let soundBId: string;
369+
const soundAName = `sound-a-${randomUUID()}`;
370+
const soundBName = `sound-b-${randomUUID()}`;
371+
372+
before(async () => {
373+
soundAId = await createCustomSound(soundAName, mockWavAudioPath);
374+
soundBId = await createCustomSound(soundBName, mockMp3AudioPath);
375+
});
376+
377+
after(async () => {
378+
await deleteCustomSound(soundAId);
379+
await deleteCustomSound(soundBId);
380+
});
381+
382+
it('should not mutate the underlying file or metadata if the update fails validation (e.g., name collision)', async () => {
383+
await request
384+
.post(api('custom-sounds.update'))
385+
.set(credentials)
386+
.attach('sound', mockMp3AudioPath)
387+
.field('_id', soundAId)
388+
.field('name', soundBName)
389+
.expect(400)
390+
.expect((res) => {
391+
expect(res.body).to.have.property('success', false);
392+
expect(res.body).to.have.property('error', 'The custom sound name is already in use [Custom_Sound_Error_Name_Already_In_Use]');
393+
});
394+
395+
await request
396+
.get(api('custom-sounds.getOne'))
397+
.set(credentials)
398+
.query({ _id: soundAId })
399+
.expect(200)
400+
.expect((res) => {
401+
expect(res.body.sound).to.have.property('name', soundAName);
402+
expect(res.body.sound).to.have.property('extension', 'wav');
403+
});
404+
405+
const originalWavBuffer = fs.readFileSync(mockWavAudioPath);
406+
407+
await request
408+
.get(`/custom-sounds/${soundAId}.wav`)
409+
.set(credentials)
410+
.expect(200)
411+
.expect((res) => {
412+
expect(res.headers).to.have.property('content-type', 'audio/wav');
413+
expect(Buffer.isBuffer(res.body)).to.be.true;
414+
expect(originalWavBuffer.equals(res.body)).to.be.true;
415+
});
416+
417+
await request.get(`/custom-sounds/${soundAId}.mp3`).set(credentials).expect(404);
418+
});
419+
});
364420
});
365421

366422
describe('[/custom-sounds.list]', () => {

0 commit comments

Comments
 (0)