Skip to content

Commit f6dc977

Browse files
committed
fix: Correctly handle multiple routing classes
Due to the RoutedHttpRequestHandler catching unsupported requests with a default handler, it was impossible to combine multiple with a WaterfallHandler. This is now solved by moving most of the behaviour to the canHandle, and having a static throw handler for the 404.
1 parent 89eb900 commit f6dc977

2 files changed

Lines changed: 29 additions & 21 deletions

File tree

packages/uma/config/default.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,7 @@
106106
{ "@id": "urn:uma:default:ResourceRegistrationRoute" },
107107
{ "@id": "urn:uma:default:ResourceRegistrationOpsRoute" },
108108
{ "@id": "urn:uma:default:IntrospectionRoute" }
109-
],
110-
"defaultHandler": {
111-
"@type": "DefaultRequestHandler"
112-
}
109+
]
113110
}
114111
},
115112
{
@@ -121,6 +118,10 @@
121118
{ "@id": "urn:uma:default:LogRoute" },
122119
{ "@id": "urn:uma:default:VCRoute" }
123120
]
121+
},
122+
{
123+
"@type": "StaticThrowHandler",
124+
"error": { "@type": "NotFoundHttpError" }
124125
}
125126
]
126127
}

packages/uma/src/util/http/server/RoutedHttpRequestHandler.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
import { getLoggerFor } from '@solid/community-server';
1+
import {
2+
getLoggerFor,
3+
InternalServerError,
4+
MethodNotAllowedHttpError,
5+
NotImplementedHttpError
6+
} from '@solid/community-server';
27
import Template from 'uri-template-lite';
38
import { HttpHandler, HttpHandlerContext, HttpHandlerResponse } from '../models/HttpHandler';
49
import { HttpHandlerRoute } from '../models/HttpHandlerRoute';
510

11+
type RouteMatch = { parameters: Record<string, string>, route: HttpHandlerRoute };
12+
613
/**
714
* A {@link HttpHandler} handling requests based on routes in a given list of {@link HttpHandlerRoute}s.
815
* Route paths can contain variables as described in RFC 6570.
@@ -18,16 +25,17 @@ import { HttpHandlerRoute } from '../models/HttpHandlerRoute';
1825
* E.g., `http://example.com/foo/bar` will match the route template `/bar`.
1926
*/
2027
export class RoutedHttpRequestHandler extends HttpHandler {
28+
protected readonly logger = getLoggerFor(this);
2129

2230
protected readonly routeMap: Map<Template, HttpHandlerRoute>;
23-
protected readonly logger = getLoggerFor(this);
31+
protected readonly handledRequests: WeakMap<HttpHandlerContext, RouteMatch>;
32+
2433

2534
/**
2635
* Creates a RoutedHttpRequestHandler, super calls the HttpHandler class and expects a list of HttpHandlerControllers.
2736
*/
2837
constructor(
2938
routes: HttpHandlerRoute[],
30-
protected readonly defaultHandler?: HttpHandler,
3139
onlyMatchTail = false,
3240
) {
3341
super();
@@ -37,9 +45,10 @@ export class RoutedHttpRequestHandler extends HttpHandler {
3745
// Add a catchall variable to the front if only the URL tail needs to be matched.
3846
this.routeMap.set(new Template(onlyMatchTail ? `{_prefix}${route.path}` : route.path), route);
3947
}
48+
this.handledRequests = new WeakMap();
4049
}
4150

42-
async handle(context: HttpHandlerContext): Promise<HttpHandlerResponse> {
51+
public async canHandle(context: HttpHandlerContext): Promise<void> {
4352
const request = context.request;
4453
const path = request.url.pathname;
4554

@@ -55,26 +64,24 @@ export class RoutedHttpRequestHandler extends HttpHandler {
5564
}
5665

5766
if (!match) {
58-
if (this.defaultHandler) {
59-
this.logger.info(`No matching route found, calling default handler. ${path}`);
60-
return this.defaultHandler.handleSafe(context);
61-
} else {
62-
this.logger.error(`No matching route found. ${path}`);
63-
return { body: '', headers: {}, status: 404 };
64-
}
67+
throw new NotImplementedHttpError();
6568
}
6669

6770
this.logger.debug(`Route matched: ${JSON.stringify({ path, parameters: match.parameters })}`);
6871

6972
if (match.route.methods && !match.route.methods.includes(request.method)) {
7073
this.logger.info(`Operation not supported. Supported operations: ${ match.route.methods }`);
71-
return {
72-
status: 405,
73-
headers: { 'allow': match.route.methods.join(', ') },
74-
body: '',
75-
};
74+
throw new MethodNotAllowedHttpError([ request.method ]);
75+
}
76+
this.handledRequests.set(context, match);
77+
}
78+
79+
public async handle(context: HttpHandlerContext): Promise<HttpHandlerResponse> {
80+
const match = this.handledRequests.get(context);
81+
if (!match) {
82+
throw new InternalServerError('Calling handle without successful canHandle');
7683
}
77-
request.parameters = { ...request.parameters, ...match.parameters };
84+
context.request.parameters = { ...context.request.parameters, ...match.parameters };
7885

7986
return match.route.handler.handleSafe(context);
8087
}

0 commit comments

Comments
 (0)