fix: video attachments not reproducing on iOS

app/containers/message/components/Attachments/Image/Image.tsx

@@ -69,7 +69,7 @@ export const MessageImage = ({ uri, status, encrypted = false, imagePreview, ima
 		<>
 			{showImage ? (
 				<View style={[containerStyle, borderStyle]}>
-					<Image autoplay={autoplayGifs} style={imageStyle} source={{ uri: encodeURI(uri) }} contentFit='cover' />
+					<Image autoplay={autoplayGifs} style={imageStyle} source={{ uri }} contentFit='cover' />
 				</View>
 			) : null}
 			{['loading', 'to-download'].includes(status) || (status === 'downloaded' && !showImage) ? (

app/containers/message/hooks/__tests__/useFile.test.ts

@@ -2,106 +2,47 @@ import { act, renderHook } from '@testing-library/react-native';
 
 import { useFile } from '../useFile';
 import { type IAttachment } from '../../../../definitions';
-import { getMessageById } from '../../../../lib/database/services/Message';
-import { getThreadMessageById } from '../../../../lib/database/services/ThreadMessage';
-
-jest.mock('../../../../lib/database/services/Message', () => ({
-	getMessageById: jest.fn()
-}));
-
-jest.mock('../../../../lib/database/services/ThreadMessage', () => ({
-	getThreadMessageById: jest.fn()
-}));
-
-const mockGetMessageById = getMessageById as jest.Mock;
-const mockGetThreadMessageById = getThreadMessageById as jest.Mock;
 
 const file = { title: 'original.png', title_link: '/original' } as IAttachment;
 
-// Flushes the async checkMessage effect (two awaited DB reads) inside act so any
-// setIsMessagePersisted update settles before assertions run.
-const flushEffect = () =>
-	act(async () => {
-		await Promise.resolve();
-		await Promise.resolve();
-	});
-
 describe('useFile', () => {
-	beforeEach(() => {
-		jest.clearAllMocks();
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue(undefined);
-	});
-
-	it('returns the file prop and treats forwarded merges as a no-op while the thread message persists', async () => {
-		mockGetThreadMessageById.mockResolvedValue({ id: 'thread-message' });
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
-
-		await flushEffect();
-		expect(mockGetThreadMessageById).toHaveBeenCalledWith('msg-id');
-		expect(mockGetMessageById).not.toHaveBeenCalled();
-		expect(result.current[0]).toBe(file);
-
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).toBe(file);
-	});
-
-	it('stays persisted when no thread message exists but the message is found', async () => {
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue({ id: 'message' });
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
-
-		await flushEffect();
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
-		expect(result.current[0]).toBe(file);
+	it('returns the file prop untouched until something is merged', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
 		expect(result.current[0]).toBe(file);
 	});
 
-	it('becomes not-persisted and returns the merged localFile when neither message is found', async () => {
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
+	it('merges an override on top of the file prop', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		await flushEffect();
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0].title_link).toBe('/forwarded');
 		expect(result.current[0]).not.toBe(file);
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
 		expect(result.current[0].title).toBe('original.png');
 	});
 
-	it('starts not-persisted when messageId is empty and merges forwarded files into localFile', async () => {
-		const { result } = renderHook(() => useFile(file, ''));
+	it('accumulates successive overrides', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		await flushEffect();
-		expect(result.current[0]).toBe(file);
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
+		act(() => result.current[1]({ e2e: 'done' }));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).not.toBe(file);
-		expect(result.current[0].title_link).toBe('/forwarded');
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
+		expect(result.current[0].e2e).toBe('done');
 	});
 
-	// Documents the current contract: isMessagePersisted is seeded once and the effect only ever
-	// flips it to false, so it can't recover to true on a later messageId change. Not fixed: the
-	// only caller keys its list by that same id, so a real id change always remounts the hook fresh.
-	it('stays stuck not-persisted after rerendering with a real persisted messageId (documents current contract)', async () => {
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue(undefined);
-		const { result, rerender } = renderHook(({ messageId }: { messageId: string }) => useFile(file, messageId), {
-			initialProps: { messageId: '' }
+	// The override is what the download resolved to locally, so it has to win over a later `file`
+	// prop that still carries the remote url — that stale prop is exactly the bug this hook fixes.
+	it('keeps overrides applied when the file prop changes', () => {
+		const { result, rerender } = renderHook(({ f }: { f: IAttachment }) => useFile(f), {
+			initialProps: { f: file }
 		});
 
-		await flushEffect();
-		expect(result.current[0]).toBe(file);
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
+		rerender({ f: { ...file, title: 'renamed.png' } as IAttachment });
 
-		mockGetMessageById.mockResolvedValue({ id: 'msg-id' });
-		rerender({ messageId: 'msg-id' });
-		await flushEffect();
-
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).not.toBe(file);
-		expect(result.current[0].title_link).toBe('/forwarded');
+		expect(result.current[0].title).toBe('renamed.png');
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
 	});
 });

app/containers/message/hooks/useFile.tsx

@@ -1,30 +1,14 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 
 import { type IAttachment } from '../../../definitions';
-import { getMessageById } from '../../../lib/database/services/Message';
-import { getThreadMessageById } from '../../../lib/database/services/ThreadMessage';
 
-export const useFile = (file: IAttachment, messageId: string) => {
-	const [localFile, setLocalFile] = useState(file);
-	const [isMessagePersisted, setIsMessagePersisted] = useState(!!messageId);
-	useEffect(() => {
-		const checkMessage = async () => {
-			const threadMessage = await getThreadMessageById(messageId);
-			if (!threadMessage) {
-				const message = await getMessageById(messageId);
-				if (!message) {
-					setIsMessagePersisted(false);
-				}
-			}
-		};
-		checkMessage();
-	}, [messageId]);
+// Merges local overrides (the downloaded uri) over the `file` prop, unconditionally: persistMessage no-ops without a message row.
+export const useFile = (file: IAttachment) => {
+	const [overrides, setOverrides] = useState<Partial<IAttachment> | null>(null);
 
-	const manageForwardedFile = (f: Partial<IAttachment>) => {
-		if (isMessagePersisted) {
-			return;
-		}
-		setLocalFile(prev => ({ ...prev, ...f }));
+	const mergeFile = (f: Partial<IAttachment>) => {
+		setOverrides(prev => ({ ...prev, ...f }));
 	};
-	return [isMessagePersisted ? file : localFile, manageForwardedFile] as const;
+
+	return [overrides ? { ...file, ...overrides } : file, mergeFile] as const;
 };

app/containers/message/hooks/useMediaAutoDownload.tsx

@@ -80,7 +80,7 @@ export const useMediaAutoDownload = ({
 	const baseUrl = useBaseUrl();
 	const user = useMessageUser();
 	const [status, dispatchDownloadEvent] = useReducer(downloadStatusReducer, 'to-download');
-	const [currentFile, setCurrentFile] = useFile(file, id ?? '');
+	const [currentFile, setCurrentFile] = useFile(file);
 	const originalUrl = getOriginalURL(file);
 	const url = formatAttachmentUrl(
 		file.title_link || getFileProperty(currentFile, fileType, 'url'),

app/lib/methods/handleMediaDownload.test.ts

@@ -34,6 +34,30 @@ describe('matchDownloadUrl', () => {
 			matchDownloadUrl({ image_url: '/file-upload/abc/photo.jpg' }, 'https://server.com/file-upload/abc/audio.mp3')
 		).toBeFalsy();
 	});
+
+	it('matches a raw attachment url against an encoded downloadUrl', () => {
+		expect(
+			matchDownloadUrl(
+				{ video_url: '/file-upload/abc/Screen Recording.mov' },
+				'https://server.com/file-upload/abc/Screen%20Recording.mov'
+			)
+		).toBeTruthy();
+	});
+
+	it('matches an encoded attachment url against an encoded downloadUrl', () => {
+		expect(
+			matchDownloadUrl(
+				{ video_url: '/file-upload/abc/Screen%20Recording.mov' },
+				'https://server.com/file-upload/abc/Screen%20Recording.mov'
+			)
+		).toBeTruthy();
+	});
+
+	it('still compares when the attachment url has a malformed escape', () => {
+		expect(
+			matchDownloadUrl({ image_url: '/file-upload/abc/%ZZ.jpg' }, 'https://server.com/file-upload/abc/%ZZ.jpg')
+		).toBeTruthy();
+	});
 });
 
 describe('Test the getFilename', () => {

app/lib/methods/handleMediaDownload.ts

@@ -200,10 +200,22 @@ export async function cancelDownload(messageUrl: string): Promise<void> {
 	}
 }
 
-export const matchDownloadUrl = (att: IAttachment, downloadUrl: string) =>
-	(att.image_url && downloadUrl.includes(att.image_url)) ||
-	(att.audio_url && downloadUrl.includes(att.audio_url)) ||
-	(att.video_url && downloadUrl.includes(att.video_url));
+// decodeURIComponent throws on a malformed escape (`%ZZ`); fall back to the raw value so the comparison still runs.
+const decodeUrl = (url: string) => {
+	try {
+		return decodeURIComponent(url);
+	} catch {
+		return url;
+	}
+};
+
+// `downloadUrl` has been through formatAttachmentUrl (so spaces are `%20`) while the attachment rows carry the
+// server value verbatim. Compare decoded so both forms line up regardless of which one the server sent.
+export const matchDownloadUrl = (att: IAttachment, downloadUrl: string) => {
+	const target = decodeUrl(downloadUrl);
+	const matches = (attachmentUrl?: string) => !!attachmentUrl && target.includes(decodeUrl(attachmentUrl));
+	return matches(att.image_url) || matches(att.audio_url) || matches(att.video_url);
+};
 
 const mapAttachments = ({
 	attachments,
@@ -249,11 +261,14 @@ const persistMessage = async (messageId: string, uri: string, encryption: boolea
 			})
 		);
 	}
-	if (batch.length) {
-		await db.write(async () => {
-			await db.batch(batch);
-		});
+	if (!batch.length) {
+		// Expected when the attachment has no message row (forwarded, attachment lists); useFile keeps the local uri.
+		console.log('[handleMediaDownload] no message row for attachment, cached uri not persisted');
+		return;
 	}
+	await db.write(async () => {
+		await db.batch(batch);
+	});
 };
 
 export function downloadMediaFile({

app/lib/methods/helpers/__tests__/formatAttachmentUrl.test.ts

@@ -0,0 +1,61 @@
+import { encodeAttachmentUrl } from '../formatAttachmentUrl';
+
+describe('encodeAttachmentUrl', () => {
+	it('encodes an unencoded path', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/Screen Recording.mov')).toBe(
+			'https://open.rocket.chat/file-upload/1/Screen%20Recording.mov'
+		);
+	});
+
+	it('leaves an already-encoded path untouched', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/Screen%20Recording.mov')).toBe(
+			'https://open.rocket.chat/file-upload/1/Screen%20Recording.mov'
+		);
+	});
+
+	it('leaves an already-escaped reserved character in the path', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/a%20video%20%232.mov')).toBe(
+			'https://open.rocket.chat/file-upload/1/a%20video%20%232.mov'
+		);
+	});
+
+	// Known limitation, not a regression: `#` is the fragment delimiter per the URL spec, so a raw one ends the
+	// path and the rest becomes the fragment — which HTTP drops, so the server sees a truncated path. The previous
+	// encodeURI behaved identically (it leaves `#` unescaped too). Only reachable if the server sends a raw `#`.
+	it('treats a raw reserved `#` in the path as a fragment', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/a video #2.mov')).toBe(
+			'https://open.rocket.chat/file-upload/1/a%20video%20#2.mov'
+		);
+	});
+
+	it('preserves the query string', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/Screen Recording.mov?rc_token=abc&rc_uid=123')).toBe(
+			'https://open.rocket.chat/file-upload/1/Screen%20Recording.mov?rc_token=abc&rc_uid=123'
+		);
+	});
+
+	// WHATWG URL passes a malformed escape through rather than throwing, so this exercises the try branch.
+	it('returns the raw url when it has a malformed escape', () => {
+		expect(encodeAttachmentUrl('https://open.rocket.chat/file-upload/1/%ZZ.mov')).toBe(
+			'https://open.rocket.chat/file-upload/1/%ZZ.mov'
+		);
+	});
+
+	// A non-absolute url is what actually throws — reachable when the server/CDN prefix is empty.
+	it('returns the raw url when it is not absolute', () => {
+		expect(encodeAttachmentUrl('/file-upload/1/Screen Recording.mov')).toBe('/file-upload/1/Screen Recording.mov');
+	});
+
+	// Cache filenames keep unicode letters, so local uris need encoding before they reach the native players.
+	it('encodes unicode letters in a local file uri', () => {
+		expect(encodeAttachmentUrl('file:///var/app/Documents/server/msg1/vídeo.mov')).toBe(
+			'file:///var/app/Documents/server/msg1/v%C3%ADdeo.mov'
+		);
+	});
+
+	it('leaves an already-encoded local file uri untouched', () => {
+		expect(encodeAttachmentUrl('file:///var/app/Documents/server/msg1/v%C3%ADdeo.mov')).toBe(
+			'file:///var/app/Documents/server/msg1/v%C3%ADdeo.mov'
+		);
+	});
+});

