Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
76f5afd
refactor: remove router-level body parsing on form-data requests
ricardogarim Jan 8, 2026
06628de
refactor: remove unnecessary buffering for file size check
ricardogarim Jan 8, 2026
9c1bdd2
refactor: avoid re-buffering before file validation
ricardogarim Jan 8, 2026
35ccddf
refactor: remove unneeded bufferToStream from initialData
ricardogarim Jan 8, 2026
60555df
refactor: add UploadService
ricardogarim Jan 8, 2026
027c99e
refactor: adjust POST /rooms.media/:rid to use UploadService
ricardogarim Jan 8, 2026
0c2facf
refactor: adjust rest of upload routes to use UploadService
ricardogarim Jan 8, 2026
fbc8e35
refactor: use builtin finished api to await the pipe
d-gubert Jan 8, 2026
eb257ec
refactor: streamline removeTempFile in ufsComplete
d-gubert Jan 8, 2026
0de765c
refactor: stripExifFromFile stop calling fs.stat
d-gubert Jan 8, 2026
34ac403
refactor: always warn if cleanup fails
d-gubert Jan 8, 2026
dd7afaf
test: adjust router test to comply with new form-data behavior
ricardogarim Jan 8, 2026
69f9ea6
refactor: remove unnecessary stat call in ufs-store
d-gubert Jan 9, 2026
d22a57e
refactor: use pipeline promises
d-gubert Jan 9, 2026
bf2ac8d
refactor: get file size from fs.WriteStream
d-gubert Jan 9, 2026
fabf791
fix: reject on file truncated as previous implementation
d-gubert Jan 9, 2026
a29a3ae
refactor: reuse static cleanup function
d-gubert Jan 9, 2026
c6370fe
refactor: remove `rawRequest` from route context in favor of existing…
d-gubert Jan 9, 2026
f1dff4f
refactor: array view validation before writeFile
d-gubert Jan 9, 2026
b3669a8
fix: move exif stripping to after image rotation
d-gubert Jan 9, 2026
b386694
chore: remove redundant call to unlink temp file
d-gubert Jan 9, 2026
52555d9
fix: prevent body consumption in logger middleware
d-gubert Jan 11, 2026
f1b8d81
fix: increase maxSize by 1 byte so we don't block files exactly as bi…
d-gubert Jan 11, 2026
7584815
refactor: simplify request type checks in UploadService
d-gubert Jan 11, 2026
1aee7a0
refactor: expose internal http request to route handler
d-gubert Jan 11, 2026
4de8fcd
refactor: revert endpoints that require buffer
d-gubert Jan 13, 2026
64e7839
refactor: settings usage refactor
d-gubert Jan 13, 2026
f68aab1
refactor: remove unnecessary if on finishHandler
d-gubert Jan 13, 2026
78649c6
refactor: rename UploadService to MultipartUploadHandler
d-gubert Jan 13, 2026
6ba1d78
fix: properly handle errors during transform pipeline
d-gubert Jan 13, 2026
975e795
feat: adapt app upload event trigger to new upload changes
d-gubert Jan 15, 2026
72b8ae8
refactor: changed way of identifying finished files
d-gubert Jan 15, 2026
27f875a
fix: types on ufs-filter
d-gubert Jan 15, 2026
2de5cea
refactor: more correct buffer creation from view
d-gubert Jan 15, 2026
89c8fc2
refactor: improve multipart skipping in parseBodyParams
d-gubert Jan 15, 2026
1d5dc95
add changeset
d-gubert Jan 15, 2026
92bada3
refactor: suggestion from review
d-gubert Jan 15, 2026
0b8781e
Merge branch 'develop' into refactor/s3-large-uploads
kodiakhq[bot] Jan 16, 2026
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
6 changes: 6 additions & 0 deletions .changeset/twelve-sheep-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/http-router': patch
'@rocket.chat/meteor': patch
---

Improves file upload flow to prevent buffering of contents in memory
3 changes: 3 additions & 0 deletions apps/meteor/app/api/server/definition.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { IncomingMessage } from 'http';

import type { IUser, LicenseModule } from '@rocket.chat/core-typings';
import type { Logger } from '@rocket.chat/logger';
import type { Method, MethodOf, OperationParams, OperationResult, PathPattern, UrlParams } from '@rocket.chat/rest-typings';
Expand Down Expand Up @@ -184,6 +186,7 @@ export type ActionThis<TMethod extends Method, TPathPattern extends PathPattern,
: // TODO remove the extra (optionals) params when all the endpoints that use these are typed correctly
Partial<OperationParams<TMethod, TPathPattern>>;
readonly request: Request;
readonly incoming: IncomingMessage;

readonly queryOperations: TOptions extends { queryOperations: infer T } ? T : never;
readonly queryFields: TOptions extends { queryFields: infer T } ? T : never;
Expand Down
205 changes: 205 additions & 0 deletions apps/meteor/app/api/server/lib/MultipartUploadHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
import fs from 'fs';
import { IncomingMessage } from 'http';
import type { Stream, Transform } from 'stream';
import { Readable } from 'stream';
import { pipeline } from 'stream/promises';

