fix(share): load cover under COEP, keep share links out of the clipper, fix in-app import (#4636)

Three issues found while debugging shared-book links:

- /s cover was a broken <img>: the page runs under COEP: require-corp (for
  Turso SharedArrayBuffer), and the cover redirects to a cross-origin R2
  presigned URL that can't carry a Cross-Origin-Resource-Policy header, so the
  browser blocked it. R2 already has CORS, but that's a different header — a
  plain no-cors <img> needs CORP, which presigned URLs can't set. Serve /s with
  COEP: credentialless, which keeps the page cross-origin isolated (the Turso
  replica still boots there) while allowing the image. Scoped to /s; every other
  route keeps require-corp.

- Android share links (https://web.readest.com/s/{token}) were run through the
  article clipper: useClipUrlIngress excluded annotation links but not share
  links, so they fell through to clip_url/readability. Skip parseShareDeepLink
  URLs — useOpenShareLink owns that path.

- In-app book import failed with "Origin null is not allowed": the importer
  fetched /share/{token}/download with the renderer's fetch, and on the app
  (tauri.localhost -> web -> R2) the second cross-origin redirect nulls the
  request Origin, which R2's CORS rejects. Use the native HTTP client
  (tauriFetch) on the app — it follows the redirect and ignores CORS, needs no
  server change, and works against the deployed server. Web is unaffected: its
  fetch's redirect to R2 is the first cross-origin hop, so the Origin is
  preserved and R2 allows it.

Adds unit tests (middleware COEP per route, clipper skips share links, download
route 302, importer uses native HTTP on app and the renderer fetch on web).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-06-18 12:18:28 +08:00
committed by GitHub
parent 8bcb9f9b2a
commit 1ea607829c
7 changed files with 269 additions and 6 deletions
@@ -0,0 +1,48 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
const resolveActiveShareMock = vi.fn();
const getDownloadSignedUrlMock = vi.fn();
vi.mock('@/libs/shareServer', async (orig) => {
const actual = await orig<typeof import('@/libs/shareServer')>();
return { ...actual, resolveActiveShare: (...a: unknown[]) => resolveActiveShareMock(...a) };
});
vi.mock('@/utils/object', () => ({
getDownloadSignedUrl: (...a: unknown[]) => getDownloadSignedUrlMock(...a),
}));
import { GET } from '@/app/api/share/[token]/download/route';
const PRESIGNED = 'https://r2.example.com/u/Books/hash/book.epub?X-Amz-Signature=xyz';
const params = (token: string) => ({ params: Promise.resolve({ token }) });
beforeEach(() => {
resolveActiveShareMock.mockReset().mockResolvedValue({
ok: true,
share: { bookFileKey: 'u/Books/hash/book.epub' },
});
getDownloadSignedUrlMock.mockReset().mockResolvedValue(PRESIGNED);
});
describe('GET /api/share/[token]/download', () => {
// The importer relies on this 302: the client (native HTTP on app, the page's
// own fetch on web) follows it to R2 without tripping CORS.
it('302-redirects to the presigned URL', async () => {
const res = await GET(
new Request('https://web.readest.com/api/share/tok/download'),
params('tok'),
);
expect(res.status).toBe(302);
expect(res.headers.get('location')).toBe(PRESIGNED);
});
it('propagates rejection status without presigning', async () => {
resolveActiveShareMock.mockResolvedValue({ ok: false, reason: { kind: 'expired' } });
const res = await GET(
new Request('https://web.readest.com/api/share/tok/download'),
params('tok'),
);
expect(res.status).toBe(410);
expect(getDownloadSignedUrlMock).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,53 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { renderHook } from '@testing-library/react';
const invokeMock = vi.fn();
vi.mock('@tauri-apps/api/core', () => ({ invoke: (...a: unknown[]) => invokeMock(...a) }));
vi.mock('@/services/environment', async (orig) => {
const actual = await orig<typeof import('@/services/environment')>();
return { ...actual, isTauriAppPlatform: () => true };
});
vi.mock('@/context/EnvContext', () => ({ useEnv: () => ({ appService: {}, envConfig: {} }) }));
vi.mock('@/context/AuthContext', () => ({ useAuth: () => ({ user: null }) }));
vi.mock('@/hooks/useTranslation', () => ({ useTranslation: () => (k: string) => k }));
// Clip pipeline collaborators — should never be reached for share links; stub
// so an accidental article-clip can't blow up the test environment.
vi.mock('@/services/send/clipOptions', () => ({ getClipOptions: () => ({}) }));
vi.mock('@/services/send/conversion/conversionWorker', () => ({
convertToEpubWithWorker: vi.fn(),
}));
vi.mock('@/services/ingestService', () => ({ ingestFile: vi.fn() }));
import { useClipUrlIngress } from '@/hooks/useClipUrlIngress';
import { eventDispatcher } from '@/utils/event';
beforeEach(() => {
// Reject so clipAndImport short-circuits right after invoke('clip_url') —
// we only need to observe whether the clip was attempted.
invokeMock.mockReset().mockRejectedValue(new Error('stub'));
});
describe('useClipUrlIngress deep-link routing', () => {
it('does NOT run the article clipper on share deep links', async () => {
renderHook(() => useClipUrlIngress());
await eventDispatcher.dispatch('app-incoming-url', {
urls: ['https://web.readest.com/s/Qmup0X1A8ovl2FmKJKA8mB'],
});
await Promise.resolve();
// Share links belong to useOpenShareLink; the clipper must leave them alone.
expect(invokeMock).not.toHaveBeenCalled();
});
it('still clips ordinary article URLs', async () => {
renderHook(() => useClipUrlIngress());
await eventDispatcher.dispatch('app-incoming-url', {
urls: ['https://example.com/some-article'],
});
await Promise.resolve();
expect(invokeMock).toHaveBeenCalledWith(
'clip_url',
expect.objectContaining({ url: 'https://example.com/some-article' }),
);
});
});
@@ -0,0 +1,103 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { AppService } from '@/types/system';
import type { ShareMetadata } from '@/libs/share';
const getShareMock = vi.fn();
const tauriFetchMock = vi.fn();
let isTauri = false;
vi.mock('@tauri-apps/plugin-http', () => ({ fetch: (...a: unknown[]) => tauriFetchMock(...a) }));
vi.mock('@/services/environment', async (orig) => {
const actual = await orig<typeof import('@/services/environment')>();
return { ...actual, isTauriAppPlatform: () => isTauri };
});
let libraryState: {
libraryLoaded: boolean;
library: unknown[];
setLibrary: ReturnType<typeof vi.fn>;
getBookByHash: (h: string) => unknown;
};
vi.mock('@/store/libraryStore', () => ({
useLibraryStore: { getState: () => libraryState },
}));
vi.mock('@/libs/share', async (orig) => {
const actual = await orig<typeof import('@/libs/share')>();
return { ...actual, getShare: (...a: unknown[]) => getShareMock(...a) };
});
import { ensureSharedBookLocal } from '@/libs/shareImport';
const TOKEN = 'Qmup0X1A8ovl2FmKJKA8mB';
const makeAppService = (importedBook: unknown) => ({
loadLibraryBooks: vi.fn().mockResolvedValue([]),
importBook: vi.fn().mockResolvedValue(importedBook),
saveLibraryBooks: vi.fn().mockResolvedValue(undefined),
});
const newImportArgs = (appService: ReturnType<typeof makeAppService>) => ({
token: TOKEN,
importResult: { fileId: 'f', alreadyOwned: false, bookHash: 'hash', cfi: null },
appService: appService as unknown as AppService,
meta: { title: 'Book', format: 'EPUB' } as unknown as ShareMetadata,
});
beforeEach(() => {
isTauri = false;
getShareMock.mockReset();
tauriFetchMock.mockReset();
libraryState = {
libraryLoaded: false,
library: [],
setLibrary: vi.fn(),
getBookByHash: () => undefined,
};
});
describe('ensureSharedBookLocal (new import)', () => {
it('on web, fetches the /download endpoint and follows the 302 with the renderer fetch', async () => {
const blob = new Blob([new Uint8Array([1, 2, 3])], { type: 'application/epub+zip' });
const fetchMock = vi.fn().mockResolvedValue({ ok: true, blob: async () => blob });
vi.stubGlobal('fetch', fetchMock);
const importedBook = { hash: 'hash', title: 'Book' };
const appService = makeAppService(importedBook);
const result = await ensureSharedBookLocal(newImportArgs(appService));
// Web hits /download directly; the redirect to R2 is its first cross-origin
// hop, so the Origin is preserved and R2's CORS allows it.
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(fetchMock.mock.calls[0]![0]).toContain(`/share/${TOKEN}/download`);
expect(tauriFetchMock).not.toHaveBeenCalled();
const fileArg = appService.importBook.mock.calls[0]![0] as File;
expect(fileArg).toBeInstanceOf(File);
expect(fileArg.name).toBe('Book.epub');
expect(result).toBe(importedBook);
vi.unstubAllGlobals();
});
it('on the app, downloads via native HTTP (which ignores CORS) on the same /download endpoint', async () => {
isTauri = true;
const blob = new Blob([new Uint8Array([4, 5, 6])], { type: 'application/epub+zip' });
tauriFetchMock.mockResolvedValue({ ok: true, blob: async () => blob });
const fetchMock = vi.fn();
vi.stubGlobal('fetch', fetchMock);
const appService = makeAppService({ hash: 'hash', title: 'Book' });
await ensureSharedBookLocal(newImportArgs(appService));
// The app must use native HTTP, not the renderer's fetch: tauri.localhost→
// web→R2 is a second cross-origin hop that nulls the Origin and trips CORS.
expect(tauriFetchMock).toHaveBeenCalledTimes(1);
expect(tauriFetchMock.mock.calls[0]![0]).toContain(`/share/${TOKEN}/download`);
expect(fetchMock).not.toHaveBeenCalled();
vi.unstubAllGlobals();
});
});
@@ -0,0 +1,35 @@
import { describe, it, expect } from 'vitest';
import { NextRequest } from 'next/server';
import { middleware } from '@/middleware';
const coep = (path: string) =>
middleware(new NextRequest(`http://localhost:3000${path}`)).headers.get(
'Cross-Origin-Embedder-Policy',
);
describe('middleware cross-origin isolation headers', () => {
it('serves COEP credentialless on the /s share landing so the R2 cover <img> loads', () => {
// The cover redirects to a cross-origin R2 URL that can't carry a CORP
// header; credentialless keeps the page isolated while allowing it.
expect(coep('/s')).toBe('credentialless');
expect(coep('/s/Qmup0X1A8ovl2FmKJKA8mB')).toBe('credentialless');
});
it('keeps the stricter require-corp on every other document route', () => {
expect(coep('/')).toBe('require-corp');
expect(coep('/library')).toBe('require-corp');
// Must not be caught by a naive startsWith('/s').
expect(coep('/settings')).toBe('require-corp');
expect(coep('/search')).toBe('require-corp');
});
it('always pairs COOP same-origin on document responses', () => {
const res = middleware(new NextRequest('http://localhost:3000/s/tok'));
expect(res.headers.get('Cross-Origin-Opener-Policy')).toBe('same-origin');
});
it('does not put COEP on /api routes', () => {
const res = middleware(new NextRequest('http://localhost:3000/api/share/tok'));
expect(res.headers.get('Cross-Origin-Embedder-Policy')).toBeNull();
});
});
@@ -10,6 +10,7 @@ import { convertToEpubWithWorker } from '@/services/send/conversion/conversionWo
import { getClipOptions } from '@/services/send/clipOptions';
import { eventDispatcher } from '@/utils/event';
import { parseAnnotationDeepLink } from '@/utils/deeplink';
import { parseShareDeepLink } from '@/utils/share';
import { useTranslation } from './useTranslation';
interface ClipOptions {
@@ -149,6 +150,11 @@ export function useClipUrlIngress() {
// Annotation deep links can come over https (web.readest.com).
// Skip them — useOpenAnnotationLink owns that path.
if (parseAnnotationDeepLink(url)) return;
// Share links (https://web.readest.com/s/{token}) also arrive over https.
// Skip them — useOpenShareLink owns that path. Without this guard the
// share landing URL is run through the article clipper instead of
// importing the shared book.
if (parseShareDeepLink(url)) return;
void clipAndImport(url);
};
+13 -5
View File
@@ -1,7 +1,8 @@
import { fetch as tauriFetch } from '@tauri-apps/plugin-http';
import type { Book } from '@/types/book';
import type { AppService } from '@/types/system';
import { useLibraryStore } from '@/store/libraryStore';
import { getAPIBaseUrl } from '@/services/environment';
import { getAPIBaseUrl, isTauriAppPlatform } from '@/services/environment';
import { ShareApiError, getShare, type ImportShareResponse, type ShareMetadata } from './share';
interface EnsureSharedBookLocalArgs {
@@ -91,12 +92,19 @@ export const ensureSharedBookLocal = async ({
return existing;
}
// Book is not in the local library yet. Fetch the bytes via the public share
// download endpoint (302 → presigned URL; fetch follows redirects), then
// hand them to importBook which knows how to create the proper local Book.
// Book is not in the local library yet. Pull the bytes so importBook can build
// the proper local Book. Both platforms hit the `/download` 302 and let the
// client follow it to R2 — without tripping CORS:
// - App: Tauri's native HTTP follows the redirect and ignores CORS.
// - Web: the page's own fetch follows it; the redirect to R2 is the FIRST
// cross-origin hop, so the request keeps the page's Origin (which R2's CORS
// allows). This is why the app needed native HTTP but web doesn't —
// tauri.localhost→web→R2 is a second hop that nulls the Origin.
const shareMeta = meta ?? (await getShare(token));
const downloadUrl = `${getAPIBaseUrl()}/share/${encodeURIComponent(token)}/download`;
const response = await fetch(downloadUrl);
const response = isTauriAppPlatform()
? await tauriFetch(downloadUrl)
: await globalThis.fetch(downloadUrl);
if (!response.ok) {
throw new ShareApiError(
response.status,
+11 -1
View File
@@ -53,7 +53,17 @@ export function middleware(request: NextRequest) {
// of the top-level browsing context, determined by the document's headers.
const response = NextResponse.next();
response.headers.set('Cross-Origin-Opener-Policy', 'same-origin');
response.headers.set('Cross-Origin-Embedder-Policy', 'require-corp');
// The /s share landing embeds the book cover via an <img> that redirects to a
// cross-origin R2 presigned URL. Under COEP: require-corp the browser blocks
// that image, because R2 can't attach a Cross-Origin-Resource-Policy header to
// a presigned GET. `credentialless` keeps the page cross-origin isolated — so
// EnvContext can still boot the Turso replica (SharedArrayBuffer) here when the
// user has sync enabled — while dropping the CORP requirement for no-cors
// subresources, letting the cover load with no client-side change. Every other
// route keeps the stricter require-corp.
const path = request.nextUrl.pathname;
const coep = path === '/s' || path.startsWith('/s/') ? 'credentialless' : 'require-corp';
response.headers.set('Cross-Origin-Embedder-Policy', coep);
return response;
}