Skip to content

Commit fa9a763

Browse files
authored
fix(tauri): deduplicate concurrent media fetches (#1205)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes # #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. -->
2 parents b0cd802 + 3000ea1 commit fa9a763

18 files changed

Lines changed: 679 additions & 140 deletions

src-tauri/src/network/media_protocol.rs

Lines changed: 259 additions & 63 deletions
Large diffs are not rendered by default.

src/app/components/url-preview/UrlPreviewCard.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ export const UrlPreviewCard = as<
8686
const mx = useMatrixClient();
8787
const useAuthentication = useMediaAuthentication();
8888
const [linkPreviewImageMaxHeight] = useSetting(settingsAtom, 'linkPreviewImageMaxHeight');
89+
const [mediaAutoLoad] = useSetting(settingsAtom, 'mediaAutoLoad');
8990

9091
const [previewStatus, loadPreview] = useAsyncCallback(
9192
useCallback(() => {
@@ -281,7 +282,7 @@ export const UrlPreviewCard = as<
281282
}}
282283
mediaLayout="contained"
283284
fillsPreviewSlot
284-
autoPlay
285+
autoPlay={mediaAutoLoad}
285286
onAuxClick={handleAuxClick}
286287
body={prev['og:title']}
287288
url={prev['og:image']}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
22

3-
export type DesktopRuntimeState = { trayAvailable: boolean };
3+
export type DesktopRuntimeState = { trayAvailable: boolean, };
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
22

3-
export type DesktopSettings = { closeToBackgroundOnClose: boolean; showSystemTrayIcon: boolean };
3+
export type DesktopSettings = { closeToBackgroundOnClose: boolean, showSystemTrayIcon: boolean, };

src/app/pages/client/ClientRoot.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export function ClientRoot({ children }: ClientRootProps) {
272272
log.log('initClient for', activeSession.userId);
273273
const newMx = await initClient(activeSession);
274274
loadedUserIdRef.current = activeSession.userId;
275-
pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
275+
await pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
276276
return newMx;
277277
}, [activeSession, activeSessionId, setActiveSessionId])
278278
);
@@ -311,7 +311,7 @@ export function ClientRoot({ children }: ClientRootProps) {
311311
activeSession.userId,
312312
'— reloading client'
313313
);
314-
pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
314+
void pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
315315
if (mx?.clientRunning) {
316316
stopClient(mx);
317317
}

src/app/utils/matrix.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import type { MatrixClient } from '$types/matrix-sdk';
23

