From 6a859fc8753e1ff82c1c9f6e54f3a79ca21bfa50 Mon Sep 17 00:00:00 2001 From: kamijo-haruto Date: Wed, 29 Jul 2026 20:29:59 +0900 Subject: [PATCH 1/5] =?UTF-8?q?[chore]=20=E4=B8=80=E6=99=82=E7=9A=84?= =?UTF-8?q?=E3=81=AAgit=20worktree=E3=82=92biome=E3=81=AE=E6=A4=9C?= =?UTF-8?q?=E6=9F=BB=E5=AF=BE=E8=B1=A1=E3=81=8B=E3=82=89=E5=A4=96=E3=81=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ツールが作る作業ツリーはリポジトリのもう1つの完全なチェックアウトを含むため、 pnpm lint がそのコピーまで検査していた。実際のツリーには存在しないCRLFの指摘が 大量に出て、自分のコードに問題が無いときでも lint が壊れているように見える。 除外は .git/info/exclude に書かれているが、biome の useIgnoreFile はルートの .gitignore しか読まないのでどちらも届かない。loadtest/.out と同じ理由。 Co-Authored-By: Claude Opus 5 (1M context) --- biome.jsonc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/biome.jsonc b/biome.jsonc index 6815355..6fed21d 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -17,7 +17,13 @@ "**/worker-configuration.d.ts", // Generated load-test fixtures. `useIgnoreFile` above only reads the root // .gitignore, so the nested one under loadtest/ does not reach biome. - "**/loadtest/.out" + "**/loadtest/.out", + // Throwaway git worktrees created by tooling. They hold a second full + // checkout, so without this `pnpm lint` reports another copy of the repo — + // often with CRLF diagnostics that do not exist in the real tree, which + // reads as "lint is broken" when nothing is wrong. Excluded via + // .git/info/exclude rather than .gitignore, and biome reads neither. + "**/.claude/worktrees" ] }, "linter": { From 1481df988a1c53d717d5df84c5f3bea224dd532d Mon Sep 17 00:00:00 2001 From: kamijo-haruto Date: Wed, 29 Jul 2026 20:28:25 +0900 Subject: [PATCH 2/5] =?UTF-8?q?[feat]=20QR=E3=82=B3=E3=83=BC=E3=83=89?= =?UTF-8?q?=E3=81=AB=E7=9F=AD=E7=B8=AE=E3=82=B3=E3=83=BC=E3=83=89=E3=82=92?= =?UTF-8?q?=E8=BF=BD=E5=8A=A0=E3=81=97=E3=80=81=E5=8D=B0=E5=88=B7=E3=81=99?= =?UTF-8?q?=E3=82=8BURL=E3=82=92=E7=9F=AD=E3=81=8F=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QRに焼き込むURLが 103文字 / 57×57モジュールあり、印刷時に読み取りづらかった。 7桁の短縮コードを併設して 74文字 / 49×49 にする(実測)。 主キーを差し替えるのではなく short_code 列を足した。QRCodes.id は AccessLogs.qr_id の外部キー参照先で、スキーマにも「印刷されたURLに焼き込まれる ので永久に安定していなければならない」と明記してある。既存のUUID形式のURLは 今後も解決し続ける必要があり、紙は刷り直せない。 スキャン経路は id = ? OR short_code = ? の1クエリで両形式を受ける。 形状判定(36桁ハイフン区切りならUUID)は採らない。idは任意のTEXTで、負荷テストの シーダーは lt-qr-001-001 形式を使うため壊れる。2クエリにするとアプリで最も熱い 経路のサブリクエストが倍になる。両列とも索引付き(idは主キー、short_codeは一意 索引)なので索引探索2回で済む。 アクセスログの書き込みで、スキャンされた値ではなく行自身のidを使うよう修正した。 AccessLogs.qr_id は QRCodes(id) への外部キーなので、短縮コードでスキャンされると 制約違反になる。この書き込みは waitUntil でレスポンス経路の外にあるため、 リダイレクトは成功したまま短縮コード経由のスキャンだけが記録されずに消えていた。 文字集合は 23456789abcdefghijkmnpqrstuvwxyz。0/1/l/o を除いて読み間違いを防ぎ、 小文字のみにしたのは SQLite の TEXT 比較が既定で大文字小文字を区別するため。 ちょうど32文字なので byte & 31 で偏りなく写像できる。1文字減らすと生成器に 偏りが出る。URL安全な文字だけなので、qrTargetUrl が id を生で補間している (パーセントエンコードしない)前提を保てる。 既存行には scripts/backfill-short-codes.mjs でコードを割り当てる。 マイグレーションとバックフィルは対で、後者を実行するまで既存行は NULL のまま (UUID形式では引き続き解決する)。ローカル255行で検証済み、二度目は no-op。 Co-Authored-By: Claude Opus 5 (1M context) --- .../migrations/0004_add_qrcode_short_code.sql | 48 ++++ packages/api/schema.sql | 31 ++- packages/api/scripts/backfill-short-codes.mjs | 263 ++++++++++++++++++ packages/api/src/db/schema.ts | 16 +- packages/api/src/errors.ts | 9 + packages/api/src/routes/forward.ts | 31 ++- packages/api/src/routes/projects.ts | 67 ++++- packages/api/src/short-code.test.ts | 219 +++++++++++++++ packages/api/src/short-code.ts | 128 +++++++++ packages/web/src/lib/api.ts | 1 + packages/web/src/lib/i18n.tsx | 4 + packages/web/src/lib/qr.test.ts | 47 +++- packages/web/src/lib/qr.ts | 28 +- packages/web/src/pages/QRCodesPage.tsx | 27 +- 14 files changed, 883 insertions(+), 36 deletions(-) create mode 100644 packages/api/migrations/0004_add_qrcode_short_code.sql create mode 100644 packages/api/scripts/backfill-short-codes.mjs create mode 100644 packages/api/src/short-code.test.ts create mode 100644 packages/api/src/short-code.ts diff --git a/packages/api/migrations/0004_add_qrcode_short_code.sql b/packages/api/migrations/0004_add_qrcode_short_code.sql new file mode 100644 index 0000000..13b36f7 --- /dev/null +++ b/packages/api/migrations/0004_add_qrcode_short_code.sql @@ -0,0 +1,48 @@ +-- Adds QRCodes.short_code: a 7-character stand-in for the UUID `id` in the URL a +-- printed QR code carries. +-- +-- Background: the scan URL was `/?id=&p=`. Measured against the +-- deployed host at error-correction level H: +-- +-- 103 chars 57x57 /?id=550e8400-e29b-41d4-a716-446655440000&p=instagram +-- 74 chars 49x49 /?id=q7mfe3x&p=instagram +-- +-- Fewer modules means physically larger modules at the same printed size, which +-- is what decides whether a poster still reads from across a corridor or in bad +-- light. Nothing else about the app changes. +-- +-- A NEW COLUMN, not a shorter `id`. AccessLogs.qr_id is a foreign key into +-- QRCodes(id) with ON DELETE CASCADE, and the id is baked into flyers that are +-- already on walls — see the comment on the table in schema.sql. Shortening the +-- id in place would break both. The scan endpoint therefore resolves +-- `id = ? OR short_code = ?`, so a poster carrying either form keeps working +-- forever, including after a row has been backfilled with a short code. +-- +-- THIS MIGRATION AND THE BACKFILL ARE A PAIR — run the backfill immediately +-- after this file. The column is nullable so that adding it to a populated +-- database cannot fail, which means every existing row lands here with no code, +-- and the admin UI shows only the short URL. Rows left un-backfilled fall back to +-- rendering the long UUID URL, i.e. exactly the symbol size this change exists to +-- remove: +-- +-- node scripts/backfill-short-codes.mjs --local +-- node scripts/backfill-short-codes.mjs --remote +-- +-- The unique index is what makes the create path's bounded collision retry +-- meaningful — without it a duplicate code would silently point two posters at +-- one row. SQLite treats NULLs as distinct in a UNIQUE index, so the rows this +-- file leaves empty do not collide with each other; that is what allows the +-- column to be added before the backfill fills it in. +-- +-- Idempotent? No — re-running errors with "duplicate column name". A brand-new +-- database does not need this file at all; schema.sql already has the column and +-- the index. +-- +-- Apply with: +-- wrangler d1 execute trackinglink-db --local --file=./migrations/0004_add_qrcode_short_code.sql +-- wrangler d1 execute trackinglink-db --remote --file=./migrations/0004_add_qrcode_short_code.sql +-- Then re-run schema.sql, then run the backfill above. + +ALTER TABLE QRCodes ADD COLUMN short_code TEXT; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_qrcodes_short_code ON QRCodes(short_code); diff --git a/packages/api/schema.sql b/packages/api/schema.sql index 66091fa..739fbfa 100644 --- a/packages/api/schema.sql +++ b/packages/api/schema.sql @@ -31,11 +31,29 @@ CREATE TABLE IF NOT EXISTS Projects ( -- QR code *metadata*. The QR image itself is never stored: the web app renders -- it client-side on every view (see useQRDataUrl in QRCodesPage.tsx). A row --- exists because the id is baked into the printed URL (`/?id=`), so it has --- to be stable forever — regenerating it would break already-posted flyers — --- and because AccessLogs joins on it to attribute scans to a medium/location. +-- exists because the identifier is baked into the printed URL (`/?id=<...>`), so +-- it has to be stable forever — regenerating it would break already-posted +-- flyers — and because AccessLogs joins on it to attribute scans to a +-- medium/location. +-- +-- Two columns can address a row: `id` and `short_code`. New QR codes are printed +-- with the short code; flyers printed before it existed carry the id. The scan +-- endpoint accepts either, and neither is ever reassigned. CREATE TABLE IF NOT EXISTS QRCodes ( id TEXT PRIMARY KEY, + -- What new QR codes put in the printed URL, in place of the 36-character id: + -- `/?id=q7mfe3x&p=instagram` is 74 characters and encodes as a 49x49 symbol, + -- against 103 characters and 57x57 for the id. Bigger modules at the same + -- printed size is the entire benefit. 7 characters from a 32-character + -- alphabet that omits 0, 1, l and o, so a code survives being read aloud, and + -- lowercase only because TEXT comparison here is case-sensitive. + -- + -- Nullable only so migrations/0004 can add it to a populated database without + -- inventing values in SQL. Every row is expected to carry one — the migration + -- is paired with scripts/backfill-short-codes.mjs, which fills in the rows + -- that predate the column. Never overwrite one: it is as printed, and as + -- permanent, as the id. + short_code TEXT, -- What the QR code is printed on, e.g. "造形大ポスター". Unique per project: -- one QR code per physical item. name TEXT NOT NULL, @@ -74,6 +92,13 @@ CREATE INDEX IF NOT EXISTS idx_qrcodes_project_id ON QRCodes(project_id); -- (built from the name) unique. CREATE UNIQUE INDEX IF NOT EXISTS idx_qrcodes_project_name ON QRCodes(project_id, name); +-- A short code is what a scan resolves against, so a duplicate would attribute +-- two posters' scans to one row and send some visitors to the wrong project. Also +-- the backstop the create path's collision retry relies on. NULLs count as +-- distinct in a SQLite UNIQUE index, which is what lets migrations/0004 add the +-- column before the backfill populates it. +CREATE UNIQUE INDEX IF NOT EXISTS idx_qrcodes_short_code ON QRCodes(short_code); + -- Serves both the access-log list and the CSV export, which filter by -- project_id and order by accessed_at DESC — the leading column also covers -- plain project_id lookups and COUNT(*), so no separate index on project_id is diff --git a/packages/api/scripts/backfill-short-codes.mjs b/packages/api/scripts/backfill-short-codes.mjs new file mode 100644 index 0000000..2508362 --- /dev/null +++ b/packages/api/scripts/backfill-short-codes.mjs @@ -0,0 +1,263 @@ +// Assigns a short code to every QRCodes row that does not have one yet. +// +// Pairs with migrations/0004_add_qrcode_short_code.sql. That migration adds +// short_code as a nullable column — it has to be nullable, because SQL alone +// cannot invent a value that satisfies the alphabet and the unique index — so +// every row that existed before it lands with no code. This script fills them in. +// +// It is not optional. The admin UI renders only the short URL, so a row left +// without a code falls back to the 36-character UUID form and prints the 57x57 +// symbol that the whole change exists to avoid. Real, already-printed QR codes are +// affected: production held 13 rows when this was written, and a local +// development database typically holds a couple of hundred. +// +// Backfilling does NOT invalidate anything already in print. The scan endpoint +// resolves `id = ? OR short_code = ?`, so a poster carrying the UUID keeps +// working after its row gains a short code — the two are additional handles on +// the same row, and neither is ever reassigned. +// +// Why codes are generated here in JavaScript rather than in SQL: SQLite's +// randomblob()/hex() emits hex, which contains 0 and 1 — characters the alphabet +// deliberately omits so a code can be read aloud — and it has no way to react to +// a unique-index collision. Generating them here uses the same alphabet and the +// same bounded retry as the create path. +// +// Usage: +// node scripts/backfill-short-codes.mjs --local +// node scripts/backfill-short-codes.mjs --remote +// node scripts/backfill-short-codes.mjs --local --dry-run +// +// Safe to run twice: it only ever touches rows WHERE short_code IS NULL, so a +// second run reports 0 updated and writes nothing. +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Kept identical to SHORT_CODE_ALPHABET / SHORT_CODE_LENGTH in src/short-code.ts. + * + * Duplicated rather than imported because this file is run by plain `node`, which + * cannot import a TypeScript module, and adding a build step for one 8-line + * function would cost more than it saves. src/short-code.test.ts imports both and + * asserts they agree, so the two cannot drift silently — if you change one, that + * test fails. + */ +export const SHORT_CODE_ALPHABET = '23456789abcdefghijkmnpqrstuvwxyz'; +export const SHORT_CODE_LENGTH = 7; + +/** @returns {string} */ +export function generateShortCode() { + const bytes = new Uint8Array(SHORT_CODE_LENGTH); + crypto.getRandomValues(bytes); + let code = ''; + for (const byte of bytes) { + // 256 is a whole multiple of 32, so the low 5 bits are uniform. + code += SHORT_CODE_ALPHABET[byte & 31]; + } + return code; +} + +/** Matches SHORT_CODE_ATTEMPTS in src/routes/projects.ts. */ +const ATTEMPTS_PER_ROW = 5; + +const DB_NAME = 'trackinglink-db'; + +function parseArgs(argv) { + const args = { target: '', db: DB_NAME, dryRun: false }; + for (const arg of argv) { + const [key, value] = arg.replace(/^--/, '').split('='); + if (key === 'local' || key === 'remote') args.target = key; + else if (key === 'dry-run') args.dryRun = true; + else if (key === 'db') args.db = value ?? DB_NAME; + else { + console.error(`unknown argument: ${arg}`); + process.exit(1); + } + } + return args; +} + +const wranglerBin = join( + 'node_modules', + '.bin', + process.platform === 'win32' ? 'wrangler.CMD' : 'wrangler', +); + +/** + * Windows needs `shell: true` to launch wrangler.CMD at all, and a shell splits + * arguments on spaces — so `--command=SELECT id FROM QRCodes` arrives as ten + * arguments and wrangler rejects it. Quoting has to be explicit here; the SQL + * built below only ever uses single quotes, so wrapping in double quotes is safe. + */ +const useShell = process.platform === 'win32'; + +function quoteArg(arg) { + if (!useShell || !/[\s"]/.test(arg)) return arg; + return `"${arg.replace(/"/g, '\\"')}"`; +} + +/** + * Runs one `wrangler d1 execute` and returns its combined output. + * + * Failure detection copies loadtest/seed/run.mjs deliberately: a bare /error/i + * also matches wrangler's own "update to prevent critical errors" version notice, + * which makes every run look broken. `[ERROR]` is wrangler's actual marker. + */ +function d1(args, extra) { + const argv = ['d1', 'execute', args.db, `--${args.target}`, ...extra]; + const result = spawnSync(wranglerBin, argv.map(quoteArg), { + encoding: 'utf8', + shell: useShell, + }); + const output = `${result.stdout ?? ''}${result.stderr ?? ''}`; + if (result.status !== 0 || /\[ERROR\]/.test(output)) { + throw new Error(`wrangler d1 execute failed:\n${output}`); + } + return output; +} + +/** + * Pulls the result rows out of `wrangler d1 execute --json` output. + * + * Wrangler prints a version banner and other chatter around the JSON, so the + * payload is located rather than assumed to be the whole of stdout. + */ +function queryRows(args, sql) { + const output = d1(args, [`--command=${sql}`, '--json']); + const start = output.indexOf('['); + const end = output.lastIndexOf(']'); + if (start === -1 || end <= start) { + throw new Error(`could not find JSON in wrangler output:\n${output}`); + } + const parsed = JSON.parse(output.slice(start, end + 1)); + return parsed.flatMap((entry) => entry.results ?? []); +} + +/** Doubles single quotes — ids are arbitrary TEXT, not necessarily UUIDs. */ +function sqlQuote(value) { + return `'${String(value).replace(/'/g, "''")}'`; +} + +async function main() { + const args = parseArgs(process.argv.slice(2)); + + if (!args.target) { + console.error( + [ + 'Specify --local or --remote.', + '', + ' node scripts/backfill-short-codes.mjs --local', + ' node scripts/backfill-short-codes.mjs --remote', + ].join('\n'), + ); + process.exit(1); + } + + // Same hazard as the loadtest seeder: workerd holding the local database makes + // writes fail in a confusing way. + if (args.target === 'local') { + console.log( + 'NOTE: stop `wrangler dev` first — workerd holds the local DB.\n', + ); + } + + const pending = queryRows( + args, + 'SELECT id FROM QRCodes WHERE short_code IS NULL ORDER BY id', + ); + + if (pending.length === 0) { + console.log('Every QR code already has a short code — nothing to do.'); + return; + } + + // Read the codes already in use so collisions are resolved before writing + // rather than by absorbing a failed statement. The unique index remains the + // real arbiter; this just makes a collision cost nothing. + const taken = new Set( + queryRows( + args, + 'SELECT short_code FROM QRCodes WHERE short_code IS NOT NULL', + ).map((row) => row.short_code), + ); + + console.log( + `${pending.length} QR code(s) without a short code; ${taken.size} code(s) already in use.`, + ); + + const updates = []; + for (const row of pending) { + let code = ''; + for (let attempt = 0; attempt < ATTEMPTS_PER_ROW; attempt++) { + const candidate = generateShortCode(); + if (!taken.has(candidate)) { + code = candidate; + break; + } + } + if (!code) { + // Impossible at 32^7 codes against this many rows, so treat it as a broken + // generator rather than bad luck and stop before writing anything. + throw new Error( + `could not find a free short code for ${row.id} in ${ATTEMPTS_PER_ROW} attempts; check generateShortCode.`, + ); + } + taken.add(code); + // `AND short_code IS NULL` makes each statement individually idempotent, so + // a partially-applied run can be repeated without overwriting a code that + // has already been printed. + updates.push( + `UPDATE QRCodes SET short_code = ${sqlQuote(code)} WHERE id = ${sqlQuote(row.id)} AND short_code IS NULL;`, + ); + } + + if (args.dryRun) { + console.log(`\n--dry-run: would apply ${updates.length} update(s).`); + for (const statement of updates.slice(0, 5)) console.log(` ${statement}`); + if (updates.length > 5) console.log(` … ${updates.length - 5} more`); + return; + } + + // One wrangler invocation for the whole batch. Per-row invocations would be + // correct but take minutes for a few hundred rows, since each spawn pays + // wrangler's startup cost. + const dir = mkdtempSync(join(tmpdir(), 'tl-shortcode-')); + const sqlPath = join(dir, 'backfill.sql'); + try { + writeFileSync(sqlPath, `${updates.join('\n')}\n`, 'utf8'); + d1(args, [`--file=${sqlPath}`]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + + const [remaining] = queryRows( + args, + 'SELECT COUNT(*) AS n FROM QRCodes WHERE short_code IS NULL', + ); + const left = Number(remaining?.n ?? 0); + + console.log( + `\nUpdated ${updates.length} row(s) in ${args.db} (${args.target}).`, + ); + if (left > 0) { + console.error( + `${left} row(s) still have no short code — re-run this script; it only touches NULLs.`, + ); + process.exit(1); + } + console.log('All QR codes now have a short code.'); +} + +// Importable for tests without running the backfill. +const invokedDirectly = + process.argv[1] && + resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url)); + +if (invokedDirectly) { + main().catch((error) => { + console.error(`\n${error.message}`); + process.exit(1); + }); +} diff --git a/packages/api/src/db/schema.ts b/packages/api/src/db/schema.ts index 8fbea24..6243f10 100644 --- a/packages/api/src/db/schema.ts +++ b/packages/api/src/db/schema.ts @@ -23,13 +23,22 @@ export const projects = sqliteTable('Projects', { }); // QR code *metadata*. The image is never persisted — the web app regenerates it -// client-side on every view. The row exists because `id` is baked into the -// printed URL (`/?id=`) and so has to stay stable forever, and because +// client-side on every view. The row exists because the identifier is baked into +// the printed URL (`/?id=<...>`) and so has to stay stable forever, and because // AccessLogs joins on it. export const qrCodes = sqliteTable( 'QRCodes', { id: text('id').primaryKey(), + // Short stand-in for `id` in the printed URL, which takes the symbol from + // 57x57 modules to 49x49 — see ../short-code.ts for the measurements and the + // alphabet. + // + // Nullable because migrations/0004 adds it to existing rows empty and a + // separate backfill fills them in; treat it as present everywhere and fall + // back to `id` when it is not, since the scan endpoint resolves either. Never + // reassign one — it is as printed, and as permanent, as the id. + shortCode: text('short_code'), projectId: text('project_id') .notNull() .references(() => projects.projectId, { onDelete: 'cascade' }), @@ -57,6 +66,9 @@ export const qrCodes = sqliteTable( table.projectId, table.name, ), + // The backstop behind the create path's collision retry. A duplicate would + // point two posters at one row. + shortCode: uniqueIndex('idx_qrcodes_short_code').on(table.shortCode), }), ); diff --git a/packages/api/src/errors.ts b/packages/api/src/errors.ts index a5149bc..2c96a46 100644 --- a/packages/api/src/errors.ts +++ b/packages/api/src/errors.ts @@ -25,6 +25,13 @@ export const ErrorCodes = { PROJECT_NOT_FOUND: 'PROJECT_NOT_FOUND', QR_CODE_NOT_FOUND: 'QR_CODE_NOT_FOUND', DUPLICATE_NAME: 'DUPLICATE_NAME', + /** + * Could not mint a unique short code for a new QR code. Distinct from a + * generic 500 because it is retryable by simply trying again, and because + * seeing it at all would mean the keyspace assumption in short-code.ts is + * wrong. + */ + SHORT_CODE_UNAVAILABLE: 'SHORT_CODE_UNAVAILABLE', CSV_EXPORT_DISABLED: 'CSV_EXPORT_DISABLED', TOO_MANY_ROWS: 'TOO_MANY_ROWS', RATE_LIMITED: 'RATE_LIMITED', @@ -43,6 +50,8 @@ const FALLBACK_MESSAGES: Record = { PROJECT_NOT_FOUND: 'Project not found', QR_CODE_NOT_FOUND: 'QR code not found', DUPLICATE_NAME: 'A QR code with this name already exists in this project', + SHORT_CODE_UNAVAILABLE: + 'Could not allocate a short code for this QR code — please try again', CSV_EXPORT_DISABLED: 'CSV export is not enabled', TOO_MANY_ROWS: 'Too many rows — narrow the date range', RATE_LIMITED: 'Too many attempts — try again shortly', diff --git a/packages/api/src/routes/forward.ts b/packages/api/src/routes/forward.ts index 819799b..c8a517c 100644 --- a/packages/api/src/routes/forward.ts +++ b/packages/api/src/routes/forward.ts @@ -1,4 +1,4 @@ -import { eq } from 'drizzle-orm'; +import { eq, or } from 'drizzle-orm'; import { type Context, Hono } from 'hono'; import { getConnInfo } from 'hono/cloudflare-workers'; import { html } from 'hono/html'; @@ -22,9 +22,13 @@ const reportedMissingKeywords = new Set(); // GET /?id=&p= — scan a QR code: look it up, log the access, // redirect. // +// `id` is either a QRCodes.id (a UUID, on flyers printed before short codes +// existed) or a QRCodes.short_code. Both are accepted for the lifetime of the +// app: paper cannot be reissued, so neither form may ever stop resolving. +// // This is the only hot path in the app: every person who scans a printed poster -// goes through it, in bursts. It is deliberately one D1 read (a primary-key -// lookup) on the critical path — see the join and the waitUntil below. +// goes through it, in bursts. It is deliberately one D1 read on the critical path +// — see the join and the waitUntil below. forwardApp.get('/', async (c) => { const { id, p: fallbackKey } = c.req.query(); const userAgent = c.req.header('User-Agent') || 'unknown'; @@ -41,6 +45,13 @@ forwardApp.get('/', async (c) => { // fallback. let target: | { + /** + * The row's own id, which is not necessarily what was scanned. The + * access-log insert below needs this rather than the query parameter: + * AccessLogs.qr_id is a foreign key into QRCodes(id), so writing a + * short code there would fail the constraint and lose the scan. + */ + id: string; projectId: string; location: string; projectName: string | null; @@ -64,6 +75,7 @@ forwardApp.get('/', async (c) => { // the two 404 branches below rely on. target = await db .select({ + id: schema.qrCodes.id, projectId: schema.qrCodes.projectId, location: schema.qrCodes.location, projectName: schema.projects.name, @@ -74,7 +86,13 @@ forwardApp.get('/', async (c) => { schema.projects, eq(schema.qrCodes.projectId, schema.projects.projectId), ) - .where(eq(schema.qrCodes.id, id)) + // One OR rather than a shape test on `id`, and rather than a second query. + // A shape test ("36 chars with hyphens is a UUID") would be wrong: ids are + // arbitrary TEXT, and the load-test harness seeds them as `lt-qr-001-001`. + // A second query would double the subrequests on the hottest path in the + // app. Both columns are indexed — id is the primary key, short_code has a + // unique index — so this stays two index probes, not a scan. + .where(or(eq(schema.qrCodes.id, id), eq(schema.qrCodes.shortCode, id))) .get(); } catch (error) { dbUnavailable = true; @@ -122,7 +140,10 @@ forwardApp.get('/', async (c) => { const write = db .insert(schema.accessLogs) .values({ - qrId: id, + // target.id, not the scanned `id`: the latter may be a short code, which + // the qr_id foreign key would reject. Logging the row's own id also keeps + // scan history joinable regardless of which URL form was printed. + qrId: target.id, projectId: target.projectId, accessedAt: new Date().toISOString(), userAgent, diff --git a/packages/api/src/routes/projects.ts b/packages/api/src/routes/projects.ts index 7c4168a..1906a25 100644 --- a/packages/api/src/routes/projects.ts +++ b/packages/api/src/routes/projects.ts @@ -17,7 +17,9 @@ import type { HonoEnv } from '../auth'; import { getDb, schema } from '../db'; import { ErrorCodes, fail } from '../errors'; import { parseFallbackMap } from '../fallback'; +import { logEvent } from '../log'; import { Permissions, hasPermission } from '../permissions'; +import { ShortCodeExhaustedError, insertWithShortCode } from '../short-code'; // Length caps keep a single row (and therefore the database, and the CSV export) // bounded. Without them a 10k-character name is accepted and then wrecks every @@ -125,6 +127,30 @@ function isUniqueConstraintError(error: unknown): boolean { ); } +/** + * True if the violated constraint was specifically the short-code index. + * + * QRCodes carries two unique indexes — (project_id, name) and short_code — and + * they need opposite handling: a name clash is the user's to fix (409), a short + * code clash is ours to retry silently. SQLite names the offending columns in the + * message ("UNIQUE constraint failed: QRCodes.short_code"), which is the only + * signal available to tell them apart. + * + * Getting this wrong is not hypothetical: without the column check, creating a + * second QR code with a name already in use would be treated as a collision, + * retried five times, and then reported as "could not allocate a short code" + * instead of "that name is taken". + * + * Exported for tests — the two messages are the whole contract. + */ +export function isShortCodeCollision(error: unknown): boolean { + return ( + isUniqueConstraintError(error) && + error instanceof Error && + /QRCodes\.short_code/i.test(error.message) + ); +} + /** * Returns a 403 response when the caller lacks `permission`, or null to proceed. * @@ -1098,24 +1124,44 @@ projectsApp.post('/:id/qrcodes', async (c) => { } const { name, medium, location } = parsed.data; + // The UUID stays: it is the primary key AccessLogs cascades from, and QR codes + // already in print address rows by it. The short code is an additional handle, + // and the one new posters carry — see ../short-code.ts. const qrId = crypto.randomUUID(); const createdAt = new Date().toISOString(); const db = getDb(c.env.DB); + + let shortCode: string; try { - await db.insert(schema.qrCodes).values({ - id: qrId, - projectId, - name, - medium, - location: location ?? '', - createdAt, - creatorId: user?.sub ?? null, - }); + ({ shortCode } = await insertWithShortCode( + (candidate) => + db.insert(schema.qrCodes).values({ + id: qrId, + projectId, + name, + medium, + location: location ?? '', + shortCode: candidate, + createdAt, + creatorId: user?.sub ?? null, + }), + isShortCodeCollision, + )); } catch (error) { + if (error instanceof ShortCodeExhaustedError) { + // Never expected to happen — see SHORT_CODE_ATTEMPTS. Logged because if it + // ever does, the generator is at fault and nothing else would say so. + logEvent('qr_short_code_exhausted', { + projectId, + attempts: error.attempts, + }); + return fail(c, 503, ErrorCodes.SHORT_CODE_UNAVAILABLE); + } if (isUniqueConstraintError(error)) { // One QR code per named item within a project. `fields` lets the web app - // mark the offending input rather than making the user guess. + // mark the offending input rather than making the user guess. Reached + // rather than retried because isShortCodeCollision rejected it. return fail(c, 409, ErrorCodes.DUPLICATE_NAME, { fields: ['name'] }); } throw error; @@ -1128,6 +1174,7 @@ projectsApp.post('/:id/qrcodes', async (c) => { name, medium, location: location ?? '', + shortCode, createdAt, }, 201, diff --git a/packages/api/src/short-code.test.ts b/packages/api/src/short-code.test.ts new file mode 100644 index 0000000..272c035 --- /dev/null +++ b/packages/api/src/short-code.test.ts @@ -0,0 +1,219 @@ +import { describe, expect, it } from 'vitest'; +// The backfill script carries its own copy of the generator because plain `node` +// cannot import a .ts module. These tests are what stop the two from drifting. +import { + SHORT_CODE_ALPHABET as SCRIPT_ALPHABET, + SHORT_CODE_LENGTH as SCRIPT_LENGTH, + generateShortCode as scriptGenerateShortCode, +} from '../scripts/backfill-short-codes.mjs'; +import { isShortCodeCollision } from './routes/projects'; +import { + SHORT_CODE_ALPHABET, + SHORT_CODE_ATTEMPTS, + SHORT_CODE_LENGTH, + ShortCodeExhaustedError, + generateShortCode, + insertWithShortCode, +} from './short-code'; + +/** + * A short code ends up printed on paper and is then permanent. Every property + * asserted here is one that, if broken, is only discovered after a print run: + * a code that needs percent-encoding, a character that cannot be transcribed, or + * a length that changes the symbol size. + */ + +const SAMPLE_SIZE = 2000; +const samples = Array.from({ length: SAMPLE_SIZE }, () => generateShortCode()); + +describe('SHORT_CODE_ALPHABET', () => { + it('is 32 characters, so the low 5 bits of a byte map onto it without bias', () => { + expect(SHORT_CODE_ALPHABET).toHaveLength(32); + }); + + it('contains no duplicates', () => { + expect(new Set(SHORT_CODE_ALPHABET).size).toBe(SHORT_CODE_ALPHABET.length); + }); + + it('omits the characters that get misread when a code is read aloud', () => { + // 0/o and 1/l. Their absence is why the alphabet is 32 rather than 36 — which + // is also what makes the bit-masking uniform, so this cannot be "fixed" by + // dropping more characters. + for (const char of ['0', '1', 'l', 'o']) { + expect(SHORT_CODE_ALPHABET).not.toContain(char); + } + }); + + it('keeps `i`, which would otherwise leave 31 characters', () => { + // Pinned deliberately: `i` looks droppable alongside 1 and l, but removing it + // breaks the `byte & 31` mapping and biases the generator. If `i` ever has to + // go, the generator needs rejection sampling first. + expect(SHORT_CODE_ALPHABET).toContain('i'); + }); + + it('is lowercase only, so the printed URL is not case-sensitive', () => { + // SQLite compares TEXT case-sensitively by default: with a mixed-case + // alphabet, `/?id=Q7mfe3x` would 404 while `/?id=q7mfe3x` resolved. + expect(SHORT_CODE_ALPHABET).toBe(SHORT_CODE_ALPHABET.toLowerCase()); + }); +}); + +describe('generateShortCode', () => { + it(`is ${SHORT_CODE_LENGTH} characters`, () => { + for (const code of samples) expect(code).toHaveLength(SHORT_CODE_LENGTH); + }); + + it('uses only characters from the alphabet', () => { + for (const code of samples) { + for (const char of code) expect(SHORT_CODE_ALPHABET).toContain(char); + } + }); + + it('needs no percent-encoding to sit in a URL', () => { + // packages/web/src/lib/qr.ts interpolates the code into the scan URL raw — + // only the fallback keyword is passed through encodeURIComponent. A code that + // changed under encoding would produce a QR pointing at a URL that does not + // resolve, discovered only after printing. + for (const code of samples) { + expect(encodeURIComponent(code)).toBe(code); + } + }); + + it('round-trips through URL parsing unchanged', () => { + // The end-to-end version of the check above: what the Worker reads out of + // `?id=` must be exactly what was generated. + for (const code of samples.slice(0, 200)) { + const url = new URL(`https://example.com/?id=${code}&p=instagram`); + expect(url.searchParams.get('id')).toBe(code); + } + }); + + it('does not repeat itself', () => { + // Not a uniqueness guarantee — that is the database's job — but 2000 codes + // colliding would mean the generator is broken (e.g. a fixed seed). + expect(new Set(samples).size).toBe(SAMPLE_SIZE); + }); + + it('reaches every character in the alphabet across enough samples', () => { + // Catches an off-by-one in the bit masking, which would silently make part of + // the alphabet unreachable and shrink the keyspace. + const seen = new Set(samples.join('')); + expect(seen.size).toBe(SHORT_CODE_ALPHABET.length); + }); +}); + +/** The two messages SQLite actually produces for QRCodes' two unique indexes. */ +const SHORT_CODE_VIOLATION = new Error( + 'D1_ERROR: UNIQUE constraint failed: QRCodes.short_code: SQLITE_CONSTRAINT', +); +const NAME_VIOLATION = new Error( + 'D1_ERROR: UNIQUE constraint failed: QRCodes.project_id, QRCodes.name: SQLITE_CONSTRAINT', +); + +describe('isShortCodeCollision', () => { + it('recognises a short-code violation', () => { + expect(isShortCodeCollision(SHORT_CODE_VIOLATION)).toBe(true); + }); + + it('does not mistake a duplicate name for a collision', () => { + // The failure this guard exists to prevent: a name clash retried five times + // and then reported as a short-code problem, leaving the user with no idea + // that their chosen name was taken. + expect(isShortCodeCollision(NAME_VIOLATION)).toBe(false); + }); + + it('ignores unrelated failures', () => { + for (const error of [ + new Error('D1_ERROR: no such table: QRCodes'), + new Error('NOT NULL constraint failed: QRCodes.name'), + new Error('network error'), + 'a string', + null, + undefined, + ]) { + expect(isShortCodeCollision(error)).toBe(false); + } + }); +}); + +describe('insertWithShortCode', () => { + it('passes a valid code to the insert and reports the one that stuck', async () => { + const seen: string[] = []; + const { shortCode, result } = await insertWithShortCode(async (code) => { + seen.push(code); + return `row:${code}`; + }, isShortCodeCollision); + expect(seen).toEqual([shortCode]); + expect(result).toBe(`row:${shortCode}`); + expect(shortCode).toHaveLength(SHORT_CODE_LENGTH); + }); + + it('retries with a *fresh* code after a collision', async () => { + // Retrying with the same code would loop until the attempt budget ran out and + // then fail, which is indistinguishable from a broken generator. + const seen: string[] = []; + const { shortCode } = await insertWithShortCode(async (code) => { + seen.push(code); + if (seen.length < 3) throw SHORT_CODE_VIOLATION; + return 'ok'; + }, isShortCodeCollision); + + expect(seen).toHaveLength(3); + expect(new Set(seen).size).toBe(3); + expect(shortCode).toBe(seen[2]); + }); + + it(`gives up after ${SHORT_CODE_ATTEMPTS} attempts`, async () => { + let calls = 0; + await expect( + insertWithShortCode(async () => { + calls++; + throw SHORT_CODE_VIOLATION; + }, isShortCodeCollision), + ).rejects.toThrow(ShortCodeExhaustedError); + expect(calls).toBe(SHORT_CODE_ATTEMPTS); + }); + + it('rethrows a duplicate name immediately instead of retrying it', async () => { + // Retrying a name clash would waste four writes and then report the wrong + // error to the user. + let calls = 0; + await expect( + insertWithShortCode(async () => { + calls++; + throw NAME_VIOLATION; + }, isShortCodeCollision), + ).rejects.toThrow(/QRCodes\.project_id/); + expect(calls).toBe(1); + }); + + it('rethrows anything unrelated without retrying', async () => { + let calls = 0; + await expect( + insertWithShortCode(async () => { + calls++; + throw new Error('D1_ERROR: database is locked'); + }, isShortCodeCollision), + ).rejects.toThrow('database is locked'); + expect(calls).toBe(1); + }); +}); + +describe('the backfill script generator', () => { + // Backfilled and freshly-created codes have to be indistinguishable: they are + // the same kind of identifier, printed the same way, and any difference would + // mean the alphabet guarantees above hold for only some rows. + it('shares the alphabet and length with src/short-code.ts', () => { + expect(SCRIPT_ALPHABET).toBe(SHORT_CODE_ALPHABET); + expect(SCRIPT_LENGTH).toBe(SHORT_CODE_LENGTH); + }); + + it('produces codes indistinguishable from the create path', () => { + const pattern = new RegExp( + `^[${SHORT_CODE_ALPHABET}]{${SHORT_CODE_LENGTH}}$`, + ); + for (let i = 0; i < 500; i++) { + expect(scriptGenerateShortCode()).toMatch(pattern); + } + }); +}); diff --git a/packages/api/src/short-code.ts b/packages/api/src/short-code.ts new file mode 100644 index 0000000..ec1b4d6 --- /dev/null +++ b/packages/api/src/short-code.ts @@ -0,0 +1,128 @@ +/** + * Short codes: the identifier a printed QR code carries instead of a UUID. + * + * The scan URL used to be `/?id=&p=`. Measured against the + * deployed host, at error-correction level H: + * + * 103 chars 57x57 /?id=550e8400-e29b-41d4-a716-446655440000&p=instagram + * 74 chars 49x49 /?id=q7mfe3x&p=instagram + * + * Fewer modules means physically larger modules at the same printed size, which + * is what decides whether a poster still reads from across a corridor or in bad + * light. That is the whole reason this module exists — it buys nothing else. + * + * A short code does NOT replace QRCodes.id, and adding one must never change an + * existing row's id: AccessLogs.qr_id is a foreign key into QRCodes(id) with ON + * DELETE CASCADE, and the id is already baked into flyers that are on walls. The + * scan endpoint resolves `id = ? OR short_code = ?` so both forms keep working + * forever — see migrations/0004_add_qrcode_short_code.sql. + * + * Pure and dependency-free, so the alphabet and the length can be tested without + * a database or a request context. + */ + +/** + * The 32 characters a short code may contain. + * + * The digits 0 and 1 and the letters l and o are omitted: during an event a code + * gets read aloud and transcribed by hand, and 0/o and 1/l are the pairs people + * get wrong. `i` stays — with both 1 and l gone there is nothing left for it to be + * confused with — and dropping it as well would leave 31 characters, which is what + * would actually hurt: the uniform bit-masking in generateShortCode depends on the + * alphabet dividing 256 exactly. + * + * Lowercase only because SQLite compares TEXT case-sensitively by default. A + * mixed-case code would make the printed URL case-sensitive, so `/?id=Q7mfe3x` + * would 404 while `/?id=q7mfe3x` resolved — a failure nobody would suspect while + * holding a correctly-printed poster. + * + * Every character here is an unreserved URL character, which is load-bearing + * rather than incidental: the web app interpolates the code into the scan URL + * raw, without encodeURIComponent (see qrTargetUrl in + * packages/web/src/lib/qr.ts). A character needing percent-encoding would either + * corrupt the URL or, once encoded, cost three characters of payload and push the + * symbol back up a version — undoing the point of shortening it. + */ +export const SHORT_CODE_ALPHABET = '23456789abcdefghijkmnpqrstuvwxyz'; + +/** + * 7 characters, i.e. 32^7 ≈ 3.4e10 possible codes. + * + * Chosen for the keyspace, not for the symbol: measured against the deployed + * host, every length from 5 to 10 encodes to the same 49x49 symbol, so the 6th + * and 7th characters are free in print terms. Spending them buys enough headroom + * that the create path's collision retry should never fire at this app's scale, + * while staying short enough to read aloud. + */ +export const SHORT_CODE_LENGTH = 7; + +/** + * How many codes to try before giving up on an insert. + * + * At 32^7 codes against a few thousand rows a single collision is already + * implausible, so five in a row means the generator is broken rather than that we + * were unlucky. Bounded rather than open-ended because retrying forever would + * hold a request open instead of surfacing that. + */ +export const SHORT_CODE_ATTEMPTS = 5; + +/** + * Thrown when every attempt collided. Its own type so the caller can answer with + * a specific code instead of a generic 500. + */ +export class ShortCodeExhaustedError extends Error { + constructor(readonly attempts: number) { + super(`could not mint a unique short code in ${attempts} attempts`); + this.name = 'ShortCodeExhaustedError'; + } +} + +/** + * Runs `insert` with a fresh short code, retrying while `isCollision` says the + * failure was this column's unique index. + * + * Insert-and-retry rather than "SELECT then insert": a check first would still + * race two concurrent creates, so the unique index has to be the arbiter anyway, + * and in the overwhelmingly common no-collision case this costs one write. + * + * Errors `isCollision` rejects are rethrown untouched — that is what keeps a + * duplicate-name violation from being mistaken for a code collision and retried + * five times before failing with the wrong message. + * + * Split out from the route so the retry can be tested without a database. + */ +export async function insertWithShortCode( + insert: (shortCode: string) => Promise, + isCollision: (error: unknown) => boolean, + attempts = SHORT_CODE_ATTEMPTS, +): Promise<{ shortCode: string; result: T }> { + for (let attempt = 0; attempt < attempts; attempt++) { + const shortCode = generateShortCode(); + try { + return { shortCode, result: await insert(shortCode) }; + } catch (error) { + if (!isCollision(error)) throw error; + } + } + throw new ShortCodeExhaustedError(attempts); +} + +/** + * Mints a short code. Uniqueness is the database's job, not this function's — + * callers insert against the unique index and retry on a collision. + */ +export function generateShortCode(): string { + const bytes = new Uint8Array(SHORT_CODE_LENGTH); + // getRandomValues, not Math.random: these end up printed and effectively + // permanent, and Math.random is both biased and seedable across isolates. + crypto.getRandomValues(bytes); + + let code = ''; + for (const byte of bytes) { + // The alphabet is exactly 32 long and 256 is a whole multiple of 32, so + // taking the low 5 bits is uniform. With any other alphabet size this would + // silently over-represent the first few characters. + code += SHORT_CODE_ALPHABET[byte & 31]; + } + return code; +} diff --git a/packages/web/src/lib/api.ts b/packages/web/src/lib/api.ts index a4c7efc..da2fece 100644 --- a/packages/web/src/lib/api.ts +++ b/packages/web/src/lib/api.ts @@ -163,6 +163,7 @@ const CODE_TO_KEY: Record = { PROJECT_NOT_FOUND: 'error.projectNotFound', QR_CODE_NOT_FOUND: 'error.qrNotFound', DUPLICATE_NAME: 'error.duplicateName', + SHORT_CODE_UNAVAILABLE: 'error.shortCodeUnavailable', CSV_EXPORT_DISABLED: 'csvExport.disabled', TOO_MANY_ROWS: 'error.tooManyRows', RATE_LIMITED: 'error.rateLimited', diff --git a/packages/web/src/lib/i18n.tsx b/packages/web/src/lib/i18n.tsx index f2c391e..80be709 100644 --- a/packages/web/src/lib/i18n.tsx +++ b/packages/web/src/lib/i18n.tsx @@ -60,6 +60,8 @@ const en: Dictionary = { 'That QR code no longer exists. It may have been deleted.', 'error.duplicateName': 'A QR code with this name already exists in this project.', + 'error.shortCodeUnavailable': + 'Could not allocate a short code for this QR code. Please try again.', 'error.tooManyRows': 'Too many rows ({total}) to export at once. The limit is {max} — narrow the date range.', 'error.rateLimited': 'Too many attempts. Please wait a moment and try again.', @@ -230,6 +232,8 @@ const ja: Dictionary = { 'error.qrNotFound': 'このQRコードは存在しません。削除された可能性があります。', 'error.duplicateName': 'この名前のQRコードはこのプロジェクトに既にあります。', + 'error.shortCodeUnavailable': + '短縮コードを発行できませんでした。もう一度お試しください。', 'error.tooManyRows': '件数が多すぎます（{total}件）。一度に出力できるのは{max}件までです。期間を絞ってください。', 'error.rateLimited': diff --git a/packages/web/src/lib/qr.test.ts b/packages/web/src/lib/qr.test.ts index 5dcc74a..06da3bb 100644 --- a/packages/web/src/lib/qr.test.ts +++ b/packages/web/src/lib/qr.test.ts @@ -60,8 +60,10 @@ describe('deriveFallbackKey', () => { }); describe('qrTargetUrl', () => { + const UUID = '550e8400-e29b-41d4-a716-446655440000'; + it('includes the keyword when there is one', () => { - expect(qrTargetUrl('abc-123', 'instagram')).toMatch( + expect(qrTargetUrl({ id: 'abc-123' }, 'instagram')).toMatch( /\/\?id=abc-123&p=instagram$/, ); }); @@ -69,12 +71,49 @@ describe('qrTargetUrl', () => { it('omits &p= entirely when there is no keyword', () => { // A blank `&p=` is payload noise, and the Worker treats missing and blank // identically anyway. - expect(qrTargetUrl('abc-123', '')).toMatch(/\/\?id=abc-123$/); - expect(qrTargetUrl('abc-123')).toMatch(/\/\?id=abc-123$/); + expect(qrTargetUrl({ id: 'abc-123' }, '')).toMatch(/\/\?id=abc-123$/); + expect(qrTargetUrl({ id: 'abc-123' })).toMatch(/\/\?id=abc-123$/); }); it('percent-encodes the keyword', () => { - expect(qrTargetUrl('abc', 'a b')).toContain('&p=a%20b'); + expect(qrTargetUrl({ id: 'abc' }, 'a b')).toContain('&p=a%20b'); + }); + + // The reason the column exists: the short form is 74 characters and a 49x49 + // symbol, the UUID form 103 and 57x57. + it('prefers the short code over the id', () => { + expect( + qrTargetUrl({ id: UUID, shortCode: 'q7mfe3x' }, 'instagram'), + ).toMatch(/\/\?id=q7mfe3x&p=instagram$/); + }); + + it('never puts the id in the URL when a short code is present', () => { + const url = qrTargetUrl({ id: UUID, shortCode: 'q7mfe3x' }, 'instagram'); + expect(url).not.toContain(UUID); + }); + + it('falls back to the id when there is no short code', () => { + // Deliberate, not dead code: the Worker resolves either form, so a long URL + // still scans, whereas rendering nothing would produce an unusable poster. + // Covers a response from an older API (absent), a row the backfill has not + // reached (null), and a blank value. + for (const qr of [ + { id: UUID }, + { id: UUID, shortCode: null }, + { id: UUID, shortCode: '' }, + ]) { + expect(qrTargetUrl(qr, 'instagram')).toMatch( + new RegExp(`/\\?id=${UUID}&p=instagram$`), + ); + } + }); + + it('leaves a short code unencoded, so the printed URL is the short one', () => { + // The alphabet in packages/api/src/short-code.ts is URL-safe precisely so this + // interpolation can stay raw; percent-encoding here would add payload back. + const url = qrTargetUrl({ id: UUID, shortCode: 'q7mfe3x' }); + expect(url).toContain('?id=q7mfe3x'); + expect(url).not.toContain('%'); }); }); diff --git a/packages/web/src/lib/qr.ts b/packages/web/src/lib/qr.ts index 0f3dfd1..aa46f58 100644 --- a/packages/web/src/lib/qr.ts +++ b/packages/web/src/lib/qr.ts @@ -54,9 +54,28 @@ export function deriveFallbackKey(destinationUrl: string): string { ); } +/** + * The identifiers a scan URL can be built from — the subset of a QR code record + * that qrTargetUrl needs. + */ +export interface QrLinkIdentity { + id: string; + /** + * Preferred in the printed URL. Optional because a response from an API older + * than the short-code column omits it entirely, and null on a row the backfill + * has not reached. + */ + shortCode?: string | null; +} + /** * The URL baked into a printed QR code. * + * Built from the short code rather than the id: `/?id=q7mfe3x&p=instagram` is 74 + * characters and encodes as a 49x49 symbol, against 103 characters and 57x57 for + * the UUID. Bigger modules at the same printed size is the difference between a + * poster that scans from across a corridor and one that does not. + * * `&p=` is what lets a scan still reach somewhere sensible when the * Worker cannot read D1 — see packages/api/src/fallback.ts for why this is a * keyword rather than the destination URL itself. @@ -65,8 +84,13 @@ export function deriveFallbackKey(destinationUrl: string): string { * posters that have to be reprinted — and because a future bulk-print view needs * exactly this and nothing else from the dialog. */ -export function qrTargetUrl(qrId: string, fallbackKey = ''): string { - const base = `${FWD_BASE_URL}/?id=${qrId}`; +export function qrTargetUrl(qr: QrLinkIdentity, fallbackKey = ''): string { + // Falling back to the UUID is not dead code, even though the backfill leaves + // every row with a short code: the Worker resolves either form, so a long URL + // still scans correctly, whereas rendering no URL at all would produce a blank + // QR image and a poster nobody can use. A briefly-larger symbol is the far + // cheaper failure. Empty string is treated as absent along with null. + const base = `${FWD_BASE_URL}/?id=${qr.shortCode || qr.id}`; // Omitted rather than sent empty: `&p=` with no value is noise in the payload // and the Worker treats a missing key and a blank one identically. return fallbackKey ? `${base}&p=${encodeURIComponent(fallbackKey)}` : base; diff --git a/packages/web/src/pages/QRCodesPage.tsx b/packages/web/src/pages/QRCodesPage.tsx index 958f24d..e25281b 100644 --- a/packages/web/src/pages/QRCodesPage.tsx +++ b/packages/web/src/pages/QRCodesPage.tsx @@ -33,6 +33,7 @@ import { useTranslation } from '../lib/i18n'; import { QR_CAPTION_DEFAULTS, type QrCaptionOptions, + type QrLinkIdentity, deriveFallbackKey, qrCaptionLines, qrPngBlob, @@ -54,6 +55,13 @@ import { cn } from '../lib/utils'; interface QRCodeRecord { id: string; + /** + * What goes in the printed URL. Absent on responses from an older API, null on + * a row the backfill has not reached — qrTargetUrl falls back to `id` in both + * cases. Never shown on its own: the id is an implementation detail, and the + * only identifier a user needs to see is the scan URL. + */ + shortCode?: string | null; projectId: string; name: string; medium: string; @@ -108,12 +116,14 @@ function useQRDataUrl(text: string | null) { * announcing the image again would just be noise. */ function QRThumbnail({ - qrId, + qr: { id, shortCode }, fallbackKey, -}: { qrId: string; fallbackKey: string }) { +}: { qr: QrLinkIdentity; fallbackKey: string }) { + // The thumbnail has to encode the same URL as the dialog and the download, or + // it would be a preview of a different QR code than the one being printed. const url = useMemo( - () => qrTargetUrl(qrId, fallbackKey), - [qrId, fallbackKey], + () => qrTargetUrl({ id, shortCode }, fallbackKey), + [id, shortCode, fallbackKey], ); const { dataUrl } = useQRDataUrl(url); return ( @@ -189,8 +199,8 @@ function QRDialog({ const { t } = useTranslation(); const toast = useToast(); const url = useMemo( - () => qrTargetUrl(qr.id, fallbackKey), - [qr.id, fallbackKey], + () => qrTargetUrl(qr, fallbackKey), + [qr.id, qr.shortCode, fallbackKey], ); const { dataUrl, failed } = useQRDataUrl(url); const [isDownloading, setIsDownloading] = useState(false); @@ -624,10 +634,7 @@ function QRCodesContent() { className="p-4 transition-colors hover:bg-muted/30" >
- +