import { MeteorError } from '@rocket.chat/core-services';
import { Random } from '@rocket.chat/random';
import busboy, { type BusboyConfig } from 'busboy';
import ExifTransformer from 'exif-be-gone';

import { UploadFS } from '../../../../server/ufs';
import { getMimeType } from '../../../utils/lib/mimeTypes';

export type ParsedUpload = {
tempFilePath: string;
filename: string;
mimetype: string;
size: number;
fieldname: string;
};

export type ParseOptions = {
field: string;
maxSize?: number;
allowedMimeTypes?: string[];
transforms?: Transform[]; // Optional transform pipeline (e.g., EXIF stripping)
fileOptional?: boolean;
};

export class MultipartUploadHandler {
static transforms = {
stripExif(): Transform {
return new ExifTransformer();
},
};

static async cleanup(tempFilePath: string): Promise<void> {
try {
await fs.promises.unlink(tempFilePath);
} catch (error: any) {
console.warn(`[UploadService] Failed to cleanup temp file: ${tempFilePath}`, error);
}
}

static async stripExifFromFile(tempFilePath: string): Promise<number> {
const strippedPath = `${tempFilePath}.stripped`;

try {
const writeStream = fs.createWriteStream(strippedPath);

await pipeline(fs.createReadStream(tempFilePath), new ExifTransformer(), writeStream);

await fs.promises.rename(strippedPath, tempFilePath);

return writeStream.bytesWritten;
} catch (error) {
void this.cleanup(strippedPath);

throw error;
}
}

static async parseRequest(
request: IncomingMessage | Request,
options: ParseOptions,
): Promise<{ file: ParsedUpload | null; fields: Record<string, string> }> {
const limits: BusboyConfig['limits'] = { files: 1 };

if (options.maxSize && options.maxSize > 0) {
// We add an extra byte to the configured limit so we don't fail the upload
// of a file that is EXACTLY maxSize
limits.fileSize = options.maxSize + 1;
}

const headers =
request instanceof IncomingMessage ? (request.headers as Record<string, string>) : Object.fromEntries(request.headers.entries());

const bb = busboy({
headers,
defParamCharset: 'utf8',
limits,
});

const fields: Record<string, string> = {};
let parsedFile: ParsedUpload | null = null;
let busboyFinished = false;
let filePendingCount = 0;

const { promise, resolve, reject } = Promise.withResolvers<{
file: ParsedUpload | null;
fields: Record<string, string>;
}>();

const tryResolve = () => {
if (busboyFinished && filePendingCount < 1) {
if (!parsedFile && !options.fileOptional) {
return reject(new MeteorError('error-no-file', 'No file uploaded'));
}
resolve({ file: parsedFile, fields });
}
};

bb.on('field', (fieldname: string, value: string) => {
fields[fieldname] = value;
});

bb.on('file', (fieldname, file, info) => {
const { filename, mimeType } = info;

++filePendingCount;

if (options.field && fieldname !== options.field) {
file.resume();
return reject(new MeteorError('invalid-field'));
}

if (options.allowedMimeTypes && !options.allowedMimeTypes.includes(mimeType)) {
file.resume();
return reject(new MeteorError('error-invalid-file-type', `File type ${mimeType} not allowed`));
}

const fileId = Random.id();
const tempFilePath = UploadFS.getTempFilePath(fileId);

const writeStream = fs.createWriteStream(tempFilePath);

let currentStream: Stream = file;
if (options.transforms?.length) {
const fileDestroyer = file.destroy.bind(file);
for (const transform of options.transforms) {
transform.on('error', fileDestroyer);
currentStream = currentStream.pipe(transform);
}
}

currentStream.pipe(writeStream);

writeStream.on('finish', () => {
if (file.truncated) {
void this.cleanup(tempFilePath);
return reject(new MeteorError('error-file-too-large', 'File size exceeds the allowed limit'));
}

parsedFile = {
tempFilePath,
filename,
mimetype: getMimeType(mimeType, filename),
size: writeStream.bytesWritten,
fieldname,
};

--filePendingCount;

tryResolve();
});

writeStream.on('error', (err) => {
file.destroy();
void this.cleanup(tempFilePath);
reject(new MeteorError('error-file-upload', err.message));
});

file.on('error', (err) => {
writeStream.destroy();
void this.cleanup(tempFilePath);
reject(new MeteorError('error-file-upload', err.message));
});
});

bb.on('finish', () => {
busboyFinished = true;
tryResolve();
});

bb.on('error', (err: any) => {
reject(new MeteorError('error-upload-failed', err.message));
});

bb.on('filesLimit', () => {
reject(new MeteorError('error-too-many-files', 'Too many files in upload'));
});

bb.on('partsLimit', () => {
reject(new MeteorError('error-too-many-parts', 'Too many parts in upload'));
});

bb.on('fieldsLimit', () => {
reject(new MeteorError('error-too-many-fields', 'Too many fields in upload'));
});

if (request instanceof IncomingMessage) {
request.pipe(bb);
} else {
if (!request.body) {
return Promise.reject(new MeteorError('error-no-body', 'Request has no body'));
}

const nodeStream = Readable.fromWeb(request.body as any);
nodeStream.pipe(bb);
}

return promise;
}
}
13 changes: 9 additions & 4 deletions apps/meteor/app/api/server/middlewares/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ export const loggerMiddleware =