app/lib/methods/helpers/formatAttachmentUrl.ts

@@ -9,6 +9,14 @@ function setParamInUrl({ url, token, userId }: { url: string; token: string; use
 	urlObj.searchParams.set('rc_uid', userId);
 	return urlObj.toString();
 }
+// Idempotent encode: the server already sends encoded paths, and a plain encodeURI would turn `%20` into `%2520`.
+export const encodeAttachmentUrl = (url: string): string => {
+	try {
+		return new URL(url).toString();
+	} catch {
+		return url;
+	}
+};
 
 export const formatAttachmentUrl = (
 	attachmentUrl: string | undefined,
@@ -19,26 +27,32 @@ export const formatAttachmentUrl = (
 ): string => {
 	const protectFiles = store.getState().settings.FileUpload_ProtectFiles;
 
-	if ((attachmentUrl && isImageBase64(attachmentUrl)) || attachmentUrl?.startsWith('file://')) {
+	// A data: uri is already its own encoding — running it through the parser would corrupt the payload.
+	if (attachmentUrl && isImageBase64(attachmentUrl)) {
 		return attachmentUrl;
 	}
+	// Cache filenames keep unicode letters (sanitizeLikeString only strips `[^\p{L}\p{Nd}]`), so `vídeo.mov` reaches
+	// here verbatim and the native players reject the unescaped path. Encode local uris too.
+	if (attachmentUrl?.startsWith('file://')) {
+		return encodeAttachmentUrl(attachmentUrl);
+	}
 	if (attachmentUrl && attachmentUrl.startsWith('http')) {
 		if (_originalUrl && !_originalUrl.startsWith(server)) {
-			return _originalUrl;
+			return encodeAttachmentUrl(_originalUrl);
 		}
 
 		if (attachmentUrl.includes('rc_token')) {
-			return encodeURI(attachmentUrl);
+			return encodeAttachmentUrl(attachmentUrl);
 		}
 
-		if (protectFiles) return setParamInUrl({ url: attachmentUrl, token, userId });
-		return attachmentUrl;
+		if (protectFiles) return encodeAttachmentUrl(setParamInUrl({ url: attachmentUrl, token, userId }));
+		return encodeAttachmentUrl(attachmentUrl);
 	}
 	let cdnPrefix = store?.getState().settings.CDN_PREFIX as string;
 	cdnPrefix = cdnPrefix?.trim();
 	if (cdnPrefix && cdnPrefix.startsWith('http')) {
 		server = cdnPrefix.replace(/\/+$/, '');
 	}
-	if (protectFiles) return setParamInUrl({ url: `${server}${attachmentUrl}`, token, userId });
-	return `${server}${attachmentUrl}`;
+	if (protectFiles) return encodeAttachmentUrl(setParamInUrl({ url: `${server}${attachmentUrl}`, token, userId }));
+	return encodeAttachmentUrl(`${server}${attachmentUrl}`);
 };