From f9ca912080cc9cf3732b1bddfa4db55f4498d9b7 Mon Sep 17 00:00:00 2001 From: kamijo-haruto Date: Wed, 29 Jul 2026 20:29:30 +0900 Subject: [PATCH 3/5] =?UTF-8?q?[fix]=20=E3=83=95=E3=82=A9=E3=83=BC?= =?UTF-8?q?=E3=83=AB=E3=83=90=E3=83=83=E3=82=AF=E7=94=A8=E3=82=AD=E3=83=BC?= =?UTF-8?q?=E3=83=AF=E3=83=BC=E3=83=89=E3=81=AE=E6=A4=9C=E8=A8=BC=E3=82=92?= =?UTF-8?q?=E6=96=87=E5=AD=97=E7=A8=AE=E3=81=8B=E3=82=89=E8=A8=AD=E5=AE=9A?= =?UTF-8?q?=E3=81=B8=E3=81=AE=E7=99=BB=E9=8C=B2=E6=9C=89=E7=84=A1=E3=81=AB?= =?UTF-8?q?=E5=A4=89=E3=81=88=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit キーワードは FALLBACK_DESTINATIONS から引いたプルダウンで選ぶ形になったので、 どの文字を使っているかは何も決めない。設定に登録があるかどうかが全てを決める。 設定にないキーワードはD1が落ちる当日まで何も起きていないように見えるため、 気づくのが最悪のタイミングになる。 文字種の正規表現を撤去し、parseFallbackMap への登録有無を検証する。長さ上限 (40文字)はスキーマに残す。列と印刷されるペイロードを縛るのはこちらの役目。 登録の必須化には例外を1つ設けた。保存済みの値と同じであれば通す。これが無いと、 設定からキーワードが消えたプロジェクトは保存自体ができなくなり、誰も触っていない フィールドを理由に改名が400で落ちる。そのキーワードは既にポスターに印刷されて いるため、管理画面も同じ理由で孤立したキーを表示したまま保持している。作成時は 保存済みの値が無いので登録が必須。 登録判定は Object.keys(map).includes(next) を使う。next in map だと constructor や toString がプロトタイプチェーン経由で「登録済み」と誤判定される。 クライアント側も同じ規則にする。転送先リストの取得に失敗すると入力欄がテキスト 入力に退化してリストを知り得ないので、そのときは一致検証を飛ばして長さだけ見る。 編集画面では保存済みの値を渡す。渡さないと、サーバーが許す改名をクライアントが 送信前に弾いてしまう。 ASCIIに寄せる根拠(日本語キーワードは1文字9文字分にパーセントエンコードされ、 シンボルが 57×57 から 61×61 に育つ)は消えるのではなく、設定を書く人の責任に 移る。wrangler.jsonc と README に記録した。READMEの手順も逆順に直した。 「プロジェクトを作ってから設定に追記」では400になる。 Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 4 +- packages/api/src/errors.ts | 6 + packages/api/src/routes/projects.test.ts | 119 ++++++++++++++++++ packages/api/src/routes/projects.ts | 99 ++++++++++++--- packages/api/wrangler.jsonc | 10 ++ packages/web/src/hooks/useFieldErrors.test.ts | 87 ++++++------- packages/web/src/hooks/useFieldErrors.ts | 33 +++-- packages/web/src/lib/api.ts | 1 + packages/web/src/lib/i18n.tsx | 8 +- packages/web/src/lib/qr.test.ts | 12 +- packages/web/src/pages/CreateProjectPage.tsx | 10 +- packages/web/src/pages/ManageProjectsPage.tsx | 13 +- 12 files changed, 320 insertions(+), 82 deletions(-) create mode 100644 packages/api/src/routes/projects.test.ts diff --git a/README.md b/README.md index d916193..896afef 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,9 @@ pnpm --filter @tracking-link/web build } ``` -キーワードはプロジェクトごとに管理画面で設定します(転送先URLから自動で提案されます)。**プロジェクトを作ったら、ここにも追記してください。** 忘れても壊れはせず `FALLBACK_URL` に落ちるだけで、平常運転中に `scan_fallback_not_configured` がWorkers Logsに出るので、障害が起きる前に気づけます。 +キーワードはプロジェクトごとに管理画面で設定しますが、**選べるのはここに書いたキーワードだけです**(APIも設定にないキーワードを400で拒否します)。ここに未登録のキーワードは、D1が落ちた当日まで何も起きていないように見えてしまうためです。順序としては**先にここへ追記し、そのあとで管理画面から選んでください。** なおキーワードを設定しないプロジェクトは `FALLBACK_URL` に落ちるだけで壊れはせず、平常運転中に `scan_fallback_not_configured` がWorkers Logsに出るので、障害が起きる前に気づけます。 + +キーワードは短い半角英数字にしてください。印刷するQRコードに入るため、日本語のキーワードは1文字9文字分にパーセントエンコードされ、シンボルが 57×57 から 61×61 モジュールに大きくなります(この判断はここを書く人の責任です。APIは文字種を見ません)。 転送先URLそのものではなくキーワードを埋めているのは、QRから復元できるURLはQRに入っていなければならず、シンボルが 53×53 から 69×69 まで大きくなるためです(ハッシュは一方向で復元できず、暗号化は平文より長くなるので、暗号技術では回避できません)。キーワードなら +4モジュールで済み、副次的に`&p=`が設定済みの一覧からしか選べないため**オープンリダイレクトが構造的に不可能**になります。 diff --git a/packages/api/src/errors.ts b/packages/api/src/errors.ts index 2c96a46..adb7964 100644 --- a/packages/api/src/errors.ts +++ b/packages/api/src/errors.ts @@ -21,6 +21,10 @@ export const ErrorCodes = { PERMISSION_REQUIRED: 'PERMISSION_REQUIRED', NOT_OWNER: 'NOT_OWNER', INVALID_BODY: 'INVALID_BODY', + // Distinct from INVALID_BODY so the form can name the keyword and list the + // configured ones, instead of reporting a generic bad body for a field the user + // picked from a dropdown. + FALLBACK_KEY_NOT_CONFIGURED: 'FALLBACK_KEY_NOT_CONFIGURED', NO_FIELDS_TO_UPDATE: 'NO_FIELDS_TO_UPDATE', PROJECT_NOT_FOUND: 'PROJECT_NOT_FOUND', QR_CODE_NOT_FOUND: 'QR_CODE_NOT_FOUND', @@ -46,6 +50,8 @@ const FALLBACK_MESSAGES: Record = { PERMISSION_REQUIRED: 'Insufficient permissions', NOT_OWNER: 'You can only modify items you created', INVALID_BODY: 'Invalid request body', + FALLBACK_KEY_NOT_CONFIGURED: + 'That fallback keyword is not in FALLBACK_DESTINATIONS', NO_FIELDS_TO_UPDATE: 'No fields to update', PROJECT_NOT_FOUND: 'Project not found', QR_CODE_NOT_FOUND: 'QR code not found', diff --git a/packages/api/src/routes/projects.test.ts b/packages/api/src/routes/projects.test.ts new file mode 100644 index 0000000..90b6c97 --- /dev/null +++ b/packages/api/src/routes/projects.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest'; +import { parseFallbackMap } from '../fallback'; +import { + createProjectBodySchema, + isFallbackKeyAllowed, + updateProjectBodySchema, +} from './projects'; + +/** + * What a project body has to satisfy before it reaches D1. + * + * The handlers themselves need real D1 and a request context, so they are + * exercised through `wrangler dev --local`. These two pieces are pure, and they + * are the pair that is easy to get wrong in opposite directions: the schema used + * to enforce a character set that said nothing about whether a keyword actually + * resolves, and the membership check has to stay lenient enough that a keyword + * already printed on posters can still be saved after someone removes it from the + * configuration. + */ + +// Includes a Japanese keyword, which the old character-set rule refused outright, +// and the underscore handle the X account uses. +const CONFIGURED = parseFallbackMap({ + instagram: 'https://www.instagram.com/nutfes', + web: 'https://www.nutfes.net/', + nut_fes: 'https://x.com/nut_fes', + インスタ: 'https://www.instagram.com/nutfes', +}); + +const VALID_BODY = { + projectName: '造形大祭2026', + destinationUrl: 'https://www.nutfes.net/', +}; + +/** Mirrors the create handler: parse the body, then check the keyword. */ +function createAccepts(fallbackKey?: string): boolean { + const parsed = createProjectBodySchema.safeParse({ + ...VALID_BODY, + ...(fallbackKey === undefined ? {} : { fallbackKey }), + }); + if (!parsed.success) return false; + return isFallbackKeyAllowed(parsed.data.fallbackKey ?? '', CONFIGURED); +} + +/** Mirrors the update handler, including the stored value it compares against. */ +function updateAccepts(fallbackKey: string, stored: string): boolean { + const parsed = updateProjectBodySchema.safeParse({ fallbackKey }); + if (!parsed.success) return false; + return isFallbackKeyAllowed( + parsed.data.fallbackKey ?? '', + CONFIGURED, + stored, + ); +} + +describe('project fallbackKey validation', () => { + it('accepts a keyword that is in the configuration', () => { + expect(createAccepts('instagram')).toBe(true); + expect(createAccepts('nut_fes')).toBe(true); + }); + + it('accepts a blank keyword, which means the site-wide static fallback', () => { + expect(createAccepts('')).toBe(true); + // Omitted entirely is the same thing: the write path stores ''. + expect(createAccepts(undefined)).toBe(true); + }); + + it('rejects a keyword that is not in the configuration', () => { + // Character-set-legal and completely inert: it would resolve to nothing on + // the one day the fallback is needed. + expect(createAccepts('not-configured')).toBe(false); + }); + + it('accepts a non-ASCII keyword once it is configured', () => { + // This is the behaviour change. The old rule rejected it on principle; what + // matters is that FALLBACK_DESTINATIONS has an entry for it. The cost of a + // keyword like this to the printed symbol is now the config author's call — + // see the note in wrangler.jsonc. + expect(createAccepts('インスタ')).toBe(true); + }); + + it('still rejects a keyword longer than the column allows', () => { + // The one rule that stayed in the schema, because it bounds the row and the + // printed payload rather than describing what a keyword means. + const tooLong = createProjectBodySchema.safeParse({ + ...VALID_BODY, + fallbackKey: 'a'.repeat(41), + }); + expect(tooLong.success).toBe(false); + const atLimit = createProjectBodySchema.safeParse({ + ...VALID_BODY, + fallbackKey: 'a'.repeat(40), + }); + expect(atLimit.success).toBe(true); + }); + + it('accepts an unchanged stored keyword that is no longer configured', () => { + // The regression this exemption exists for: the keyword is already printed on + // posters, so an edit to the project name must not be refused because of a + // field the user never touched. + expect(updateAccepts('gone', 'gone')).toBe(true); + }); + + it('rejects changing an orphaned keyword to another unconfigured one', () => { + expect(updateAccepts('also-gone', 'gone')).toBe(false); + }); + + it('lets an orphaned keyword be cleared or moved to a configured one', () => { + expect(updateAccepts('', 'gone')).toBe(true); + expect(updateAccepts('web', 'gone')).toBe(true); + }); + + it('does not treat prototype property names as configured', () => { + // `'constructor' in map` is true for any object literal, which would wave + // through a keyword resolving to nothing. + expect(isFallbackKeyAllowed('constructor', CONFIGURED)).toBe(false); + expect(isFallbackKeyAllowed('toString', CONFIGURED)).toBe(false); + }); +}); diff --git a/packages/api/src/routes/projects.ts b/packages/api/src/routes/projects.ts index 1906a25..49a0e62 100644 --- a/packages/api/src/routes/projects.ts +++ b/packages/api/src/routes/projects.ts @@ -49,33 +49,72 @@ const httpUrl = z * Keyword baked into this project's QR codes as `&p=`, resolved against the * FALLBACK_DESTINATIONS var when D1 is unreachable (see src/fallback.ts). * - * ASCII-only on purpose: a Japanese keyword is percent-encoded at 9 characters per - * character, which grows the printed symbol from 57x57 to 61x61 modules. Optional - * — '' means the admin UI derives one from the destination host instead. + * No character rule here on purpose. Which characters a keyword uses decides + * nothing — whether it has an entry in that var decides everything, because a + * keyword the config does not know is inert until D1 goes down, which is the + * worst possible moment to find out. That check is `rejectUnconfiguredFallbackKey` + * below, and it needs the env, so it cannot live in a static schema. Length still + * belongs here: it bounds the column and the printed payload. + * + * Optional; '' means scans use the site-wide static fallback instead. */ const FALLBACK_KEY_MAX = 40; -const fallbackKey = z - .string() - .max(FALLBACK_KEY_MAX) - .regex( - // Underscore is allowed because social handles use it — the X account is - // x.com/nut_fes — and it costs nothing to carry: `_` is unreserved in RFC - // 3986, so encodeURIComponent leaves it alone, and the QR payload is - // already in byte mode (lowercase letters are absent from QR's - // alphanumeric set), so it is the same 8 bits as any other character. - // Hyphen stays last in the class: `[a-z0-9-_]` would read `9-_` as a range - // and quietly admit uppercase and punctuation. - /^$|^[a-z0-9][a-z0-9_-]*$/, - 'Use lowercase letters, digits, hyphens and underscores only', - ); +const fallbackKey = z.string().max(FALLBACK_KEY_MAX); + +/** + * Whether `next` may be stored as a project's fallback keyword. + * + * `stored` is the row's current keyword, and passing it exempts a value that is + * not being changed. Without that exemption a project whose keyword was later + * dropped from FALLBACK_DESTINATIONS could not be saved at all: renaming it would + * 400 on a field nobody touched, and that keyword is already printed on posters — + * the admin UI keeps such an orphan visible and selected for the same reason. + * Create has no stored value, so membership is required there. + */ +export function isFallbackKeyAllowed( + next: string, + map: Record, + stored?: string, +): boolean { + // '' is "no keyword", which needs no entry — see resolveFallbackUrl. + if (next === '') return true; + if (stored !== undefined && next === stored) return true; + // Object.keys rather than `next in map`, so a keyword of 'constructor' or + // 'toString' is not waved through by the prototype chain. + return Object.keys(map).includes(next); +} + +/** + * 400 when the keyword is not one an operator configured, or null to proceed. + * + * `meta` carries the offending keyword and the configured list so the form can + * name both rather than saying only that something was invalid. + */ +function rejectUnconfiguredFallbackKey( + c: Context, + next: string, + stored?: string, +) { + const map = parseFallbackMap(c.env.FALLBACK_DESTINATIONS); + if (isFallbackKeyAllowed(next, map, stored)) return null; + return fail(c, 400, ErrorCodes.FALLBACK_KEY_NOT_CONFIGURED, { + fields: ['fallbackKey'], + meta: { + fallbackKey: next, + configuredKeys: Object.keys(map).sort((a, b) => a.localeCompare(b)), + }, + }); +} -const createProjectBodySchema = z.object({ +// Exported alongside isFallbackKeyAllowed so the accepted-body rules can be +// tested without a Worker: everything else about these handlers needs real D1. +export const createProjectBodySchema = z.object({ projectName: z.string().min(1, 'Project name is required').max(NAME_MAX), destinationUrl: httpUrl, fallbackKey: fallbackKey.optional(), }); -const updateProjectBodySchema = z.object({ +export const updateProjectBodySchema = z.object({ projectName: z.string().min(1).max(NAME_MAX).optional(), destinationUrl: httpUrl.optional(), fallbackKey: fallbackKey.optional(), @@ -397,6 +436,10 @@ projectsApp.post('/', async (c) => { // Passed explicitly rather than relying on the column default, because Drizzle // deliberately keeps the field required — see the note on schema.projects. const key = parsed.data.fallbackKey ?? ''; + // No stored value to fall back on at creation, so the keyword has to be one the + // config knows. + const unconfigured = rejectUnconfiguredFallbackKey(c, key); + if (unconfigured) return unconfigured; const db = getDb(c.env.DB); await db.insert(schema.projects).values({ @@ -486,6 +529,24 @@ projectsApp.put('/:id', async (c) => { } const db = getDb(c.env.DB); + if (values.fallbackKey !== undefined) { + // One extra read, and only when the keyword is in the body — the stored value + // is what lets an already-printed orphan be saved again unchanged. A + // name-only edit never pays for this. + const stored = await db + .select({ fallbackKey: schema.projects.fallbackKey }) + .from(schema.projects) + .where(eq(schema.projects.projectId, projectId)) + .get(); + if (!stored) return fail(c, 404, ErrorCodes.PROJECT_NOT_FOUND); + const unconfigured = rejectUnconfiguredFallbackKey( + c, + values.fallbackKey, + stored.fallbackKey, + ); + if (unconfigured) return unconfigured; + } + const result = await db .update(schema.projects) .set(values) diff --git a/packages/api/wrangler.jsonc b/packages/api/wrangler.jsonc index 1b04929..97e6127 100644 --- a/packages/api/wrangler.jsonc +++ b/packages/api/wrangler.jsonc @@ -59,6 +59,16 @@ // scan falls through to FALLBACK_URL above, and `scan_fallback_not_configured` // shows up in Workers Logs during normal operation so you find out before an // outage rather than during one. + // + // This list is the only thing that decides which keywords a project may be + // given: the API accepts exactly these (plus none at all), and the admin UI + // offers exactly these. So keeping a keyword short and ASCII is the job of + // whoever writes it here — it is printed into every QR code for the project, + // and a Japanese keyword is percent-encoded at 9 characters per character, + // which grows the printed symbol from 57x57 to 61x61 modules. Underscore is + // free by that measure: it is unreserved in RFC 3986, so it is never + // percent-encoded. Max 40 characters — longer keys are dropped when this is + // parsed, so a project can never be assigned one. "FALLBACK_DESTINATIONS": { "instagram": "https://www.instagram.com/nutfes?igsh=MTB1eXhpcWJ4YnQ0cA==", "web": "https://www.nutfes.net/", diff --git a/packages/web/src/hooks/useFieldErrors.test.ts b/packages/web/src/hooks/useFieldErrors.test.ts index fdcda96..ee507bc 100644 --- a/packages/web/src/hooks/useFieldErrors.test.ts +++ b/packages/web/src/hooks/useFieldErrors.test.ts @@ -2,63 +2,64 @@ import { describe, expect, it } from 'vitest'; import { validateFallbackKey, validateHttpUrl } from './useFieldErrors'; /** - * The fallback keyword is printed into QR codes, so the set of characters - * accepted here has to match the API's exactly — a key the form accepts but the - * API rejects is a save that fails after the user has already decided. + * The rule has to match the API's exactly — a keyword the form accepts but the + * API rejects is a save that fails after the user has already decided. What the + * API checks is membership of FALLBACK_DESTINATIONS, not the characters used: a + * well-formed keyword the Worker was never told about resolves to nothing on the + * one day the fallback matters. */ describe('validateFallbackKey', () => { - const ok = (value: string) => expect(validateFallbackKey(value)).toBeNull(); - const rejected = (value: string) => - expect(validateFallbackKey(value)).toBe('validation.fallbackKey'); + // Mirrors wrangler.jsonc, plus a Japanese keyword — configured is configured, + // and the old character-set rule would have refused this one outright. + const configured = ['instagram', 'web', 'x', 'nut_fes', 'インスタ']; - it('accepts underscores, which social handles need', () => { - // The X account is x.com/nut_fes. - ok('nut_fes'); - ok('a_b_c'); - }); + const ok = ( + value: string, + options?: Parameters[2], + ) => expect(validateFallbackKey(value, configured, options)).toBeNull(); + const rejected = ( + value: string, + options?: Parameters[2], + ) => + expect(validateFallbackKey(value, configured, options)).toBe( + 'validation.fallbackKey', + ); - it('accepts the existing keywords', () => { + it('accepts a keyword that is in the configuration', () => { ok('instagram'); ok('web'); - ok('x'); - ok('my-shop'); - ok('a1'); + // The X account is x.com/nut_fes, so underscores still have to pass. + ok('nut_fes'); + ok('インスタ'); }); - // The character class must be written [a-z0-9_-] with the hyphen last. - // [a-z0-9-_] parses `9-_` as a range spanning ':' through '_', which would - // wave through uppercase and punctuation. - it('still rejects everything outside the class', () => { - for (const value of [ - 'A', - 'Insta', - 'a b', - 'a.b', - 'a:b', - 'a@b', - 'a^b', - '@', - ]) { - rejected(value); - } + it('rejects a keyword that is not in the configuration', () => { + // Every one of these would have passed the old character-set rule and then + // silently done nothing during an outage. + rejected('my-shop'); + rejected('a1'); + rejected('insta'); + }); + + it('accepts an empty value, which means the site-wide fallback', () => { + ok(''); }); - it('requires an alphanumeric first character', () => { - rejected('_leading'); - rejected('-leading'); - // Trailing is fine — only the first character is constrained. - ok('trailing_'); - ok('trailing-'); + it('skips the check when the list could not be loaded', () => { + // The field degrades to free text in that state, so there is nothing to + // compare against; the server is the real gate. + ok('anything-at-all', { listUnavailable: true }); + ok('インスタ', { listUnavailable: true }); }); - it('rejects an empty value', () => { - // The field is optional, but blank is handled by not validating at all; - // reaching here with '' means a required-field rule was expected. - rejected(''); + it('accepts an unchanged stored keyword that is no longer configured', () => { + // It is already printed on posters, and refusing it would block an edit to an + // unrelated field on that project. + ok('gone', { storedKey: 'gone' }); }); - it('rejects non-ASCII, which would inflate the printed QR', () => { - rejected('インスタ'); + it('rejects changing an orphaned keyword to another unconfigured one', () => { + rejected('also-gone', { storedKey: 'gone' }); }); }); diff --git a/packages/web/src/hooks/useFieldErrors.ts b/packages/web/src/hooks/useFieldErrors.ts index 7d2e20d..755ad9e 100644 --- a/packages/web/src/hooks/useFieldErrors.ts +++ b/packages/web/src/hooks/useFieldErrors.ts @@ -72,19 +72,30 @@ export function useFieldErrors() { } /** - * Mirrors the API's fallback-key rule (`^[a-z0-9][a-z0-9_-]*$`). + * Mirrors the API's fallback-key rule: the keyword has to be one the Worker was + * configured with. * - * ASCII-only is not arbitrary: the keyword is percent-encoded into the printed QR - * payload, and a Japanese character costs 9 bytes there — enough to grow the - * symbol from 57x57 to 61x61 modules. Underscore is fine by that measure (it is - * unreserved in RFC 3986, so it is never percent-encoded) and social handles - * need it. - * - * Keep the hyphen last in the character class — `[a-z0-9-_]` parses `9-_` as a - * range and would accept uppercase and punctuation. + * Which characters it uses is not the question — a perfectly spelled keyword that + * FALLBACK_DESTINATIONS does not list resolves to nothing on the one day the + * fallback is needed, so membership is the only check worth making. */ -export function validateFallbackKey(value: string): string | null { - return /^[a-z0-9][a-z0-9_-]*$/.test(value) ? null : 'validation.fallbackKey'; +export function validateFallbackKey( + value: string, + configuredKeys: string[], + options: { listUnavailable?: boolean; storedKey?: string } = {}, +): string | null { + // Blank is "no keyword", which needs no entry; scans use the site-wide fallback. + if (value === '') return null; + // The picker degrades to a free-text input when the list could not be fetched. + // With no list to compare against, anything we did here would reject valid + // keywords, so length is the only client-side rule and the server is the gate. + if (options.listUnavailable) return null; + // An orphaned keyword stays saveable when it is not being changed, matching the + // API's exemption: it is already printed on posters, and blocking the save would + // strand the whole row over a field the user did not touch. + if (options.storedKey !== undefined && value === options.storedKey) + return null; + return configuredKeys.includes(value) ? null : 'validation.fallbackKey'; } /** Shared http(s) check, mirroring the API's protocol allow-list. */ diff --git a/packages/web/src/lib/api.ts b/packages/web/src/lib/api.ts index da2fece..59d0387 100644 --- a/packages/web/src/lib/api.ts +++ b/packages/web/src/lib/api.ts @@ -159,6 +159,7 @@ const CODE_TO_KEY: Record = { PERMISSION_REQUIRED: 'error.permission', NOT_OWNER: 'error.notOwner', INVALID_BODY: 'error.invalidBody', + FALLBACK_KEY_NOT_CONFIGURED: 'error.fallbackKeyNotConfigured', NO_FIELDS_TO_UPDATE: 'error.noFieldsToUpdate', PROJECT_NOT_FOUND: 'error.projectNotFound', QR_CODE_NOT_FOUND: 'error.qrNotFound', diff --git a/packages/web/src/lib/i18n.tsx b/packages/web/src/lib/i18n.tsx index 80be709..535ad70 100644 --- a/packages/web/src/lib/i18n.tsx +++ b/packages/web/src/lib/i18n.tsx @@ -60,6 +60,8 @@ const en: Dictionary = { 'That QR code no longer exists. It may have been deleted.', 'error.duplicateName': 'A QR code with this name already exists in this project.', + 'error.fallbackKeyNotConfigured': + '“{fallbackKey}” is not in the Worker configuration, so it cannot be saved. Pick a configured keyword, or leave it blank.', 'error.shortCodeUnavailable': 'Could not allocate a short code for this QR code. Please try again.', 'error.tooManyRows': @@ -71,7 +73,7 @@ const en: Dictionary = { 'validation.tooLong': 'Please use {max} characters or fewer.', 'validation.url': 'Enter a URL starting with http:// or https://', 'validation.fallbackKey': - 'Use lowercase letters, digits, hyphens and underscores only, starting with a letter or digit.', + 'That keyword is not in the Worker configuration. Pick one from the list, or leave it blank to use the site-wide fallback.', 'login.sessionExpired': 'Your session expired. Please sign in again.', 'login.retryAfterNetwork': @@ -232,6 +234,8 @@ const ja: Dictionary = { 'error.qrNotFound': 'このQRコードは存在しません。削除された可能性があります。', 'error.duplicateName': 'この名前のQRコードはこのプロジェクトに既にあります。', + 'error.fallbackKeyNotConfigured': + '「{fallbackKey}」はWorkerの設定にないため保存できません。設定済みのキーワードを選ぶか、空欄にしてください。', 'error.shortCodeUnavailable': '短縮コードを発行できませんでした。もう一度お試しください。', 'error.tooManyRows': @@ -244,7 +248,7 @@ const ja: Dictionary = { 'validation.tooLong': '{max}文字以内で入力してください。', 'validation.url': 'http:// または https:// で始まるURLを入力してください。', 'validation.fallbackKey': - '半角の英小文字・数字・ハイフン・アンダーバーのみ、先頭は英数字で入力してください。', + 'このキーワードはWorkerの設定にありません。一覧から選ぶか、空欄にして全体のフォールバック先を使ってください。', 'login.sessionExpired': 'セッションの有効期限が切れました。再度ログインしてください。', diff --git a/packages/web/src/lib/qr.test.ts b/packages/web/src/lib/qr.test.ts index 06da3bb..a2bb6ef 100644 --- a/packages/web/src/lib/qr.test.ts +++ b/packages/web/src/lib/qr.test.ts @@ -25,9 +25,12 @@ describe('deriveFallbackKey', () => { expect(deriveFallbackKey('https://example.co.jp/path')).toBe('example'); }); - it('lowercases and strips characters the API would reject', () => { - // The API accepts ^[a-z0-9][a-z0-9_-]*$, so a suggestion that fails - // validation would be a self-inflicted form error. + it('lowercases and narrows the host label to a plausible keyword', () => { + // The API no longer restricts which characters a keyword may use — it checks + // membership in FALLBACK_DESTINATIONS instead. This narrowing stays because + // the output is only a *suggestion* matched against that list, and a + // suggestion carrying the host's punctuation would never match an entry + // anyone wrote by hand. expect(deriveFallbackKey('https://WWW.Insta_Gram.com/')).toBe('insta_gram'); expect(deriveFallbackKey('https://my-shop.example.com/')).toBe('my-shop'); expect(deriveFallbackKey('https://Insta!Gram.com/')).toBe('instagram'); @@ -40,7 +43,8 @@ describe('deriveFallbackKey', () => { }); it('does not leave a leading hyphen or underscore', () => { - // The rule requires an alphanumeric first character. + // Cosmetic now rather than required: a suggestion starting with punctuation + // reads like a mistake in the picker even though the API would accept it. expect(deriveFallbackKey('https://_foo.example.com/')).toBe('foo'); expect(deriveFallbackKey('https://-foo.example.com/')).toBe('foo'); }); diff --git a/packages/web/src/pages/CreateProjectPage.tsx b/packages/web/src/pages/CreateProjectPage.tsx index f31d884..4fe49e4 100644 --- a/packages/web/src/pages/CreateProjectPage.tsx +++ b/packages/web/src/pages/CreateProjectPage.tsx @@ -81,7 +81,15 @@ function CreateProjectForm() { id: 'fallbackKey', value: fallbackKey, maxLength: FALLBACK_KEY_MAX, - validate: validateFallbackKey, + // A new project has no stored keyword to be grandfathered in, so the + // value has to be one the Worker knows — unless the list never loaded, + // in which case only the server can tell. + validate: (value) => + validateFallbackKey( + value, + fallback.destinations.map((d) => d.key), + { listUnavailable: fallback.failed }, + ), }, }); if (!ok) return; diff --git a/packages/web/src/pages/ManageProjectsPage.tsx b/packages/web/src/pages/ManageProjectsPage.tsx index 985ed28..21b261f 100644 --- a/packages/web/src/pages/ManageProjectsPage.tsx +++ b/packages/web/src/pages/ManageProjectsPage.tsx @@ -251,7 +251,18 @@ function ManageProjectsContent() { id: 'editProjectFallbackKey', value: editFallbackKey, maxLength: FALLBACK_KEY_MAX, - validate: validateFallbackKey, + // The stored keyword is passed so an orphan the form is showing can be + // saved back unchanged — the API exempts it for the same reason, and + // without this an unrelated rename would be blocked here instead. + validate: (value) => + validateFallbackKey( + value, + fallback.destinations.map((d) => d.key), + { + listUnavailable: fallback.failed, + storedKey: editing.fallbackKey, + }, + ), }, }); if (!ok) return; From 9d68ab93f1710d5664a9e71ed13abfb901cf3b0c Mon Sep 17 00:00:00 2001 From: kamijo-haruto Date: Wed, 29 Jul 2026 23:08:13 +0900 Subject: [PATCH 4/5] =?UTF-8?q?[feat]=20=E8=BB=A2=E9=80=81=E5=85=88URL?= =?UTF-8?q?=E3=81=AB=E3=82=B3=E3=83=94=E3=83=BC=E3=83=9C=E3=82=BF=E3=83=B3?= =?UTF-8?q?=E3=82=92=E4=BB=98=E3=81=91=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit このURLは画面の外へ持ち出す唯一の情報で、同僚にポスターの確認を頼むチャットや、 リダイレクトを試すスマホに貼られる。折り返された70文字の等幅文字列を手で選択 するのがその失敗点で、途中まで選択されたURLは「QRが壊れている」ように見える。 押した状態は2秒だけアイコンとラベルに出す。アイコンだけだとスクリーンリーダー 利用者に何も起きたことが伝わらないため、aria-label と title の両方を状態に応じて 切り替える。タイマーはアンマウント時に解除する。閉じたダイアログに向けて発火 させないため。 navigator.clipboard は secure context を要求する。http://localhost は満たすが 平文httpのLANアドレスは満たさないので、失敗経路は実在する。黙って成功したように 見えるのが最悪なので、手動コピーを促すトーストを出す。 Co-Authored-By: Claude Opus 5 (1M context) --- packages/web/src/lib/i18n.tsx | 8 ++++ packages/web/src/pages/QRCodesPage.tsx | 64 ++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/packages/web/src/lib/i18n.tsx b/packages/web/src/lib/i18n.tsx index 535ad70..4534fe4 100644 --- a/packages/web/src/lib/i18n.tsx +++ b/packages/web/src/lib/i18n.tsx @@ -176,6 +176,10 @@ const en: Dictionary = { 'qrCodes.dialogTitle': 'QR code', 'qrCodes.imageAlt': 'QR code for {name}', 'qrCodes.scanUrlLabel': 'This code links to', + 'qrCodes.copyUrl': 'Copy this link', + 'qrCodes.copied': 'Copied', + 'qrCodes.copyFailed': + 'Could not copy. Copying needs an https or localhost address — select the link and copy it by hand.', 'qrCodes.generateFailed': 'Failed to generate the QR code', 'qrCodes.downloadButton': 'Download PNG', 'qrCodes.scanCount': '{count} scans', @@ -352,6 +356,10 @@ const ja: Dictionary = { 'qrCodes.dialogTitle': 'QRコード', 'qrCodes.imageAlt': '「{name}」のQRコード', 'qrCodes.scanUrlLabel': 'このコードの転送先', + 'qrCodes.copyUrl': 'このリンクをコピー', + 'qrCodes.copied': 'コピーしました', + 'qrCodes.copyFailed': + 'コピーできませんでした。コピーには https か localhost のアドレスが必要です。リンクを選択して手動でコピーしてください。', 'qrCodes.generateFailed': 'QRコードの生成に失敗しました', 'qrCodes.downloadButton': 'PNGをダウンロード', 'qrCodes.scanCount': '{count} 回', diff --git a/packages/web/src/pages/QRCodesPage.tsx b/packages/web/src/pages/QRCodesPage.tsx index e25281b..6e20d18 100644 --- a/packages/web/src/pages/QRCodesPage.tsx +++ b/packages/web/src/pages/QRCodesPage.tsx @@ -1,5 +1,7 @@ import { ArrowLeft, + Check, + Copy, Download, Loader2, Pencil, @@ -320,15 +322,71 @@ function QRDialog({

{t('qrCodes.scanUrlLabel')}

-

- {url} -

+
+

+ {url} +

+ +
); } +/** + * Copies `value` and says so. + * + * The URL is the one thing on this screen someone needs elsewhere — pasted into a + * chat to ask a colleague to test a poster, or into a phone to check the redirect. + * Selecting a wrapped 70-character monospace string by hand is where that goes + * wrong, and a half-selected URL fails in a way that looks like a broken QR code. + * + * navigator.clipboard needs a secure context, which http://localhost satisfies but + * a plain-http LAN address does not — so the failure path is real, not defensive + * padding, and it has to say something rather than appear to have worked. + */ +function CopyButton({ value }: { value: string }) { + const { t } = useTranslation(); + const toast = useToast(); + const [copied, setCopied] = useState(false); + + // Cleared on unmount so the tick cannot fire into a closed dialog. + useEffect(() => { + if (!copied) return; + const timer = window.setTimeout(() => setCopied(false), 2000); + return () => window.clearTimeout(timer); + }, [copied]); + + const handleCopy = async () => { + try { + if (!navigator.clipboard) throw new Error('Clipboard API unavailable'); + await navigator.clipboard.writeText(value); + setCopied(true); + } catch { + toast.error(t('qrCodes.copyFailed')); + } + }; + + return ( + + ); +} + /** * A caption checkbox that shows the text it would actually print. * From 820ab8bd8b740ee2014a7f2ce049b9f5d6762912 Mon Sep 17 00:00:00 2001 From: kamijo-haruto Date: Wed, 29 Jul 2026 23:08:31 +0900 Subject: [PATCH 5/5] =?UTF-8?q?[fix]=20FALLBACK=5FDESTINATIONS=20=E3=81=AE?= =?UTF-8?q?=E3=82=AD=E3=83=BC=E3=82=92=E5=B0=8F=E6=96=87=E5=AD=97=E3=81=AB?= =?UTF-8?q?=E7=B5=B1=E4=B8=80=E3=81=97=E3=80=81=E5=AE=9F=E9=9A=9B=E3=81=AE?= =?UTF-8?q?=E8=BB=A2=E9=80=81=E5=85=88=E3=82=92=E5=8F=8D=E6=98=A0=E3=81=99?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 本番の設定に Bingo / TikTok / "Youtube," が入っていたが、いずれも機能しない状態 だった。 照合は単純なオブジェクト参照なので大文字小文字を区別する。一方で管理画面が提案 するキーワードは遷移先ホストから導出するため必ず小文字になる。つまり Bingo という エントリは、プロジェクトに提示される bingo と永久に一致しない。しかもスキャン時に 何も報告されない。意図的な空欄とキーの取り違えを区別する手段がないため。 "Youtube," はキー名に末尾のカンマが入っていた。カンマはクエリの予約文字なので &p=Youtube%2C とエンコードが必要になり、印刷するペイロードも伸びる。 bingo / tiktok / youtube を実際の転送先とともに追加し、instagram の URL を本番で 使われている値に合わせた。nut_fes は削除した。本番の設定から消えており、どの プロジェクトも使っていない。x で足りる。 キーの変更で孤立するプロジェクトはない。本番が使っているのは "" / instagram / web / x だけであることを確認済み。 Co-Authored-By: Claude Opus 5 (1M context) --- packages/api/wrangler.jsonc | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/api/wrangler.jsonc b/packages/api/wrangler.jsonc index 97e6127..637594a 100644 --- a/packages/api/wrangler.jsonc +++ b/packages/api/wrangler.jsonc @@ -69,18 +69,21 @@ // free by that measure: it is unreserved in RFC 3986, so it is never // percent-encoded. Max 40 characters — longer keys are dropped when this is // parsed, so a project can never be assigned one. + // + // Keys are lowercase without exception. Two reasons, both of which have + // already bitten this config: the lookup is a plain object index so it is + // case-sensitive, and the admin UI's suggestion is derived from the + // destination host and therefore always lowercase — so a `Bingo` entry can + // never match the `bingo` a project would be offered, and the scan silently + // falls through to FALLBACK_URL. Nothing reports that at scan time, because + // there is no way to tell a deliberately-blank keyword from a typo. "FALLBACK_DESTINATIONS": { - "instagram": "https://www.instagram.com/nutfes?igsh=MTB1eXhpcWJ4YnQ0cA==", + "bingo": "https://bingo.nutfes.net/", + "instagram": "https://www.instagram.com/nutfes", + "tiktok": "https://www.tiktok.com/@nutfes_sns", "web": "https://www.nutfes.net/", - // X is listed under two keys on purpose. "x" is what the admin UI derives - // from the host x.com, so it is the keyword a project gets if the field is - // left blank; "nut_fes" is the handle, which is what someone naming the - // keyword by hand reaches for. An unmatched keyword would silently send - // scans to FALLBACK_URL instead — exactly the case this map exists to - // prevent — and a spare entry costs nothing. Drop one once the projects - // are set up and you know which is in use. "x": "https://x.com/nut_fes", - "nut_fes": "https://x.com/nut_fes" + "youtube": "https://youtube.com/channel/UCxpfPMcf4LCWSL1dR8ha5uQ?si=DGby9RdPrTj4b4c3" } } // Secrets (set with `wrangler secret put `, never committed):