Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cjk-slide-ids.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@open-slide/core': patch
---

Accept Unicode (including CJK) slide folder ids for discovery and mutations.
20 changes: 13 additions & 7 deletions packages/core/src/app/lib/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AssetEntry[]> {
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 ?? [];
}
Expand All @@ -25,7 +31,7 @@ export async function uploadAsset(
opts: UploadOptions = {},
): Promise<Response> {
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',
Expand All @@ -36,21 +42,21 @@ export async function uploadAsset(
}

async function renameAsset(slideId: string, from: string, to: string): Promise<Response> {
return fetch(`/__assets/${slideId}/${encodeURIComponent(from)}`, {
return fetch(assetsScopeUrl(slideId, from), {
method: 'PATCH',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ name: to }),
});
}

async function deleteAsset(slideId: string, name: string): Promise<Response> {
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<AssetUsage[]> {
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 ?? [];
Expand Down Expand Up @@ -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 };
Expand Down
15 changes: 9 additions & 6 deletions packages/core/src/app/lib/folders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,33 @@ async function getManifest(): Promise<FoldersManifest> {
}

async function patchSlideName(slideId: string, name: string): Promise<void> {
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<string> {
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<void> {
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<Folder> {
Expand Down
51 changes: 51 additions & 0 deletions packages/core/src/editing/slide-ops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import {
removePageFromDefaultExportInSource,
reorderDefaultExportPagesInSource,
reorderNotesArrayInSource,
resolveSlideEntry,
rmSlideDir,
SLIDE_ID_RE,
updateMetaTitleInSource,
validateSlideName,
} from './slide-ops.ts';
Expand All @@ -33,6 +36,35 @@ async function writeSlide(root: string, id: string, title = id): Promise<void> {
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) => {
Expand Down Expand Up @@ -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');
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/editing/slide-ops.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
18 changes: 15 additions & 3 deletions packages/core/src/vite/open-slide-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
9 changes: 5 additions & 4 deletions packages/core/src/vite/open-slide-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
17 changes: 10 additions & 7 deletions packages/core/src/vite/routes/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 scope = decodePathSegment(usagesMatch[1]);
const filename = decodeURIComponent(usagesMatch[2]);
if (!validateAssetName(filename)) return json(res, 400, { error: 'invalid path' });
if (!scope || !validateAssetName(filename))
return json(res, 400, { error: 'invalid path' });
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const isGlobal = scope === GLOBAL_SCOPE;
const assetPath = isGlobal ? `@assets/${filename}` : `./assets/${filename}`;
Expand Down Expand Up @@ -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' });

Expand Down Expand Up @@ -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,
});
}
Expand Down Expand Up @@ -152,8 +154,9 @@ export function registerAssetRoutes(server: ViteDevServer, ctx: ApiContext): voi
}

if (fileMatch) {
const slideId = fileMatch[1];
const slideId = decodePathSegment(fileMatch[1]);
const filename = decodeURIComponent(fileMatch[2]);
if (!slideId) return json(res, 400, { error: 'invalid path' });
const file = resolveScopedAssetFile(
ctx.slidesRoot,
ctx.globalAssetsRoot,
Expand Down Expand Up @@ -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)}`,
});
}

Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/vite/routes/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,12 @@ export function resolveSlidePath(
export function resolveSlideEntryPath(ctx: ApiContext, slideId: string): string | null {
return resolveSlidePath(ctx.userCwd, ctx.slidesDir, slideId);
}

/** Decode a URL path segment; returns null if the percent-encoding is malformed. */
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
export function decodePathSegment(raw: string): string | null {
try {
return decodeURIComponent(raw);
} catch {
return null;
}
}
22 changes: 13 additions & 9 deletions packages/core/src/vite/routes/slides.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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' });
Expand Down Expand Up @@ -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' });

Expand Down Expand Up @@ -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') {
Expand All @@ -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 });
Expand Down