Skip to content

Commit 730dadf

Browse files
authored
fix(media): fall back when service worker auth is unavailable (#1449)
<!-- 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 web media for video (I hope) according to the log I received #### 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 - [x] 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 d312910 + 2f013c1 commit 730dadf

7 files changed

Lines changed: 213 additions & 36 deletions

File tree

src/app/components/message/content/VideoContent.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export const VideoContent = as<'div', VideoContentProps>(
107107
// support; a stale SW build would otherwise serve the bare URL to
108108
// the homeserver and the element would fail with a 4xx.
109109
if (!preferBlobRef.current && (await probeSWMediaAuthSupport())) return mediaUrl;
110-
return createObjectURL(downloadMedia(mediaUrl));
110+
return createObjectURL(downloadMedia(mediaUrl, { forceDirectAuth: true }));
111111
}
112112
if (isTauri()) {
113113
await setMediaEncryption(mediaUrl, encInfo, mimeType);

src/app/utils/mediaTransport.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,48 @@ describe('fetchMediaBlob', () => {
251251
expect(mediaCache.putInMediaCache).toHaveBeenCalledTimes(1);
252252
});
253253

254+
it('does not share an inflight SW request with forced direct auth', async () => {
255+
swMediaAuth.getCachedSWMediaAuthSupport.mockReturnValue(true);
256+
const { fetchMediaBlob } = await import('./mediaTransport');
257+
const url = 'https://matrix.example.org/_matrix/client/v1/media/download/example.org/media-id';
258+
const headersSeen: Array<string | null> = [];
259+
let resolveSWRequest!: (response: Response) => void;
260+
const pendingSWRequest = new Promise<Response>((resolve) => {
261+
resolveSWRequest = resolve;
262+
});
263+
264+
localStorage.setItem(
265+
'matrixSessions',
266+
JSON.stringify([
267+
{
268+
baseUrl: 'https://matrix.example.org',
269+
userId: '@alice:example.org',
270+
deviceId: 'DEVICE',
271+
accessToken: 'token-1',
272+
},
273+
])
274+
);
275+
localStorage.setItem('matrixActiveSession', '@alice:example.org');
276+
277+
vi.mocked(fetch).mockImplementation(async (_input, init) => {
278+
const authorization = new Headers(init?.headers).get('authorization');
279+
headersSeen.push(authorization);
280+
if (authorization === null) return pendingSWRequest;
281+
return new Response('direct', { status: 200 });
282+
});
283+
284+
const ordinaryRequest = fetchMediaBlob(url);
285+
await vi.waitFor(() => expect(fetch).toHaveBeenCalledTimes(1));
286+
287+
const directRequest = fetchMediaBlob(url, { forceDirectAuth: true });
288+
await vi.waitFor(() => expect(fetch).toHaveBeenCalledTimes(2));
289+
await expect(directRequest).resolves.toHaveProperty('size', 6);
290+
expect(headersSeen).toEqual([null, 'Bearer token-1']);
291+
292+
resolveSWRequest(new Response('ordinary', { status: 200 }));
293+
await expect(ordinaryRequest).resolves.toHaveProperty('size', 8);
294+
});
295+
254296
it('re-resolves auth once after a 401 in direct-fetch mode', async () => {
255297
const { fetchMediaBlob } = await import('./mediaTransport');
256298
const url = 'https://matrix.example.org/_matrix/client/v1/media/download/example.org/media-id';
@@ -397,6 +439,37 @@ describe('fetchMediaBlob', () => {
397439
expect(headersSeen).toEqual(['Bearer widget-token']);
398440
});
399441

442+
it('bypasses the service worker path when direct auth is forced', async () => {
443+
swMediaAuth.getCachedSWMediaAuthSupport.mockReturnValue(true);
444+
const { fetchMediaBlob } = await import('./mediaTransport');
445+
const url = 'https://matrix.example.org/_matrix/client/v1/media/download/example.org/media-id';
446+
const headersSeen: Array<string | null> = [];
447+
448+
localStorage.setItem(
449+
'matrixSessions',
450+
JSON.stringify([
451+
{
452+
baseUrl: 'https://matrix.example.org',
453+
userId: '@alice:example.org',
454+
deviceId: 'DEVICE',
455+
accessToken: 'token-1',
456+
},
457+
])
458+
);
459+
localStorage.setItem('matrixActiveSession', '@alice:example.org');
460+
461+
vi.mocked(fetch).mockImplementation(async (_input, init) => {
462+
const headers = new Headers(init?.headers);
463+
headersSeen.push(headers.get('authorization'));
464+
return new Response('ok', { status: 200 });
465+
});
466+
467+
const blob = await fetchMediaBlob(url, { forceDirectAuth: true });
468+
469+
expect(await blob.text()).toBe('ok');
470+
expect(headersSeen).toEqual(['Bearer token-1']);
471+
});
472+
400473
it('uses direct auth fetches when service workers are supported but not controlling', async () => {
401474
swMediaAuth.getCachedSWMediaAuthSupport.mockReturnValue(false);
402475
const { fetchMediaBlob } = await import('./mediaTransport');

src/app/utils/mediaTransport.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export type MediaFetchCacheMode = 'default' | 'reload' | 'bypass';
1212

1313
export type MediaTransportOptions = {
1414
cache?: MediaFetchCacheMode;
15+
forceDirectAuth?: boolean;
1516
accessToken?: string | null;
1617
getAccessToken?: () => string | null | undefined;
1718
sessionScope?: string;
@@ -179,8 +180,13 @@ function getFetchCacheMode(cacheMode: MediaFetchCacheMode): RequestCache {
179180
return 'default';
180181
}
181182

182-
function getRequestKey(url: string, cacheMode: MediaFetchCacheMode): string {
183-
return `${cacheMode}:${getStableMediaCacheKeyFragment(url)}`;
183+
function getRequestKey(
184+
url: string,
185+
cacheMode: MediaFetchCacheMode,
186+
forceDirectAuth: boolean
187+
): string {
188+
const transportMode = forceDirectAuth ? 'direct-auth' : 'default';
189+
return `${transportMode}:${cacheMode}:${getStableMediaCacheKeyFragment(url)}`;
184190
}
185191

186192
type MatrixMediaInfo = {
@@ -290,7 +296,9 @@ async function fetchMediaBlobInternal(url: string, options?: MediaTransportOptio
290296
// Only let the service worker attach the token once it has proven media-auth
291297
// support; a stale SW build would forward the request bare and get a 4xx.
292298
const useServiceWorker =
293-
getCachedSWMediaAuthSupport() === true && !hasExplicitMediaAuthOverride(options);
299+
getCachedSWMediaAuthSupport() === true &&
300+
!hasExplicitMediaAuthOverride(options) &&
301+
!options?.forceDirectAuth;
294302
const fetchAndCache = async (response: Response): Promise<Blob> => {
295303
if (!response.ok) {
296304
throw new Error(`Failed to fetch media: ${response.status} ${response.statusText}`);
@@ -351,7 +359,8 @@ export async function fetchMediaBlob(url: string, options?: MediaTransportOption
351359
const cacheMode = options?.cache ?? 'default';
352360
const requestKey = getRequestKey(
353361
getScopedMediaCacheKey(url, resolveSessionScope(options)),
354-
cacheMode
362+
cacheMode,
363+
options?.forceDirectAuth === true
355364
);
356365

357366
const inflight = inflightRequests.get(requestKey);

src/app/utils/swMediaAuth.test.ts

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@ const platform = vi.hoisted(() => ({
66

77
vi.mock('$utils/platform', () => platform);
88

9-
function stubServiceWorker(controller: unknown): void {
9+
function stubServiceWorker(controller: unknown) {
10+
const serviceWorker = {
11+
controller,
12+
addEventListener: vi.fn<(...args: unknown[]) => void>(),
13+
removeEventListener: vi.fn<(...args: unknown[]) => void>(),
14+
};
1015
Object.defineProperty(navigator, 'serviceWorker', {
1116
configurable: true,
12-
value: {
13-
controller,
14-
addEventListener: vi.fn<(...args: unknown[]) => void>(),
15-
removeEventListener: vi.fn<(...args: unknown[]) => void>(),
16-
},
17+
value: serviceWorker,
1718
});
19+
return serviceWorker;
1820
}
1921

2022
describe('swMediaAuth', () => {
@@ -24,6 +26,7 @@ describe('swMediaAuth', () => {
2426
});
2527

2628
afterEach(() => {
29+
vi.useRealTimers();
2730
vi.restoreAllMocks();
2831
});
2932

@@ -68,9 +71,75 @@ describe('swMediaAuth', () => {
6871
const mod = await import('./swMediaAuth');
6972

7073
await expect(mod.probeSWMediaAuthSupport()).resolves.toBe(false);
71-
expect(mod.getCachedSWMediaAuthSupport()).toBe(false);
74+
expect(mod.getCachedSWMediaAuthSupport()).toBeUndefined();
75+
}, 10_000);
76+
77+
it('retries a timed-out probe for the same controller', async () => {
78+
platform.hasServiceWorker.mockReturnValue(true);
79+
let probeCount = 0;
80+
const postMessage = vi.fn<(...args: unknown[]) => void>((...args: unknown[]) => {
81+
probeCount += 1;
82+
if (probeCount === 2) {
83+
const [port] = args[1] as MessagePort[];
84+
port?.postMessage({ type: 'swMediaAuth', supported: true, version: 1 });
85+
}
86+
});
87+
stubServiceWorker({ postMessage });
88+
const mod = await import('./swMediaAuth');
89+
90+
const firstProbe = mod.probeSWMediaAuthSupport();
91+
expect(postMessage).toHaveBeenCalledOnce();
92+
await expect(firstProbe).resolves.toBe(false);
93+
expect(mod.getCachedSWMediaAuthSupport()).toBeUndefined();
94+
95+
await expect(mod.probeSWMediaAuthSupport()).resolves.toBe(true);
96+
expect(postMessage).toHaveBeenCalledTimes(2);
7297
}, 10_000);
7398

99+
it('notifies unsupported while a replacement controller probe is unresolved', async () => {
100+
platform.hasServiceWorker.mockReturnValue(true);
101+
const firstController = {
102+
postMessage: vi.fn<(...args: unknown[]) => void>((...args: unknown[]) => {
103+
const [port] = args[1] as MessagePort[];
104+
port?.postMessage({ type: 'swMediaAuth', supported: true, version: 1 });
105+
}),
106+
};
107+
const serviceWorker = stubServiceWorker(firstController);
108+
const mod = await import('./swMediaAuth');
109+
const listener = vi.fn<(supported: boolean) => void>();
110+
mod.subscribeSWMediaAuthSupport(listener);
111+
112+
await expect(mod.probeSWMediaAuthSupport()).resolves.toBe(true);
113+
listener.mockClear();
114+
vi.useFakeTimers();
115+
116+
serviceWorker.controller = { postMessage: vi.fn<() => void>() };
117+
const controllerChange = serviceWorker.addEventListener.mock.calls.find(
118+
([type]) => type === 'controllerchange'
119+
)?.[1] as (() => void) | undefined;
120+
controllerChange?.();
121+
122+
expect(controllerChange).toBeTypeOf('function');
123+
expect(listener).toHaveBeenCalledWith(false);
124+
expect(mod.getCachedSWMediaAuthSupport()).toBeUndefined();
125+
126+
await vi.advanceTimersByTimeAsync(1500);
127+
expect(mod.getCachedSWMediaAuthSupport()).toBeUndefined();
128+
});
129+
130+
it('resolves false when posting the probe throws', async () => {
131+
platform.hasServiceWorker.mockReturnValue(true);
132+
stubServiceWorker({
133+
postMessage: vi.fn<() => void>(() => {
134+
throw new Error('postMessage failed');
135+
}),
136+
});
137+
const mod = await import('./swMediaAuth');
138+
139+
await expect(mod.probeSWMediaAuthSupport()).resolves.toBe(false);
140+
expect(mod.getCachedSWMediaAuthSupport()).toBe(false);
141+
});
142+
74143
it('resolves false when posting to a stale controller throws', async () => {
75144
platform.hasServiceWorker.mockReturnValue(true);
76145
const postMessage = vi.fn<() => void>(() => {

src/app/utils/swMediaAuth.ts

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ export function probeSWMediaAuthSupport(): Promise<boolean> {
5151

5252
probedController = controller;
5353
inflightProbe = new Promise<boolean>((resolve) => {
54-
const channel = new MessageChannel();
54+
let channel: MessageChannel | undefined;
55+
let timeoutId: ReturnType<typeof setTimeout> | undefined;
5556
let settled = false;
5657
const onMessage = (ev: MessageEvent) => {
5758
const data = ev.data as { type?: unknown; supported?: unknown; version?: unknown };
@@ -62,55 +63,58 @@ export function probeSWMediaAuthSupport(): Promise<boolean> {
6263
data.version >= SW_MEDIA_AUTH_PROTOCOL_VERSION
6364
);
6465
};
65-
const timeoutId = setTimeout(() => finish(false), PROBE_TIMEOUT_MS);
66-
67-
const finish = (supported: boolean) => {
66+
const finish = (supported: boolean, shouldCache = true) => {
6867
if (settled) return;
6968
settled = true;
70-
clearTimeout(timeoutId);
71-
channel.port1.removeEventListener('message', onMessage);
72-
channel.port1.close();
69+
if (timeoutId !== undefined) clearTimeout(timeoutId);
70+
channel?.port1.removeEventListener('message', onMessage);
71+
channel?.port1.close();
7372
// Controller may have changed mid-probe; activation resets the cache and
7473
// starts a fresh probe, so don't clobber that state with a stale answer.
7574
if (navigator.serviceWorker.controller === controller) {
76-
cachedSupport = supported;
77-
notify(supported);
75+
if (shouldCache) {
76+
cachedSupport = supported;
77+
notify(supported);
78+
}
7879
}
7980
resolve(supported);
8081
};
8182

82-
channel.port1.addEventListener('message', onMessage);
83-
channel.port1.start();
84-
8583
try {
84+
channel = new MessageChannel();
85+
timeoutId = setTimeout(() => finish(false, false), PROBE_TIMEOUT_MS);
86+
channel.port1.addEventListener('message', onMessage);
87+
channel.port1.start();
88+
8689
// oxlint-disable-next-line unicorn/require-post-message-target-origin
8790
controller.postMessage({ type: 'swMediaAuthProbe' }, [channel.port2]);
8891
} catch {
8992
// The controller can become redundant between reading it and posting the
9093
// probe. Treat that race like any other unsupported controller so callers
9194
// can use their authenticated blob fallback.
92-
channel.port2.close();
95+
channel?.port2.close();
9396
finish(false);
9497
}
95-
}).finally(() => {
96-
if (probedController === controller) {
97-
inflightProbe = undefined;
98-
}
99-
});
98+
})
99+
.catch(() => false)
100+
.finally(() => {
101+
if (probedController === controller) {
102+
inflightProbe = undefined;
103+
}
104+
});
100105

101106
return inflightProbe;
102107
}
103108

104109
if (typeof window !== 'undefined' && typeof navigator !== 'undefined' && hasServiceWorker()) {
105110
navigator.serviceWorker.addEventListener('controllerchange', () => {
106-
cachedSupport = undefined;
111+
const { controller } = navigator.serviceWorker;
112+
cachedSupport = controller ? undefined : false;
107113
inflightProbe = undefined;
108114
probedController = undefined;
109-
if (navigator.serviceWorker.controller) {
115+
notify(false);
116+
if (controller) {
110117
void probeSWMediaAuthSupport();
111-
} else {
112-
cachedSupport = false;
113-
notify(false);
114118
}
115119
});
116120
}

src/sw-media-auth-recovery.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ type SwTestHooks = typeof swTestHooksHelper;
1212
describe('service worker media auth recovery', () => {
1313
let swTestHooks: SwTestHooks;
1414
let clients: Map<string, Client>;
15+
let addEventListener: ReturnType<typeof vi.fn>;
1516

1617
beforeEach(async () => {
1718
vi.resetModules();
1819
clients = new Map();
20+
addEventListener = vi.fn();
1921
vi.stubGlobal('self', {
2022
__WB_MANIFEST: [],
21-
addEventListener: vi.fn(),
23+
addEventListener,
2224
caches: {
2325
open: vi.fn(async () => ({
2426
delete: vi.fn(async () => true),
@@ -37,6 +39,22 @@ describe('service worker media auth recovery', () => {
3739
swTestHooks = (await import('./sw')).swTestHooks;
3840
});
3941

42+
it('does not intercept media requests that already carry authorization', () => {
43+
const fetchHandler = addEventListener.mock.calls.find(([type]) => type === 'fetch')?.[1] as
44+
| ((event: FetchEvent) => void)
45+
| undefined;
46+
const respondWith = vi.fn();
47+
const request = new Request(
48+
'https://matrix.example.org/_matrix/client/v1/media/download/example.org/media-id',
49+
{ headers: { Authorization: 'Bearer direct-token' } }
50+
);
51+
52+
fetchHandler?.({ request, respondWith, clientId: 'client-a' } as unknown as FetchEvent);
53+
54+
expect(fetchHandler).toBeTypeOf('function');
55+
expect(respondWith).not.toHaveBeenCalled();
56+
});
57+
4058
it('shares recovery and preserves each Range header on retry', async () => {
4159
const client = {
4260
id: 'client-a',

src/sw.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,10 @@ self.addEventListener('fetch', (event: FetchEvent) => {
889889

890890
if (!mediaPath(url)) return;
891891

892+
// Direct-auth fallback requests already carry the page's token. Let the
893+
// browser send them unchanged instead of routing them back through SW auth.
894+
if (event.request.headers.has('Authorization')) return;
895+
892896
const { clientId } = event;
893897

894898
// For browser sub-resource loads (images, video, audio, etc.), 'follow' is

0 commit comments

Comments
 (0)