From b77af3f30897a3e123dce323f02cffba30ce4735 Mon Sep 17 00:00:00 2001 From: Aura Date: Wed, 6 Nov 2024 08:05:45 +0100 Subject: [PATCH] fix(cors): use `RouterNode` for the available ACAM values (#658) --- .../src/lib/structures/router/RouterNode.ts | 2 +- packages/api/src/middlewares/cookies.ts | 2 +- packages/api/src/middlewares/headers.ts | 15 +++++++------- .../lib/structures/router/RouterRoot.test.ts | 20 ++++++++++++++++++- packages/api/tests/shared.ts | 8 ++++---- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/api/src/lib/structures/router/RouterNode.ts b/packages/api/src/lib/structures/router/RouterNode.ts index 3591063e..5c58d6ed 100644 --- a/packages/api/src/lib/structures/router/RouterNode.ts +++ b/packages/api/src/lib/structures/router/RouterNode.ts @@ -12,7 +12,7 @@ export class RouterNode { /** * The methods this node supports. */ - #methods = new Collection(); + readonly #methods = new Collection(); public constructor(parent: RouterBranch) { this.parent = parent; diff --git a/packages/api/src/middlewares/cookies.ts b/packages/api/src/middlewares/cookies.ts index 357a2e25..ff3e8758 100644 --- a/packages/api/src/middlewares/cookies.ts +++ b/packages/api/src/middlewares/cookies.ts @@ -3,7 +3,7 @@ import { CookieStore } from '../lib/structures/api/CookieStore'; export class PluginMiddleware extends Middleware { private readonly production: boolean = process.env.NODE_ENV === 'production'; - private domainOverwrite: string | null = null; + private readonly domainOverwrite: string | null; public constructor(context: Middleware.LoaderContext) { super(context, { position: 30 }); diff --git a/packages/api/src/middlewares/headers.ts b/packages/api/src/middlewares/headers.ts index 847517bd..fc025a5b 100644 --- a/packages/api/src/middlewares/headers.ts +++ b/packages/api/src/middlewares/headers.ts @@ -1,7 +1,8 @@ +import { isNullish } from '@sapphire/utilities'; import { Middleware } from '../lib/structures/Middleware'; -import type { Route } from '../lib/structures/Route'; import type { RouteStore } from '../lib/structures/RouteStore'; import { HttpCodes } from '../lib/structures/http/HttpCodes'; +import type { RouterNode } from '../lib/structures/router/RouterNode'; export class PluginMiddleware extends Middleware { private readonly origin: string; @@ -18,19 +19,17 @@ export class PluginMiddleware extends Middleware { response.setHeader('Access-Control-Allow-Credentials', 'true'); response.setHeader('Access-Control-Allow-Origin', this.origin); response.setHeader('Access-Control-Allow-Headers', 'Authorization, User-Agent, Content-Type'); - response.setHeader('Access-Control-Allow-Methods', this.getMethods(request.route ?? null)); + response.setHeader('Access-Control-Allow-Methods', this.getMethods(request.routerNode)); this.ensurePotentialEarlyExit(request, response); } - private getMethods(route: Route | null) { - if (route === null) { + private getMethods(routerNode: RouterNode | null | undefined): string { + if (isNullish(routerNode)) { return this.routes.router.supportedMethods.join(', '); } - if (route.methods.size === 0) return ''; - if (route.methods.size === 1) return route.methods.keys().next().value; - return [...route.methods].join(', '); + return [...routerNode.methods()].join(', '); } /** @@ -50,7 +49,7 @@ export class PluginMiddleware extends Middleware { */ private ensurePotentialEarlyExit({ method, route, routerNode }: Middleware.Request, response: Middleware.Response) { if (method === 'OPTIONS') { - if (!route || !route.methods.has('OPTIONS')) { + if (!route?.methods.has('OPTIONS')) { response.end(); } } else if (routerNode === null) { diff --git a/packages/api/tests/lib/structures/router/RouterRoot.test.ts b/packages/api/tests/lib/structures/router/RouterRoot.test.ts index 777a9bb9..b9a6c27c 100644 --- a/packages/api/tests/lib/structures/router/RouterRoot.test.ts +++ b/packages/api/tests/lib/structures/router/RouterRoot.test.ts @@ -32,8 +32,13 @@ describe('RouterBranch', () => { test('GIVEN a child of the root THEN it should return the correct path', () => { const root = new RouterRoot(); - const value = root.add(makeRoute('test')); + const route = makeRoute('test'); + const value = root.add(route); + expect(value.path).toBe('/test'); + expect([...value.methods()]).toEqual(['GET']); + expect(value.extractParameters(['test'])).toEqual({}); + expect(value.get('GET')).toBe(route); }); }); @@ -50,6 +55,19 @@ describe('RouterBranch', () => { expect(value.children.map((child) => child.toString())).toEqual(['child1', 'child2']); }); + test('GIVEN a branch with two same-name static children with different methods THEN it should return the correct children in insert order', () => { + const value = new RouterRoot(); + value.add(makeRoute('child1', ['GET'])); + value.add(makeRoute('child1', ['POST'])); + + expect(value.children).toHaveLength(1); + expect([...value.node.methods()]).toEqual([]); + expect([...value.children[0].node.methods()]).toEqual(['GET', 'POST']); + + expect(value.supportedMethods).toEqual(['GET', 'POST']); + expect(value.children[0].supportedMethods).toEqual(['GET', 'POST']); + }); + test('GIVEN a branch with dynamic children THEN it should return the correct children in insert order', () => { const value = new RouterRoot(); value.add(makeRoute('[child1]')); diff --git a/packages/api/tests/shared.ts b/packages/api/tests/shared.ts index 2260cc68..6ac9c8f7 100644 --- a/packages/api/tests/shared.ts +++ b/packages/api/tests/shared.ts @@ -1,13 +1,13 @@ import { VirtualPath, container } from '@sapphire/pieces'; -import { Route, RouterBranch } from '../src'; +import { Route, type MethodName } from '../src'; -export function makeRoute(route: string) { +export function makeRoute(route: string, methods: readonly MethodName[] = ['GET']) { // @ts-expect-error Stub container.server ??= { options: {} }; class UserRoute extends Route { public constructor(context: Route.LoaderContext) { - super(context, { route, methods: ['GET'] }); + super(context, { route, methods }); } public override run() { @@ -19,6 +19,6 @@ export function makeRoute(route: string) { name: VirtualPath, path: VirtualPath, root: VirtualPath, - store: container.stores.get('routes')! + store: container.stores.get('routes') }); }