diff --git a/.changeset/cjk-slide-ids.md b/.changeset/cjk-slide-ids.md new file mode 100644 index 000000000..0e67980fe --- /dev/null +++ b/.changeset/cjk-slide-ids.md @@ -0,0 +1,5 @@ +--- +'@open-slide/core': patch +--- + +Accept Unicode (including CJK) slide folder ids for discovery and mutations. diff --git a/packages/core/src/app/lib/assets.ts b/packages/core/src/app/lib/assets.ts index 17c9dacda..cc37f32bc 100644 --- a/packages/core/src/app/lib/assets.ts +++ b/packages/core/src/app/lib/assets.ts @@ -12,9 +12,15 @@ export type AssetEntry = { export type UploadOptions = { overwrite?: boolean }; +function assetsScopeUrl(slideId: string, ...parts: string[]): string { + const segments = [encodeURIComponent(slideId), ...parts.map((p) => encodeURIComponent(p))]; + return `/__assets/${segments.join('/')}`; +} + export async function listAssets(slideId: string): Promise { - const res = await fetch(`/__assets/${slideId}`); - if (!res.ok) throw new Error(`GET /__assets/${slideId} ${res.status}`); + const url = assetsScopeUrl(slideId); + const res = await fetch(url); + if (!res.ok) throw new Error(`GET ${url} ${res.status}`); const data = (await res.json()) as { assets?: AssetEntry[] }; return data.assets ?? []; } @@ -25,7 +31,7 @@ export async function uploadAsset( opts: UploadOptions = {}, ): Promise { const qs = opts.overwrite ? '?overwrite=1' : ''; - return fetch(`/__assets/${slideId}/${encodeURIComponent(file.name)}${qs}`, { + return fetch(`${assetsScopeUrl(slideId, file.name)}${qs}`, { method: 'POST', headers: { 'content-type': file.type || 'application/octet-stream', @@ -36,7 +42,7 @@ export async function uploadAsset( } async function renameAsset(slideId: string, from: string, to: string): Promise { - return fetch(`/__assets/${slideId}/${encodeURIComponent(from)}`, { + return fetch(assetsScopeUrl(slideId, from), { method: 'PATCH', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ name: to }), @@ -44,13 +50,13 @@ async function renameAsset(slideId: string, from: string, to: string): Promise { - return fetch(`/__assets/${slideId}/${encodeURIComponent(name)}`, { method: 'DELETE' }); + return fetch(assetsScopeUrl(slideId, name), { method: 'DELETE' }); } export type AssetUsage = { slideId: string; count: number }; export async function listAssetUsages(slideId: string, name: string): Promise { - const res = await fetch(`/__assets/${slideId}/${encodeURIComponent(name)}/usages`); + const res = await fetch(`${assetsScopeUrl(slideId, name)}/usages`); if (!res.ok) return []; const data = (await res.json().catch(() => null)) as { usages?: AssetUsage[] } | null; return data?.usages ?? []; @@ -93,7 +99,7 @@ export async function uploadWithAutoRename( createdAt: body?.createdAt ?? now, mtime: body?.mtime ?? now, mime: body?.mime ?? uploaded.type ?? 'application/octet-stream', - url: body?.url ?? `/__assets/${slideId}/${encodeURIComponent(uploaded.name)}`, + url: body?.url ?? assetsScopeUrl(slideId, uploaded.name), unused: body?.unused ?? false, }; return { ok: true, status: res.status, entry }; diff --git a/packages/core/src/app/lib/folders.ts b/packages/core/src/app/lib/folders.ts index dc636220f..229cc1a6e 100644 --- a/packages/core/src/app/lib/folders.ts +++ b/packages/core/src/app/lib/folders.ts @@ -25,30 +25,33 @@ async function getManifest(): Promise { } async function patchSlideName(slideId: string, name: string): Promise { - const res = await fetch(`/__slides/${slideId}`, { + const id = encodeURIComponent(slideId); + const res = await fetch(`/__slides/${id}`, { method: 'PATCH', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ name }), }); - if (!res.ok) throw new Error(`PATCH /__slides/${slideId} ${res.status}`); + if (!res.ok) throw new Error(`PATCH /__slides/${id} ${res.status}`); } async function duplicateSlideReq(slideId: string, newId?: string): Promise { + const id = encodeURIComponent(slideId); const init: RequestInit = { method: 'POST' }; if (newId !== undefined) { init.headers = { 'content-type': 'application/json' }; init.body = JSON.stringify({ newId }); } - const res = await fetch(`/__slides/${slideId}/duplicate`, init); - if (!res.ok) throw new Error(`POST /__slides/${slideId}/duplicate ${res.status}`); + const res = await fetch(`/__slides/${id}/duplicate`, init); + if (!res.ok) throw new Error(`POST /__slides/${id}/duplicate ${res.status}`); const body = (await res.json()) as { slideId?: unknown }; if (typeof body.slideId !== 'string') throw new Error('duplicate response missing slideId'); return body.slideId; } async function deleteSlideReq(slideId: string): Promise { - const res = await fetch(`/__slides/${slideId}`, { method: 'DELETE' }); - if (!res.ok) throw new Error(`DELETE /__slides/${slideId} ${res.status}`); + const id = encodeURIComponent(slideId); + const res = await fetch(`/__slides/${id}`, { method: 'DELETE' }); + if (!res.ok) throw new Error(`DELETE /__slides/${id} ${res.status}`); } async function postFolder(name: string, icon: FolderIcon): Promise { diff --git a/packages/core/src/editing/slide-ops.test.ts b/packages/core/src/editing/slide-ops.test.ts index a3b5c552c..7f123dfb3 100644 --- a/packages/core/src/editing/slide-ops.test.ts +++ b/packages/core/src/editing/slide-ops.test.ts @@ -10,6 +10,9 @@ import { removePageFromDefaultExportInSource, reorderDefaultExportPagesInSource, reorderNotesArrayInSource, + resolveSlideEntry, + rmSlideDir, + SLIDE_ID_RE, updateMetaTitleInSource, validateSlideName, } from './slide-ops.ts'; @@ -33,6 +36,35 @@ async function writeSlide(root: string, id: string, title = id): Promise { await fs.writeFile(path.join(root, id, 'assets', 'hero.txt'), 'hero', 'utf8'); } +describe('SLIDE_ID_RE', () => { + it('accepts ASCII kebab ids and CJK letter ids', () => { + expect(SLIDE_ID_RE.test('cover')).toBe(true); + expect(SLIDE_ID_RE.test('intro_2')).toBe(true); + expect(SLIDE_ID_RE.test('推薦系統')).toBe(true); + expect(SLIDE_ID_RE.test('簡報-v2')).toBe(true); + }); + + it('rejects spaces, dots, and path separators', () => { + expect(SLIDE_ID_RE.test('bad id')).toBe(false); + expect(SLIDE_ID_RE.test('has.dot')).toBe(false); + expect(SLIDE_ID_RE.test('../escape')).toBe(false); + expect(SLIDE_ID_RE.test('a/b')).toBe(false); + }); +}); + +describe('resolveSlideEntry / rmSlideDir with CJK ids', () => { + it('resolves and removes a CJK slide directory', async () => { + await withSlidesRoot(async (root) => { + await writeSlide(root, '推薦系統'); + expect(resolveSlideEntry(root, '推薦系統')).toBe(path.join(root, '推薦系統', 'index.tsx')); + expect(await rmSlideDir(root, '推薦系統')).toBe(true); + await expect(fs.access(path.join(root, '推薦系統'))).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + }); +}); + describe('duplicateSlideDir', () => { it('duplicates a slide directory with an automatic copy id', async () => { await withSlidesRoot(async (root) => { @@ -68,6 +100,25 @@ describe('duplicateSlideDir', () => { }); }); + it('duplicates a CJK slide id and accepts a CJK desired id', async () => { + await withSlidesRoot(async (root) => { + await writeSlide(root, '推薦系統', '推薦系統'); + + expect(await duplicateSlideDir(root, '推薦系統')).toEqual({ + ok: true, + slideId: '推薦系統-copy', + }); + expect(await duplicateSlideDir(root, '推薦系統', '複本簡報')).toEqual({ + ok: true, + slideId: '複本簡報', + }); + await expect( + fs.access(path.join(root, '推薦系統-copy', 'index.tsx')), + ).resolves.toBeUndefined(); + await expect(fs.access(path.join(root, '複本簡報', 'index.tsx'))).resolves.toBeUndefined(); + }); + }); + it('rejects an existing desired id', async () => { await withSlidesRoot(async (root) => { await writeSlide(root, 'cover'); diff --git a/packages/core/src/editing/slide-ops.ts b/packages/core/src/editing/slide-ops.ts index b040d0161..84d2f1015 100644 --- a/packages/core/src/editing/slide-ops.ts +++ b/packages/core/src/editing/slide-ops.ts @@ -2,7 +2,11 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { parse as babelParse } from '@babel/parser'; -export const SLIDE_ID_RE = /^[a-z0-9_-]+$/i; +// Letters (any script, including CJK) + digits + hyphen/underscore. Rejects +// spaces, dots, path separators, and other punctuation so ids stay safe for +// filesystem joins and `/__slides/:id` URL segments. Path traversal is still +// blocked by the resolve+prefix checks below. +export const SLIDE_ID_RE = /^[\p{L}\p{N}_-]+$/u; type MetaTitleRead = | { kind: 'found'; title: string } diff --git a/packages/core/src/vite/open-slide-plugin.test.ts b/packages/core/src/vite/open-slide-plugin.test.ts index 43311d479..17a5a6fff 100644 --- a/packages/core/src/vite/open-slide-plugin.test.ts +++ b/packages/core/src/vite/open-slide-plugin.test.ts @@ -36,15 +36,27 @@ describe('generateSlidesModule', () => { }); }); - it('excludes folders whose id is not ASCII-safe and reports them as ignored', async () => { + it('keeps CJK slide ids and lists them alongside ASCII ids', async () => { await withSlidesRoot(async (root) => { const files = [await writeSlide(root, 'cover'), await writeSlide(root, '推薦系統')].sort(); const { code, ignored } = await generateSlidesModule(files, root, false); - expect(ignored).toEqual(['推薦系統']); + expect(ignored).toEqual([]); + expect(code).toContain('export const slideIds = ["cover","推薦系統"];'); + expect(code).toContain('推薦系統'); + }); + }); + + it('excludes folders whose id has spaces or punctuation and reports them as ignored', async () => { + await withSlidesRoot(async (root) => { + const files = [await writeSlide(root, 'cover'), await writeSlide(root, 'bad id')].sort(); + + const { code, ignored } = await generateSlidesModule(files, root, false); + + expect(ignored).toEqual(['bad id']); expect(code).toContain('export const slideIds = ["cover"];'); - expect(code).not.toContain('推薦系統'); + expect(code).not.toContain('bad id'); }); }); }); diff --git a/packages/core/src/vite/open-slide-plugin.ts b/packages/core/src/vite/open-slide-plugin.ts index 661e04dc7..265cdeb8c 100644 --- a/packages/core/src/vite/open-slide-plugin.ts +++ b/packages/core/src/vite/open-slide-plugin.ts @@ -135,9 +135,10 @@ export async function generateSlidesModule( ); // Discovery globs every `slides/*/index.*`, but a slide id is used in URLs, - // filesystem paths, and the editing routes — all guarded by SLIDE_ID_RE. Drop - // folders with an unusable id instead of listing them as slides that then fail - // every folder/edit action; `load` warns about each ignored folder. + // filesystem paths, and the editing routes — all guarded by SLIDE_ID_RE + // (Unicode letters/digits plus "-"/"_"). Drop folders with an unusable id + // instead of listing them as slides that then fail every folder/edit action; + // `load` warns about each ignored folder. const entries = scanned.filter((e) => SLIDE_ID_RE.test(e.id)); const ignored = scanned.filter((e) => !SLIDE_ID_RE.test(e.id)).map((e) => e.id); @@ -246,7 +247,7 @@ export function openSlidePlugin(opts: OpenSlidePluginOptions): Plugin { if (warnedInvalidSlideIds.has(slideId)) continue; warnedInvalidSlideIds.add(slideId); this.warn( - `Ignoring slide folder "${slideId}": slide ids must match ${SLIDE_ID_RE} (lowercase/uppercase letters, digits, "-", "_"). Rename the folder under "${slidesDir}/" to a kebab-case id so it appears in the browser and can be moved into folders.`, + `Ignoring slide folder "${slideId}": slide ids must match ${SLIDE_ID_RE} (Unicode letters/digits, "-", "_"; no spaces or punctuation). Rename the folder under "${slidesDir}/" so it appears in the browser and can be moved into folders.`, ); } return code; diff --git a/packages/core/src/vite/routes/assets.ts b/packages/core/src/vite/routes/assets.ts index 86df04171..c65d9bc5a 100644 --- a/packages/core/src/vite/routes/assets.ts +++ b/packages/core/src/vite/routes/assets.ts @@ -13,7 +13,7 @@ import { validateAssetName, } from '../../files/assets.ts'; import { validateMutationRequest } from '../../http/request-guard.ts'; -import { type ApiContext, json, readBody } from './context.ts'; +import { type ApiContext, decodePathSegment, json, readBody } from './context.ts'; // GET /__assets/:scope list assets in slide or @global // GET /__assets/:scope/:file serve raw asset bytes @@ -33,9 +33,10 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi const usagesMatch = url.pathname.match(/^\/([^/]+)\/([^/]+)\/usages$/); if (usagesMatch && method === 'GET') { - const scope = usagesMatch[1]; - const filename = decodeURIComponent(usagesMatch[2]); - if (!validateAssetName(filename)) return json(res, 400, { error: 'invalid path' }); + const scope = decodePathSegment(usagesMatch[1]); + const filename = decodePathSegment(usagesMatch[2]); + if (!scope || filename === null || !validateAssetName(filename)) + return json(res, 400, { error: 'invalid path' }); const isGlobal = scope === GLOBAL_SCOPE; const assetPath = isGlobal ? `@assets/${filename}` : `./assets/${filename}`; @@ -76,7 +77,8 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi } if (listMatch && method === 'GET') { - const slideId = listMatch[1]; + const slideId = decodePathSegment(listMatch[1]); + if (!slideId) return json(res, 400, { error: 'invalid slideId' }); const scopedDir = resolveScopedAssetsDir(ctx.slidesRoot, ctx.globalAssetsRoot, slideId); if (!scopedDir) return json(res, 400, { error: 'invalid slideId' }); @@ -109,7 +111,7 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi createdAt: assetCreatedAt(stat.birthtimeMs, stat.mtimeMs), mtime: stat.mtimeMs, mime: mimeForFilename(name), - url: `/__assets/${slideId}/${encodeURIComponent(name)}`, + url: `/__assets/${encodeURIComponent(slideId)}/${encodeURIComponent(name)}`, unused: true, }); } @@ -152,8 +154,9 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi } if (fileMatch) { - const slideId = fileMatch[1]; - const filename = decodeURIComponent(fileMatch[2]); + const slideId = decodePathSegment(fileMatch[1]); + const filename = decodePathSegment(fileMatch[2]); + if (!slideId || filename === null) return json(res, 400, { error: 'invalid path' }); const file = resolveScopedAssetFile( ctx.slidesRoot, ctx.globalAssetsRoot, @@ -230,7 +233,7 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi createdAt: assetCreatedAt(stat.birthtimeMs, stat.mtimeMs), mtime: stat.mtimeMs, mime: mimeForFilename(filename), - url: `/__assets/${slideId}/${encodeURIComponent(filename)}`, + url: `/__assets/${encodeURIComponent(slideId)}/${encodeURIComponent(filename)}`, }); } diff --git a/packages/core/src/vite/routes/context.test.ts b/packages/core/src/vite/routes/context.test.ts new file mode 100644 index 000000000..71a0aee69 --- /dev/null +++ b/packages/core/src/vite/routes/context.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { decodePathSegment } from './context.ts'; + +describe('decodePathSegment', () => { + it('decodes a percent-encoded non-ASCII segment', () => { + expect(decodePathSegment('%E5%B0%81%E9%9D%A2')).toBe('封面'); + }); + + it('returns a plain segment unchanged', () => { + expect(decodePathSegment('cover')).toBe('cover'); + }); + + it('decodes a percent-encoded filename with spaces and separators', () => { + expect(decodePathSegment('my%20photo%20(1).png')).toBe('my photo (1).png'); + }); + + it('returns null for a truncated escape rather than throwing', () => { + expect(decodePathSegment('%E0%A4%A')).toBeNull(); + }); + + it('returns null for a lone percent sign', () => { + expect(decodePathSegment('%')).toBeNull(); + }); + + it('preserves the empty segment', () => { + expect(decodePathSegment('')).toBe(''); + }); +}); diff --git a/packages/core/src/vite/routes/context.ts b/packages/core/src/vite/routes/context.ts index d355b4b5a..a284c9c99 100644 --- a/packages/core/src/vite/routes/context.ts +++ b/packages/core/src/vite/routes/context.ts @@ -74,3 +74,26 @@ export function resolveSlidePath( export function resolveSlideEntryPath(ctx: ApiContext, slideId: string): string | null { return resolveSlidePath(ctx.userCwd, ctx.slidesDir, slideId); } + +/** + * Decode one URL path segment, reporting malformed percent-encoding as null + * instead of throwing. + * + * `decodeURIComponent` raises `URIError` on a truncated or invalid escape such + * as `%E0%A4%A` or a lone `%`. Inside a dev-server request handler that becomes + * an unhandled exception and a 500, which reads as a broken server rather than + * what it is: a request the client got wrong. Returning null lets each route + * answer 400 on its own terms, alongside the invalid-path rejections it already + * performs. + * + * Supporting non-ASCII slide ids is what makes this reachable in ordinary use. + * Every segment of an asset URL is percent-encoded on the way out now, so any + * segment can come back damaged. + */ +export function decodePathSegment(raw: string): string | null { + try { + return decodeURIComponent(raw); + } catch { + return null; + } +} diff --git a/packages/core/src/vite/routes/slides.ts b/packages/core/src/vite/routes/slides.ts index 0e9ef2c33..ca8033492 100644 --- a/packages/core/src/vite/routes/slides.ts +++ b/packages/core/src/vite/routes/slides.ts @@ -16,7 +16,7 @@ import { } from '../../editing/slide-ops.ts'; import { readManifest, writeManifest } from '../../files/folders.ts'; import { validateMutationRequest } from '../../http/request-guard.ts'; -import { type ApiContext, json, readBody } from './context.ts'; +import { type ApiContext, decodePathSegment, json, readBody } from './context.ts'; // PUT /__slides/:id/reorder reorder pages { order: number[] } // DELETE /__slides/:id/pages/:i remove page @@ -40,8 +40,9 @@ export function registerSlideRoutes(server: ViteDevServer, ctx: ApiContext): voi if (!requestCheck.ok) { return json(res, requestCheck.status, { error: requestCheck.error }); } - const slideId = reorderMatch[1]; - if (!SLIDE_ID_RE.test(slideId)) return json(res, 400, { error: 'invalid slideId' }); + const slideId = decodePathSegment(reorderMatch[1]); + if (!slideId || !SLIDE_ID_RE.test(slideId)) + return json(res, 400, { error: 'invalid slideId' }); const body = (await readBody(req)) as { order?: unknown }; if (!Array.isArray(body.order)) return json(res, 400, { error: 'invalid order' }); @@ -81,10 +82,11 @@ export function registerSlideRoutes(server: ViteDevServer, ctx: ApiContext): voi const pageOpMatch = url.pathname.match(/^\/([^/]+)\/pages\/(\d+)(?:\/([a-z]+))?$/); if (pageOpMatch) { - const slideId = pageOpMatch[1]; + const slideId = decodePathSegment(pageOpMatch[1]); const pageIndex = Number.parseInt(pageOpMatch[2], 10); const op = pageOpMatch[3]; - if (!SLIDE_ID_RE.test(slideId)) return json(res, 400, { error: 'invalid slideId' }); + if (!slideId || !SLIDE_ID_RE.test(slideId)) + return json(res, 400, { error: 'invalid slideId' }); if (!Number.isInteger(pageIndex) || pageIndex < 0) return json(res, 400, { error: 'invalid page index' }); @@ -138,8 +140,9 @@ export function registerSlideRoutes(server: ViteDevServer, ctx: ApiContext): voi if (!requestCheck.ok) { return json(res, requestCheck.status, { error: requestCheck.error }); } - const slideId = duplicateMatch[1]; - if (!SLIDE_ID_RE.test(slideId)) return json(res, 400, { error: 'invalid slideId' }); + const slideId = decodePathSegment(duplicateMatch[1]); + if (!slideId || !SLIDE_ID_RE.test(slideId)) + return json(res, 400, { error: 'invalid slideId' }); const body = (await readBody(req)) as DuplicateSlideBody; if (body.newId !== undefined && typeof body.newId !== 'string') { @@ -160,8 +163,9 @@ export function registerSlideRoutes(server: ViteDevServer, ctx: ApiContext): voi const idMatch = url.pathname.match(/^\/([^/]+)$/); if (!idMatch) return next(); - const slideId = idMatch[1]; - if (!SLIDE_ID_RE.test(slideId)) return json(res, 400, { error: 'invalid slideId' }); + const slideId = decodePathSegment(idMatch[1]); + if (!slideId || !SLIDE_ID_RE.test(slideId)) + return json(res, 400, { error: 'invalid slideId' }); if (method === 'PATCH') { const requestCheck = validateMutationRequest(req, { requireJsonBody: true });