Skip to content

fix: Image orientation issues #38082

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
yash-rajpal wants to merge 6 commits into from
Closed

fix: Image orientation issues #38082

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e7a2194
fix: image orientation when stripping exif
yash-rajpal Jan 6, 2026
4ab0446
add changeset
yash-rajpal Jan 6, 2026
5c31804
Merge branch 'develop' into fix/image-orientation
dougfabris Jan 7, 2026
9852db5
Merge branch 'develop' into fix/image-orientation
yash-rajpal Jan 7, 2026
48b0c00
Merge branch 'develop' into fix/image-orientation
yash-rajpal Jan 8, 2026
4de70cd
use streams
yash-rajpal Jan 15, 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
5 changes: 5 additions & 0 deletions .changeset/dry-pumpkins-design.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 image orientation issues which were related to `Message_Attachments_Strip_Exif` setting.
11 changes: 2 additions & 9 deletions apps/meteor/app/api/server/v1/rooms.ts
View file
Open in desktop
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 @@ -208,7 +208,7 @@ API.v1.addRoute(
throw new Meteor.Error('invalid-field');
}

let { fileBuffer } = file;
const { fileBuffer } = file;

const expiresAt = new Date();
expiresAt.setHours(expiresAt.getHours() + 24);
Expand Down Expand Up @@ -236,13 +236,6 @@ API.v1.addRoute(
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;
}

const fileStore = FileUpload.getStore('Uploads');
const uploadedFile = await fileStore.insert(details, fileBuffer);

Expand Down
9 changes: 9 additions & 0 deletions apps/meteor/app/file-upload/server/lib/FileUpload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import URL from 'url';
import { hashLoginToken } from '@rocket.chat/account-utils';
import { Apps, AppEvents } from '@rocket.chat/apps';
import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions';
import { Media } from '@rocket.chat/core-services';
import { isE2EEUpload, type IUpload } from '@rocket.chat/core-typings';
import { Users, Avatars, UserDataFiles, Uploads, Settings, Subscriptions, Messages, Rooms } from '@rocket.chat/models';
import type { NextFunction } from 'connect';
Expand Down Expand Up @@ -404,6 +405,14 @@ export const FileUpload = {

await reorientation();

const stripExif = settings.get('Message_Attachments_Strip_Exif');

if (stripExif) {
const fileBuffer = fs.readFileSync(tmpFile);
const strippedFileBuffer = await Media.stripExifFromBuffer(fileBuffer);
fs.writeFileSync(tmpFile, strippedFileBuffer);
}

@d-gubert d-gubert Jan 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not use the sync versions of the FS methods, especially when writing a potentially large amount of data, as it blocks the event loop.

Also, here the server will end up buffering the file contents 2 additional times: one reading the file, and another one with the MediaService call. You could unify this by calling Media.stripExifFromStream, passing a read stream from fs.createReadStream - this would prevent any buffering from happening

@d-gubert d-gubert Jan 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from that, sharp already removes exif metadata when rotating the image, so we don't need to call the media service in those cases

Comment on lines +410 to +441

@coderabbitai coderabbitai Bot Jan 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clean up debug artifacts and add missing error handling.

Several issues in the EXIF stripping logic:

  1. Line 437: Replace console.log('ERROR', err) with proper logging—it's redundant with line 439 anyway.
  2. Lines 426-433: Remove commented-out code.
  3. Line 419: Missing error handler for exifTransformer stream—if the transformer emits an error, the promise won't reject.
  4. Line 438: unlink(exifTmpPath) may throw if the file was never created (e.g., error occurred during createReadStream).
🛠️ Proposed fix 
 	if (stripExif) {
 		const exifTmpPath = `${tmpFile}.exif-stripped`;

 		try {
 			await new Promise<void>((resolve, reject) => {
 				const readStream = fs.createReadStream(tmpFile);
 				const writeStream = fs.createWriteStream(exifTmpPath);
 				const exifTransformer = new ExifTransformer();

 				readStream.pipe(exifTransformer).pipe(writeStream);
 				writeStream.on('finish', () => resolve());
 				readStream.on('error', reject);
+				exifTransformer.on('error', reject);
 				writeStream.on('error', reject);
 			});
-			// No need to check mime. Library will ignore any files without exif/xmp tags (like BMP, ico, PDF, etc)
-			// const exifTransformer = new ExifTransformer();
-			// const readStream = fs.createReadStream(tmpFile);
-			// const writeStream = fs.createWriteStream(exifTmpPath);
-
-			// readStream.pipe(exifTransformer).pipe(writeStream);
-
-			// await pipeline(fs.createReadStream(tmpFile), exifTransformer, fs.createWriteStream(exifTmpPath));

 			await rename(exifTmpPath, tmpFile);
 		} catch (err) {
-			console.log('ERROR', err);
-			await unlink(exifTmpPath);
+			await unlink(exifTmpPath).catch(() => {});
 			SystemLogger.error(`Error stripping exif from image: ${err}`);
 		}
 	}
🤖 Prompt for AI Agents 
In `@apps/meteor/app/file-upload/server/lib/FileUpload.ts` around lines 410 - 441,
Replace the debug console.log and cleanup commented-out snippets, add proper
transformer error handling, and guard unlink so it won't throw if the temp file
was never created: specifically, in the EXIF strip block that uses tmpFile,
exifTmpPath, ExifTransformer, readStream and writeStream, remove the
commented-out pipeline code, replace console.log('ERROR', err) with
SystemLogger.error(`Error stripping exif from image: ${err}`) (or extend the
existing SystemLogger call), attach an 'error' listener to exifTransformer that
rejects the Promise like the other streams, and wrap the await
unlink(exifTmpPath) in a safe check or try/catch (or check fs.exists before
unlink) so unlink won't throw when exifTmpPath was never created.


const { size } = await fs.lstatSync(tmpFile);
await this.getCollection().updateOne(
{ _id: file._id },
Expand Down
Loading