let payload = {};

try {
payload = await c.req.raw.clone().json();
// eslint-disable-next-line no-empty
} catch {}
// We don't want to consume the request body stream for multipart requests
if (!c.req.header('content-type')?.includes('multipart/form-data')) {
try {
payload = await c.req.raw.clone().json();
// eslint-disable-next-line no-empty
} catch {}
} else {
payload = '[multipart/form-data]';
}

const log = logger.logger.child({
method: c.req.method,
Expand Down
23 changes: 14 additions & 9 deletions apps/meteor/app/api/server/router.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
/* eslint-disable @typescript-eslint/naming-convention */
import type { IncomingMessage } from 'node:http';

import type { ResponseSchema } from '@rocket.chat/http-router';
import { Router } from '@rocket.chat/http-router';
import type { Context as HonoContext } from 'hono';
import type { Context } from 'hono';

import type { TypedOptions } from './definition';

declare module 'hono' {
interface ContextVariableMap {
'route': string;
type HonoContext = Context<{
Bindings: { incoming: IncomingMessage };
Variables: {
'remoteAddress': string;
'bodyParams-override'?: Record<string, any>;
}
}
};
}>;

export type APIActionContext = {
requestIp: string;
Expand All @@ -21,6 +23,7 @@ export type APIActionContext = {
path: string;
response: any;
route: string;
incoming: IncomingMessage;
};

export type APIActionHandler = (this: APIActionContext, request: Request) => Promise<ResponseSchema<TypedOptions>>;
Expand All @@ -39,9 +42,10 @@ export class RocketChatAPIRouter<
request: req,
extra: { bodyParamsOverride: c.var['bodyParams-override'] || {} },
});

const request = req.raw.clone();

const context = {
const context: APIActionContext = {
requestIp: c.get('remoteAddress'),
urlParams: req.param(),
queryParams,
Expand All @@ -50,7 +54,8 @@ export class RocketChatAPIRouter<
path: req.path,
response: res,
route: req.routePath,
} as APIActionContext;
incoming: c.env.incoming,
};

return action.apply(context, [request]);
};
Expand Down
32 changes: 10 additions & 22 deletions apps/meteor/app/api/server/v1/rooms.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FederationMatrix, Media, MeteorError, Team } from '@rocket.chat/core-services';
import { FederationMatrix, MeteorError, Team } from '@rocket.chat/core-services';
import type { IRoom, IUpload } from '@rocket.chat/core-typings';
import { isPrivateRoom, isPublicRoom } from '@rocket.chat/core-typings';
import { Messages, Rooms, Users, Uploads, Subscriptions } from '@rocket.chat/models';
Expand Down Expand Up @@ -55,7 +55,7 @@ import { API } from '../api';
import { composeRoomWithLastMessage } from '../helpers/composeRoomWithLastMessage';
import { getPaginationItems } from '../helpers/getPaginationItems';
import { getUserFromParams } from '../helpers/getUserFromParams';
import { getUploadFormData } from '../lib/getUploadFormData';
import { MultipartUploadHandler } from '../lib/MultipartUploadHandler';
import {
findAdminRoom,
findAdminRooms,
Expand Down Expand Up @@ -197,24 +197,18 @@ API.v1.addRoute(
return API.v1.forbidden();
}

const file = await getUploadFormData(
{
request: this.request,
},
{ field: 'file', sizeLimit: settings.get<number>('FileUpload_MaxFileSize') },
);
const { file, fields } = await MultipartUploadHandler.parseRequest(this.incoming, {
field: 'file',
maxSize: settings.get<number>('FileUpload_MaxFileSize'),
});

if (!file) {
throw new Meteor.Error('invalid-field');
throw new Meteor.Error('error-no-file-uploaded', 'No file was uploaded');
}

let { fileBuffer } = file;

const expiresAt = new Date();
expiresAt.setHours(expiresAt.getHours() + 24);

const { fields } = file;

let content;

if (fields.content) {
Expand All @@ -228,23 +222,17 @@ API.v1.addRoute(

const details = {
name: file.filename,
size: fileBuffer.length,
size: file.size,
type: file.mimetype,
rid: this.urlParams.rid,
userId: this.userId,
content,
expiresAt,
};

const stripExif = settings.get('Message_Attachments_Strip_Exif');
if (stripExif) {
// No need to check mime. Library will ignore any files without exif/xmp tags (like BMP, ico, PDF, etc)
fileBuffer = await Media.stripExifFromBuffer(fileBuffer);
details.size = fileBuffer.length;
}

// TODO: In the future, we should isolate file receival from storage and post-processing.
const fileStore = FileUpload.getStore('Uploads');
const uploadedFile = await fileStore.insert(details, fileBuffer);
const uploadedFile = await fileStore.insert(details, file.tempFilePath);

uploadedFile.path = FileUpload.getPath(`${uploadedFile._id}/${encodeURI(uploadedFile.name || '')}`);

Expand Down
Loading
Loading