diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index b0e1370..d116392 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -31,12 +31,11 @@ import { getSubRoomPermissions, getSubRoomSaves, getVisitedRooms, - MAX_ROOM_NAME_LENGTH, modifySubRoom, - nameRejection, publishSubRoomSave, removeCheer, removeFavorite, + roomNameRejection, saveSubRoomData, searchRooms, setRoomDescription, @@ -1074,7 +1073,7 @@ const app = new Hono() if (name === '') return roomEnvelope(c, null, 'You must enter a name for your room.') // Shape before availability, so a rejected name costs no D1 read. - const badName = nameRejection(name, 'room name', MAX_ROOM_NAME_LENGTH) + const badName = roomNameRejection(name, 'room name') if (badName !== null) return roomEnvelope(c, null, badName) if (await getRoomByName(c.env.DB, name)) { return roomEnvelope(c, null, 'A room with that name already exists!') @@ -1202,7 +1201,7 @@ const app = new Hono() } // Same ErrorId as the empty case — the client keys off it to mark the field, and // both are the name being unusable. The sentence is what tells them which. - const badName = nameRejection(name, 'room name', MAX_ROOM_NAME_LENGTH) + const badName = roomNameRejection(name, 'room name') if (badName !== null) { return roomResult(c, { Success: false, ErrorId: 'Rooms.InvalidName', Error: badName }) } @@ -2086,7 +2085,7 @@ const app = new Hono() Error: 'You must enter a name for your room!', }) } - const badName = nameRejection(name, 'subroom name', MAX_ROOM_NAME_LENGTH) + const badName = roomNameRejection(name, 'subroom name') if (badName !== null) { return roomResult(c, { Success: false, ErrorId: 'Rooms.InvalidName', Error: badName }) } @@ -2398,7 +2397,7 @@ const app = new Hono() const body = (await c.req.parseBody().catch(() => ({}))) as Record const name = typeof body.name === 'string' ? body.name.trim() : '' if (name === '') return roomEnvelope(c, null, 'You must enter a name for your subroom!') - const badName = nameRejection(name, 'subroom name', MAX_ROOM_NAME_LENGTH) + const badName = roomNameRejection(name, 'subroom name') if (badName !== null) return roomEnvelope(c, null, badName) const result = await createSubRoom(c.env.DB, roomId, accountId, name) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index a0e54dd..953a7a6 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -2657,9 +2657,10 @@ describe('rooms endpoints', () => { }) }) -// Room and subroom names are held to the same rule as usernames — letters and digits, -// at most 32 (see `nameRejection` in @repo/domain). All four routes that take a -// player-supplied name enforce it, and each keeps its OWN refusal shape: the create +// Room and subroom names take letters, digits and underscores, at most 32 (see +// `roomNameRejection` in @repo/domain — usernames are held to the narrower rule, with no +// underscore). All four routes that take a player-supplied name enforce it, and each +// keeps its OWN refusal shape: the create // paths answer the lowercase `{ success, error, value }` envelope, the two settings // routes answer `{ Success, ErrorId, Error }` with the same `Rooms.InvalidName` id they // already used for an empty name. The client keys off those, so the rule had to fit the @@ -2668,7 +2669,7 @@ describe('rooms endpoints', () => { // Names the SERVER generates are exempt on purpose — a dorm is `@'s Dorm`, // which this rule would reject. That's why the check lives in the handlers. describe('room name validation', () => { - const bad = ['My Room', 'under_score', 'punct!', 'a'.repeat(33)] + const bad = ['My Room', 'punct!', 'a'.repeat(33)] const post = async (path: string, fields: Record, sub: string) => SELF.fetch(`${ORIGIN}${path}`, { @@ -2689,7 +2690,7 @@ describe('room name validation', () => { const res = await post('/rooms/2/clone', { name }, '1') const body = (await res.json()) as { success: boolean; error: string; value: unknown } expect(body.success, name).toBe(false) - expect(body.error).toMatch(/letters and numbers|at most 32 characters/) + expect(body.error).toMatch(/letters, numbers and underscores|at most 32 characters/) expect(body.value).toBeNull() } }) @@ -2700,12 +2701,12 @@ describe('room name validation', () => { const body = (await res.json()) as { Success: boolean; ErrorId: string; Error: string } expect(body.Success, name).toBe(false) expect(body.ErrorId).toBe('Rooms.InvalidName') - expect(body.Error).toMatch(/letters and numbers|at most 32 characters/) + expect(body.Error).toMatch(/letters, numbers and underscores|at most 32 characters/) } // Unchanged: the refusals above never reached the write. const room = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as { Name: string } - expect(room.Name).not.toMatch(/[^A-Za-z0-9]/) + expect(room.Name).not.toMatch(/[^A-Za-z0-9_]/) }) it('refuses a bad name when creating or modifying a subroom', async () => { @@ -2713,7 +2714,7 @@ describe('room name validation', () => { const created = await post('/rooms/2/subrooms', { name }, '1') const env1 = (await created.json()) as { success: boolean; error: string } expect(env1.success, name).toBe(false) - expect(env1.error).toMatch(/letters and numbers|at most 32 characters/) + expect(env1.error).toMatch(/letters, numbers and underscores|at most 32 characters/) const modified = await put( '/rooms/2/subrooms/2/modify', @@ -2726,11 +2727,15 @@ describe('room name validation', () => { } }) - it('accepts a 32-character alphanumeric name', async () => { - const name = 'a'.repeat(32) - const res = await post('/rooms/2/subrooms', { name }, '1') - const body = (await res.json()) as { success: boolean; value: { SubRooms: Array<{ Name: string }> } } - expect(body.success).toBe(true) - expect(body.value.SubRooms.some((s) => s.Name === name)).toBe(true) + it('accepts a 32-character name, and an underscore where a space is refused', async () => { + for (const name of ['a'.repeat(32), 'Laser_Tag']) { + const res = await post('/rooms/2/subrooms', { name }, '1') + const body = (await res.json()) as { + success: boolean + value: { SubRooms: Array<{ Name: string }> } + } + expect(body.success, name).toBe(true) + expect(body.value.SubRooms.some((s) => s.Name === name)).toBe(true) + } }) }) diff --git a/packages/domain/src/validation.ts b/packages/domain/src/validation.ts index a9e68b1..3415899 100644 --- a/packages/domain/src/validation.ts +++ b/packages/domain/src/validation.ts @@ -4,7 +4,8 @@ import isEmail from 'isemail' * Limits on the free text a player can put into their account and their rooms. * * Shared by `accounts` and `rooms` so one rule can't drift from the other — a username - * and a room name are held to the same shape, and both are typed into the same client. + * and a room name are held to nearly the same shape (a room name also takes underscores), + * and both are typed into the same client. * * These check only what a player SUPPLIES. Names the server generates go around them: * a dorm is called `@'s Dorm` (see `rooms-db.ts`), which the name rule below @@ -93,6 +94,32 @@ export function nameRejection(value: string, label: string, max: number): string return null } +/** + * Letters, digits and underscores — `NAME_PATTERN` plus the one separator a room name is + * allowed. A room name is a label other players read in a browse tile rather than + * something typed into a sign-in box, and the underscore is how players write the space + * the rule still refuses (`Laser_Tag`). It carries none of the homoglyph or + * right-to-left risk that widening to arbitrary Unicode would. + */ +const ROOM_NAME_PATTERN = /^[A-Za-z0-9_]+$/ + +/** + * Why a player-supplied room or subroom name is unacceptable, or `null` when it's fine. + * + * Separate from `nameRejection` rather than a flag on it: usernames are held to the + * narrower rule, and the two limits differ. `label` names the thing in the returned + * sentence ('room name', 'subroom name'), which the client renders verbatim. + */ +export function roomNameRejection(value: string, label: string): string | null { + if (value.length > MAX_ROOM_NAME_LENGTH) { + return `Your ${label} can be at most ${MAX_ROOM_NAME_LENGTH} characters.` + } + if (!ROOM_NAME_PATTERN.test(value)) { + return `Your ${label} can only contain letters, numbers and underscores.` + } + return null +} + /** * Letters, digits, spaces, dashes and colons — the title charset. Wider than * `NAME_PATTERN` because an invention is a thing with a name ("Grappling Hook v2",