From 69068cdf4de6bca8596a2e8f641fbf6dc59e63c6 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:56:53 +0300 Subject: [PATCH] Refactor url form field --- .../actions/calendar.new.server.test.ts | 82 +++++++++++++++++++ app/features/calendar/routes/calendar.new.tsx | 4 +- app/form/fields.test.ts | 17 +++- app/form/fields.ts | 48 +++++------ 4 files changed, 122 insertions(+), 29 deletions(-) diff --git a/app/features/calendar/actions/calendar.new.server.test.ts b/app/features/calendar/actions/calendar.new.server.test.ts index d69ffee70..d7e02349d 100644 --- a/app/features/calendar/actions/calendar.new.server.test.ts +++ b/app/features/calendar/actions/calendar.new.server.test.ts @@ -5,6 +5,7 @@ import * as TournamentFactory from "~/db/seed/factories/TournamentFactory"; import * as TournamentOrganizationFactory from "~/db/seed/factories/TournamentOrganizationFactory"; import * as UserFactory from "~/db/seed/factories/UserFactory"; import * as CalendarRepository from "~/features/calendar/CalendarRepository.server"; +import invariant from "~/utils/invariant"; import { wrappedAction } from "~/utils/Test"; import type { calendarNewSchemaServer } from "../calendar-new-schemas.server"; import { defaultBracketsFormValues } from "../calendar-progression-form"; @@ -127,3 +128,84 @@ describe("calendar new action: editing an event with badge prizes", () => { expect(await badgePrizeIds(tournament.eventId)).toEqual([badge.id]); }); }); + +describe("calendar new action: bracket URL", () => { + beforeEach(async () => { + await UserFactory.createRegular(null, { roles: ["TOURNAMENT_ORGANIZER"] }); + }); + + const newEventFields = ( + overrides: Partial[0]>, + ) => ({ + toToolsEnabled: false, + name: "In The Zone", + description: "", + organizationId: "", + rules: "", + date: [addDays(new Date(), 7).toISOString() as never], + startTime: null, + bracketUrl: "", + discordInviteCode: "", + tags: [], + badges: [], + trophyId: null, + avatarImgId: null, + regClosesAt: "0" as const, + minMembersPerTeam: "4" as const, + maxMembersPerTeam: undefined, + toToolsMode: "TO" as const, + pool: "", + ...defaultBracketsFormValues(), + isRanked: true, + enableNoScreenToggle: true, + enableSubs: true, + autonomousSubs: true, + requireInGameNames: false, + isInvitational: false, + isTest: false, + isDraft: false, + requireSendouQParticipation: false, + ...overrides, + }); + + test.each([ + { + why: "missing", + bracketUrl: "", + expectedError: "forms:errors.bracketUrlRequired", + }, + { + why: "javascript: protocol", + bracketUrl: "javascript:alert(1)", + expectedError: "forms:errors.invalidUrl", + }, + ])("rejects bracket URL ($why)", async ({ bracketUrl, expectedError }) => { + const res = await editAction(newEventFields({ bracketUrl }), { + user: "regular", + }); + + expect(res.fieldErrors.bracketUrl).toBe(expectedError); + }); + + test("tournament with no bracket URL gets the default one", async () => { + const res = await editAction( + newEventFields({ + toToolsEnabled: true, + date: [], + startTime: addDays(new Date(), 7).toISOString() as never, + }), + { user: "regular" }, + ); + + expect(res.fieldErrors).toBeUndefined(); + + const location = + res instanceof Response ? res.headers.get("Location") : null; + invariant(location, "expected a redirect to the created event"); + + const created = await CalendarRepository.findById( + Number(location.split("/").at(-1)), + ); + expect(created?.bracketUrl).toBe("https://sendou.ink"); + }); +}); diff --git a/app/features/calendar/routes/calendar.new.tsx b/app/features/calendar/routes/calendar.new.tsx index d86c9797f..9284782a1 100644 --- a/app/features/calendar/routes/calendar.new.tsx +++ b/app/features/calendar/routes/calendar.new.tsx @@ -198,9 +198,9 @@ function useDefaultValues() { ? databaseTimestampToDate(data.eventToEdit.startTimes[0]) : getDateAtNextFullHour(new Date()) : null, - // tournaments hide this field, so seed a valid URL for the url-format validation (the action coalesces to the same default) + // tournaments hide this field, the action coalesces the empty value to the default bracketUrl: data.isAddingTournament - ? "https://sendou.ink" + ? "" : (data.eventToEdit?.bracketUrl ?? ""), discordInviteCode: baseEvent?.discordInviteCode ?? "", tags: baseEvent?.tags ?? [], diff --git a/app/form/fields.test.ts b/app/form/fields.test.ts index 692b16ce2..7f6bcbf76 100644 --- a/app/form/fields.test.ts +++ b/app/form/fields.test.ts @@ -3,7 +3,7 @@ import { describe, expect, test } from "vitest"; import { textField, textFieldOptional } from "./fields"; describe("textField", () => { - const schema = textField({ validate: "url", maxLength: 150 }); + const urlSchema = textField({ validate: "url", maxLength: 30 }); test.each([ ["https://sendou.ink", true, "https URL"], @@ -12,18 +12,27 @@ describe("textField", () => { ["JavaScript:alert(1)", false, "javascript URL with mixed case protocol"], ["data:text/html,", false, "data URL"], ["not a url", false, "not a URL at all"], + ["https://sendou.ink/aaaaaaaaaaaaaaaaaaaaaa", false, "URL over maxLength"], ])("%s -> %s (%s)", (input, expected) => { - expect(v.safeParse(schema, input).success).toBe(expected); + expect(v.safeParse(urlSchema, input).success).toBe(expected); }); }); describe("textFieldOptional", () => { - const schema = textFieldOptional({ validate: "url", maxLength: 150 }); + const urlSchema = textFieldOptional({ validate: "url", maxLength: 30 }); test.each([ ["https://sendou.ink", true, "https URL"], ["javascript:alert(1)", false, "javascript URL"], + ["https://sendou.ink/aaaaaaaaaaaaaaaaaaaaaa", false, "URL over maxLength"], ])("%s -> %s (%s)", (input, expected) => { - expect(v.safeParse(schema, input).success).toBe(expected); + expect(v.safeParse(urlSchema, input).success).toBe(expected); + }); + + test.each([ + { input: "", why: "empty string" }, + { input: undefined, why: "missing value" }, + ])("parses to null ($why)", ({ input }) => { + expect(v.parse(urlSchema, input)).toBeNull(); }); }); diff --git a/app/form/fields.ts b/app/form/fields.ts index bd81b7377..b172bb11c 100644 --- a/app/form/fields.ts +++ b/app/form/fields.ts @@ -40,18 +40,6 @@ import type { TrophyOption, } from "./types"; -const httpUrlSchema = v.pipe( - v.string(), - v.url(), - v.check((value) => { - try { - return ["http:", "https:"].includes(new URL(value).protocol); - } catch { - return false; - } - }, "Only http(s) URLs are allowed."), -); - export const formRegistry = new WeakMap(); /** Clones the schema first so shared instances (e.g. `id`, `stageId`) each get their own registry entry. */ @@ -165,11 +153,6 @@ type TextFieldArgs = WithTypedTranslationKeys< export function textFieldOptional( args: TextFieldArgs, ): v.GenericSchema { - // validated as a plain string, so unlike other optional text fields it has no null fallback and its key stays required - if (args.validate === "url") { - return registerTextField(httpUrlSchema, args, false, false) as never; - } - return registerTextField( safeNullableStringSchema({ min: args.minLength, max: args.maxLength }), args, @@ -179,12 +162,12 @@ export function textFieldOptional( } export function textField(args: TextFieldArgs): v.GenericSchema { - const schema = - args.validate === "url" - ? httpUrlSchema - : safeStringSchema({ min: args.minLength, max: args.maxLength }); - - return registerTextField(schema, args, true, false) as never; + return registerTextField( + safeStringSchema({ min: args.minLength, max: args.maxLength }), + args, + true, + false, + ) as never; } function registerTextField>( @@ -214,6 +197,16 @@ function textFieldRefined>( ): v.GenericSchema { let result: v.GenericSchema = schema; + if (args.validate === "url") { + result = v.pipe( + result, + v.check( + (val) => val === null || isHttpUrl(val), + "forms:errors.invalidUrl", + ), + ); + } + if (args.regExp) { result = v.pipe( result, @@ -246,6 +239,15 @@ function textFieldRefined>( return result; } +/** Only http(s) is allowed so that e.g. a `javascript:` URL can never end up in a rendered link. */ +function isHttpUrl(value: string) { + try { + return ["http:", "https:"].includes(new URL(value).protocol); + } catch { + return false; + } +} + export function inGameName( args: WithTypedTranslationKeys<{ label?: FormsTranslationKey;