Skip to content

Commit

Permalink
fix(http): make sure all path parameters are available in HttpRequest…
Browse files Browse the repository at this point in the history 
…Parser

Also, make sure that cache is correctly placed.
  • Loading branch information
@marcj
marcj committed Feb 9, 2024
1 parent 1bb3641 commit e215420
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 26 deletions.
11 changes: 7 additions & 4 deletions packages/http/src/module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export class HttpModule extends createModule({
throw new Error(`Listener ${stringifyListener(listener)} requires async HttpBody. This is not yet supported. You have to parse the request manually by injecting HttpRequest.`);
}

let build: Function;
const parserCache = new Map<RouteConfig, Function>();

for (let index = 0; index < params.length; index++) {
const parameter = params[index];
if (!parameterRequiresRequest(parameter)) continue;
Expand All @@ -93,14 +94,16 @@ export class HttpModule extends createModule({

this.addProvider({
provide: uniqueType, useFactory: (httpConfig: HttpConfig, request: HttpRequest, injector: InjectorContext, config?: RouteConfig) => {
let build = config && parserCache.get(config);
if (!build) {
const params = listener.reflection.getParameters().slice(1);
build = buildRequestParser(httpConfig.parser, params, config?.getFullPath());
build = buildRequestParser(httpConfig.parser, params, config);
if (config) parserCache.set(config, build);
}

const parser = build(request);
const params = parser(injector);
return params.arguments[i];
const result = parser(injector);
return result.arguments[i];
}, scope: 'http'
});
this.addExport(uniqueType);
Expand Down
16 changes: 11 additions & 5 deletions packages/http/src/request-parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,28 @@ function isTypeUnknown(type: Type): boolean {
|| type.kind === ReflectionKind.never;
}

export function buildRequestParser(parseOptions: HttpParserOptions, parameters: ReflectionParameter[], path?: string): (request: HttpRequest) => any[] {
export function buildRequestParser(parseOptions: HttpParserOptions, parameters: ReflectionParameter[], routeConfig?: RouteConfig): (request: HttpRequest) => any[] {
const compiler = new CompilerContext();
const params = parameters.map(v => new ParameterForRequestParser(v));

//todo: parse path
let pathRegex = '';
if (path) {
const parsedPath = parseRoutePathToRegex(path, parameters);
let pathParameterNames: { [name: string]: number } = {};

if (routeConfig) {
const parsedPath = parseRoutePathToRegex(routeConfig.getFullPath(), parameters);
pathRegex = parsedPath.regex;
pathParameterNames = parsedPath.parameterNames;

for (const param of params) {
param.regexPosition = parsedPath.parameterNames[param.parameter.name];
}
}

const code = getRequestParserCodeForParameters(compiler, parseOptions, params, {});
const code = getRequestParserCodeForParameters(compiler, parseOptions, params, {
pathParameterNames,
routeConfig
});
compiler.context.set('ValidationError', ValidationError);
compiler.context.set('qs', qs);

Expand Down Expand Up @@ -254,7 +260,7 @@ export function getRequestParserCodeForParameters(
setParameters.push(`parameters.${parameter.parameter.name} = async (options = {}) => {
let res = {};
if (options.withPath !== false) {
${assignPathNames.join('\n')};
${assignPathNames.join('\n')}
}
if (options.withHeader !== false) {
Object.assign(res, _headers);
Expand Down
101 changes: 84 additions & 17 deletions packages/http/tests/router.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ test('parameter from header', async () => {
}

@http.GET('second/:userId')
handle2(userId: number, groupId: HttpHeader<number, {name: 'group_id'}>) {
handle2(userId: number, groupId: HttpHeader<number, { name: 'group_id' }>) {
return [userId, groupId];
}
}
Expand Down Expand Up @@ -1307,7 +1307,7 @@ test('body and queries in listener', async () => {
}

@http.POST('/3')
handle3({ userId }: HttpBody<{userId: number}>, session: HttpSession) {
handle3({ userId }: HttpBody<{ userId: number }>, session: HttpSession) {
return [userId, session.userId, session.auth];
}

Expand All @@ -1316,16 +1316,28 @@ test('body and queries in listener', async () => {
const data = await parser();
return [data.auth, data.userId];
}

@http.GET('/5/:userId')
async handle5(userId: number, parser: HttpRequestParser<AuthData>) {
const data = await parser();
return [userId, data.auth, data.userId];
}

@http.GET('/6/:userId')
async handle6(parser: HttpRequestParser<AuthData>) {
const data = await parser();
return [data.auth, data.userId];
}

@http.GET('/7/:userId')
async handle7(session: HttpSession) {
return [session.auth, session.userId];
}

@http.GET('/8/:id/:userId')
async handle8(id: string, session: HttpSession) {
return [id, session.auth, session.userId];
}
}

type AuthData = {
Expand All @@ -1343,14 +1355,16 @@ test('body and queries in listener', async () => {
}

const httpKernel = createHttpKernel([Controller], [{ provide: HttpSession, scope: 'http' }], [Listener]);
expect((await httpKernel.request(HttpRequest.POST('/1?userId=1').json({auth: 'secretToken1'}))).json).toEqual([1, 1, 'secretToken1']);
expect((await httpKernel.request(HttpRequest.POST('/1?userId=1').json({ auth: 'secretToken1' }))).json).toEqual([1, 1, 'secretToken1']);
expect((await httpKernel.request(HttpRequest.GET('/2?auth=secretToken1&userId=1'))).json).toEqual([1, 1, 'secretToken1']);
expect((await httpKernel.request(HttpRequest.GET('/2?userId=1'))).json.message).toEqual('Validation error:\nauth(type): Not a string');
expect((await httpKernel.request(HttpRequest.POST('/3?auth=secretToken1').json({userId: '24'}))).json).toEqual([24, 24, 'secretToken1']);
expect((await httpKernel.request(HttpRequest.POST('/3?auth=secretToken1').json({ userId: '24' }))).json).toEqual([24, 24, 'secretToken1']);
expect((await httpKernel.request(HttpRequest.GET('/4?auth=secretToken1&userId=1'))).json).toEqual(['secretToken1', '1']);
expect((await httpKernel.request(HttpRequest.GET('/4?userId=1').header('auth', 'secretToken1'))).json).toEqual(['secretToken1', '1']);
expect((await httpKernel.request(HttpRequest.GET('/5/1?auth=secretToken1'))).json).toEqual([1, 'secretToken1', '1']);
expect((await httpKernel.request(HttpRequest.GET('/6/1?auth=secretToken1'))).json).toEqual(['secretToken1', '1']);
expect((await httpKernel.request(HttpRequest.GET('/6/2?auth=secretToken1'))).json).toEqual(['secretToken1', '2']);
expect((await httpKernel.request(HttpRequest.GET('/7/3?auth=secretToken1'))).json).toEqual(['secretToken1', 3]);
expect((await httpKernel.request(HttpRequest.GET('/8/1/3?auth=secretToken1'))).json).toEqual(['1', 'secretToken1', 3]);
});

test('stream', async () => {
Expand Down Expand Up @@ -1442,8 +1456,8 @@ test('upload security', async () => {
path: '/etc/secure-file',
name: 'fakefile',
type: 'image/jpeg',
lastModifiedDate: null
}
lastModifiedDate: null,
},
}))).json.message).toContain('Not an uploaded file caused by value');

// ensure type deserialization doesn't set the invalid 'fake value' value to UploadedFileSymbol
Expand All @@ -1454,16 +1468,16 @@ test('upload security', async () => {
path: '/etc/secure-file',
name: 'fakefile',
type: 'image/jpeg',
lastModifiedDate: null
}
lastModifiedDate: null,
},
}))).json.message).toContain('Not an uploaded file caused by value');

expect((await httpKernel.request(HttpRequest.POST('/upload').multiPart([
{
name: 'someFile',
file: Buffer.from('testing a text file'),
fileName: 'test.txt'
}
fileName: 'test.txt',
},
]))).json).toMatchObject({ uploadedSize: 19 });
});

Expand Down Expand Up @@ -1622,8 +1636,8 @@ test('dependency injection unknown', async () => {
});

test('disabled reflection', async () => {
type User = {username: string};
type Doc = {id: number};
type User = { username: string };
type Doc = { id: number };

/** @reflection never */
type PopulateUser<T> = T & { user: User };
Expand All @@ -1633,9 +1647,9 @@ test('disabled reflection', async () => {
const user: DocUser = {
id: 1,
user: {
username: 'peter2'
}
}
username: 'peter2',
},
};

class Controller {
@http.GET('user/:name')
Expand All @@ -1644,6 +1658,59 @@ test('disabled reflection', async () => {
}
}

const httpKernel = createHttpKernel([Controller], [provide<DocUser>({useValue: user})]);
const httpKernel = createHttpKernel([Controller], [provide<DocUser>({ useValue: user })]);
expect((await httpKernel.request(HttpRequest.GET('/user/peter'))).json).toEqual(['peter', 1, 'peter2']);
});

test('throw when invalid data returned', async () => {
// todo: auto serialization of route return value should throw if invalid data was given
});

test('http optional query', async () => {
class Controller {
@http.GET('user')
async anyReq(name: HttpQuery<string | undefined>) {
return [name];
}
}

const httpKernel = createHttpKernel([Controller]);
expect((await httpKernel.request(HttpRequest.GET('/user'))).json).toEqual([null]);
expect((await httpKernel.request(HttpRequest.GET('/user?name=Peter'))).json).toEqual(['Peter']);
});

test('required query should be required', async () => {
class Controller {
@http.GET('')
async anyReq(entityId: HttpQuery<string>) {
return [entityId];
}
}

const httpKernel = createHttpKernel([Controller]);
expect((await httpKernel.request(HttpRequest.GET('/'))).json).toMatchObject({
message: 'Validation error:\nentityId(type): No value given',
});
expect((await httpKernel.request(HttpRequest.GET('/?entityId=asd'))).json).toEqual(['asd']);
});

test('required fields in body should be required', async () => {
interface Body {
a: string | null;
b: string;
}

class Controller {
@http.POST('')
async anyReq(body: HttpBody<Body>) {
return [body.a, body.b];
}
}

const httpKernel = createHttpKernel([Controller]);
expect((await httpKernel.request(HttpRequest.POST('/'))).json).toMatchObject({
message: 'Validation error:\nb(type): Not a string',
});
expect((await httpKernel.request(HttpRequest.POST('/').json({ b: 'asd' }))).json).toEqual([null, 'asd']);
expect((await httpKernel.request(HttpRequest.POST('/').json({ a: 'a', b: 'asd' }))).json).toEqual(['a', 'asd']);
})

0 comments on commit e215420

Please sign in to comment.