Skip to content

Commit

Permalink
fix(cors): use RouterNode for the available ACAM values (#658)
Browse files Browse the repository at this point in the history
  • Loading branch information
kyranet authored Nov 6, 2024
1 parent c49da29 commit b77af3f
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion packages/api/src/lib/structures/router/RouterNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class RouterNode {
/**
* The methods this node supports.
*/
#methods = new Collection<MethodName, Route>();
readonly #methods = new Collection<MethodName, Route>();

public constructor(parent: RouterBranch) {
this.parent = parent;
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/middlewares/cookies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
15 changes: 7 additions & 8 deletions packages/api/src/middlewares/headers.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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(', ');
}

/**
Expand All @@ -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) {
Expand Down
20 changes: 19 additions & 1 deletion packages/api/tests/lib/structures/router/RouterRoot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand All @@ -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]'));
Expand Down
8 changes: 4 additions & 4 deletions packages/api/tests/shared.ts
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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')
});
}

0 comments on commit b77af3f

Please sign in to comment.