34
const tauriApi = vi.hoisted(() => ({
45
isTauri: vi.fn<() => boolean>(),
@@ -15,7 +16,7 @@ const mediaTransport = vi.hoisted(() => ({
1516
vi.mock('@tauri-apps/api/core', () => tauriApi);
1617
vi.mock('./mediaTransport', () => mediaTransport);
1718

18-
const { rewriteAuthenticatedMediaUrl } = await import('./matrix');
19+
const { mxcUrlToHttp, rewriteAuthenticatedMediaUrl } = await import('./matrix');
1920

2021
describe('rewriteAuthenticatedMediaUrl', () => {
2122
beforeEach(() => {
@@ -59,6 +60,36 @@ describe('rewriteAuthenticatedMediaUrl', () => {
5960
);
6061
});
6162

63+
it.each([
64+
'/_matrix/media/v3/download/example.org/abc123',
65+
'/_matrix/media/v3/thumbnail/example.org/abc123?width=96&height=96&method=crop',
66+
'/_matrix/media/r0/download/example.org/abc123',
67+
'/_matrix/media/r0/thumbnail/example.org/abc123?width=96&height=96&method=crop',
68+
])('rewrites legacy media paths under Tauri: %s', (path) => {
69+
tauriApi.isTauri.mockReturnValue(true);
70+
const url = `https://matrix.example.org${path}`;
71+
const separator = url.includes('?') ? '&' : '?';
72+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(
73+
`sable-media://${url}${separator}__sable_media_cache=2&__sable_media_session=%40user%3Aexample.com`
74+
);
75+
});
76+
77+
it('does not rewrite unrelated Matrix media URLs', () => {
78+
tauriApi.isTauri.mockReturnValue(true);
79+
const url = 'https://matrix.example.org/_matrix/media/v3/config';
80+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(url);
81+
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
82+
});
83+
84+
it.each([
85+
'https://example.org/avatar.png?next=/_matrix/media/v3/download/example.org/abc123',
86+
'https://example.org/avatar.png#/_matrix/media/r0/thumbnail/example.org/abc123',
87+
])('does not rewrite a media path only present in query or hash: %s', (url) => {
88+
tauriApi.isTauri.mockReturnValue(true);
89+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(url);
90+
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
91+
});
92+
6293
it('passes through already-rewritten sable-media:// URLs', () => {
6394
tauriApi.isTauri.mockReturnValue(true);
6495
const url =
@@ -69,3 +100,17 @@ describe('rewriteAuthenticatedMediaUrl', () => {
69100
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
70101
});
71102
});
103+
104+
describe('mxcUrlToHttp', () => {
105+
it('rewrites SDK legacy media URLs under Tauri without useAuthentication', () => {
106+
tauriApi.isTauri.mockReturnValue(true);
107+
const legacyUrl = 'https://matrix.example.org/_matrix/media/v3/download/example.org/video';
108+
const mx = {
109+
mxcUrlToHttp: vi.fn<() => string>(() => legacyUrl),
110+
} as unknown as MatrixClient;
111+
112+
expect(mxcUrlToHttp(mx, 'mxc://example.org/video', false)).toBe(
113+
`sable-media://${legacyUrl}?__sable_media_cache=2&__sable_media_session=%40user%3Aexample.com`
114+
);
115+
});
116+
});

src/app/utils/matrix.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ import {
3636

3737
const DOMAIN_REGEX = /\b(?:[a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}\b/;
3838
const TAURI_MEDIA_CACHE_VERSION = '__sable_media_cache=2';
39+
const TAURI_MEDIA_PATH_PREFIXES = [
40+
'/_matrix/client/v1/media/',
41+
'/_matrix/media/v3/download/',
42+
'/_matrix/media/v3/thumbnail/',
43+
'/_matrix/media/r0/download/',
44+
'/_matrix/media/r0/thumbnail/',
45+
];
3946

4047
export const isServerName = (serverName: string): boolean => DOMAIN_REGEX.test(serverName);
4148

@@ -478,7 +485,22 @@ export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string):
478485
export const rewriteAuthenticatedMediaUrl = (httpUrl: string | null): string | null => {
479486
if (!httpUrl) return null;
480487
if (!isTauri()) return httpUrl;
481-
if (!httpUrl.includes('/_matrix/client/v1/media/')) return httpUrl;
488+
const sourceUrl = httpUrl.startsWith('sable-media://')
489+
? httpUrl.slice('sable-media://'.length)
490+
: httpUrl;
491+
let parsedUrl: URL;
492+
try {
493+
parsedUrl = new URL(sourceUrl);
494+
} catch {
495+
return httpUrl;
496+
}
497+
if (
498+
(parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') ||
499+
parsedUrl.origin === 'null' ||
500+
!TAURI_MEDIA_PATH_PREFIXES.some((path) => parsedUrl.pathname.startsWith(path))
501+
) {
502+
return httpUrl;
503+
}
482504
if (httpUrl.includes(TAURI_MEDIA_CACHE_VERSION)) return httpUrl;
483505
const mediaUrl = httpUrl.startsWith('sable-media://')
484506
? httpUrl
@@ -508,9 +530,7 @@ export const mxcUrlToHttp = (
508530
useAuthentication
509531
);
510532

511-
// Authenticated media has no service worker under Tauri to attach the token, so route it
512-
// through the native sable-media:// protocol which injects the token in Rust.
513-
if (httpUrl && useAuthentication) {
533+
if (httpUrl && isTauri()) {
514534
return rewriteAuthenticatedMediaUrl(httpUrl);
515535
}
516536
return httpUrl;

src/app/utils/mediaTransport.test.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ describe('fetchMediaBlob', () => {
356356
expect(headersSeen).toEqual([null]);
357357
});
358358

359-
it('retries once on the service worker path without direct auth headers', async () => {
359+
it('fetches once on the service worker path when it returns an auth error', async () => {
360360
platform.hasControllingServiceWorker.mockReturnValue(true);
361361
const { fetchMediaBlob } = await import('./mediaTransport');
362362
const url = 'https://example.org/auth-media.png';
@@ -365,17 +365,13 @@ describe('fetchMediaBlob', () => {
365365
vi.mocked(fetch).mockImplementation(async (_input, init) => {
366366
const headers = new Headers(init?.headers);
367367
headersSeen.push(headers.get('authorization'));
368-
if (headersSeen.length === 1) {
369-
return new Response('denied', { status: 403 });
370-
}
371-
return new Response('ok', { status: 200 });
368+
return new Response('denied', { status: 403 });
372369
});
373370

374-
const blob = await fetchMediaBlob(url);
371+
await expect(fetchMediaBlob(url)).rejects.toThrow('Failed to fetch media: 403');
375372

376-
expect(await blob.text()).toBe('ok');
377-
expect(headersSeen).toEqual([null, null]);
378-
expect(fetch).toHaveBeenCalledTimes(2);
373+
expect(headersSeen).toEqual([null]);
374+
expect(fetch).toHaveBeenCalledTimes(1);
379375
});
380376

381377
it('bypasses the service worker path when explicit auth overrides are provided', async () => {

src/app/utils/mediaTransport.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,6 @@ async function fetchMediaBlobInternal(url: string, options?: MediaTransportOptio
296296
};
297297

298298
if (useServiceWorker) {
299-
const response = await fetchMediaResponse(url, undefined, cacheMode);
300-
if (response.ok || !isRetryableAuthError(response)) {
301-
return fetchAndCache(response);
302-
}
303299
return fetchAndCache(await fetchMediaResponse(url, undefined, cacheMode));
304300
}
305301

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const tauriApi = vi.hoisted(() => ({
4+
isTauri: vi.fn<() => boolean>(),
5+
}));
6+
const commands = vi.hoisted(() => ({
7+
clearMediaSession: vi.fn<() => Promise<void>>(),
8+
setMediaSession:
9+
vi.fn<({ baseUrl, token }: { baseUrl: string; token: string }) => Promise<void>>(),
10+
}));
11+
const mediaTransport = vi.hoisted(() => ({
12+
getActiveMediaSession: vi.fn<() => { baseUrl: string; accessToken: string } | undefined>(),
13+
}));
14+
15+
vi.mock('@tauri-apps/api/core', () => tauriApi);
16+
vi.mock('$generated/tauri/commands', () => commands);
17+
vi.mock('./mediaTransport', () => mediaTransport);
18+
19+
const { initTauriMediaSession, updateTauriMediaSession } = await import('./tauriMediaAuth');
20+
21+
describe('Tauri media session coordinator', () => {
22+
beforeEach(() => {
23+
vi.clearAllMocks();
24+
tauriApi.isTauri.mockReturnValue(true);
25+
commands.clearMediaSession.mockResolvedValue();
26+
commands.setMediaSession.mockResolvedValue();
27+
});
28+
29+
it('serializes writes and applies the last requested state', async () => {
30+
let resolveFirst: (() => void) | undefined;
31+
commands.setMediaSession
32+
.mockImplementationOnce(
33+
() =>
34+
new Promise<void>((resolve) => {
35+
resolveFirst = resolve;
36+
})
37+
)
38+
.mockResolvedValueOnce();
39+
40+
const first = updateTauriMediaSession('https://one.example', 'one');
41+
const second = updateTauriMediaSession('https://two.example', 'two');
42+
const clear = updateTauriMediaSession();
43+
44+
await Promise.resolve();
45+
expect(commands.setMediaSession).toHaveBeenCalledTimes(1);
46+
resolveFirst?.();
47+
await Promise.all([first, second, clear]);
48+
49+
expect(commands.setMediaSession).toHaveBeenNthCalledWith(1, {
50+
baseUrl: 'https://one.example',
51+
token: 'one',
52+
});
53+
expect(commands.setMediaSession).toHaveBeenNthCalledWith(2, {
54+
baseUrl: 'https://two.example',
55+
token: 'two',
56+
});
57+
expect(commands.clearMediaSession).toHaveBeenCalledTimes(1);
58+
});
59+
60+
it('waits for the initial active session write', async () => {
61+
let resolveWrite: (() => void) | undefined;
62+
mediaTransport.getActiveMediaSession.mockReturnValue({
63+
baseUrl: 'https://matrix.example',
64+
accessToken: 'token',
65+
});
66+
commands.setMediaSession.mockImplementation(
67+
() =>
68+
new Promise<void>((resolve) => {
69+
resolveWrite = resolve;
70+
})
71+
);
72+
73+
const ready = initTauriMediaSession();
74+
let complete = false;
75+
void ready.then(() => {
76+
complete = true;
77+
});
78+
await Promise.resolve();
79+
expect(complete).toBe(false);
80+
81+
resolveWrite?.();
82+
await ready;
83+
expect(complete).toBe(true);
84+
});
85+
});

0 commit comments

Comments
 (0)