Skip to content

Commit fba2b81

Browse files
WoHWoH
authored
Merge pull request #1739 from kchobantonov/file-validation
 required validation for file is not executed closes #1737 and closes #1128 and closes #1738
2 parents 95021e8 + 18ced30 commit fba2b81

19 files changed

+111
-26
lines changed

‎packages/cli/src/routeGeneration/templates/hapi.hbs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export function RegisterRoutes(server: any) {
107107
throw error;
108108
}
109109

110-
const boomErr = boomify(error instanceof Error ? error : new Error(error.message));
110+
const boomErr = boomify(error instanceof Error ? error : new Error(error.message), { statusCode: error.status || 500 });
111111
boomErr.output.statusCode = error.status || 500;
112112
boomErr.output.payload = {
113113
name: error.name,
@@ -199,7 +199,7 @@ export function RegisterRoutes(server: any) {
199199
throw error;
200200
}
201201

202-
const boomErr = boomify(error instanceof Error ? error : new Error(error.message));
202+
const boomErr = boomify(error instanceof Error ? error : new Error(error.message), { statusCode: error.status || 500 });
203203
boomErr.output.statusCode = error.status || 401;
204204
boomErr.output.payload = {
205205
name: error.name,

‎packages/runtime/src/routeGeneration/templates/express/expressTemplateService.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,19 @@ export class ExpressTemplateService extends TemplateService<ExpressApiHandlerPar
7373
case 'body':
7474
return this.validationService.ValidateParam(param, request.body, name, fieldErrors, true, undefined);
7575
case 'body-prop':
76-
return this.validationService.ValidateParam(param, request.body[name], name, fieldErrors, true, 'body.');
76+
return this.validationService.ValidateParam(param, request.body?.[name], name, fieldErrors, true, 'body.');
7777
case 'formData': {
7878
const files = Object.values(args).filter(p => p.dataType === 'file' || (p.dataType === 'array' && p.array && p.array.dataType === 'file'));
7979
if ((param.dataType === 'file' || (param.dataType === 'array' && param.array && param.array.dataType === 'file')) && files.length > 0) {
80-
const requestFiles = request.files as { [fileName: string]: Express.Multer.File[] };
81-
if (requestFiles[name] === undefined) {
82-
return undefined;
83-
}
80+
const requestFiles = request.files as { [fileName: string]: Express.Multer.File[] } | undefined;
8481

85-
const fileArgs = this.validationService.ValidateParam(param, requestFiles[name], name, fieldErrors, false, undefined);
82+
const fileArgs = this.validationService.ValidateParam(param, requestFiles?.[name], name, fieldErrors, false, undefined);
8683
if (param.dataType === 'array') {
8784
return fileArgs;
8885
}
89-
return fileArgs.length === 1 ? fileArgs[0] : fileArgs;
86+
return Array.isArray(fileArgs) && fileArgs.length === 1 ? fileArgs[0] : fileArgs;
9087
}
91-
return this.validationService.ValidateParam(param, request.body[name], name, fieldErrors, false, undefined);
88+
return this.validationService.ValidateParam(param, request.body?.[name], name, fieldErrors, false, undefined);
9289
}
9390
case 'res':
9491
return (status: number | undefined, data: any, headers: any) => {
@@ -114,11 +111,24 @@ export class ExpressTemplateService extends TemplateService<ExpressApiHandlerPar
114111
Object.keys(headers).forEach((name: string) => {
115112
response.set(name, headers[name]);
116113
});
114+
115+
// Check if the response is marked to be JSON
116+
const isJsonResponse = response.get('Content-Type')?.includes('json') || false;
117+
117118
if (data && typeof data.pipe === 'function' && data.readable && typeof data._read === 'function') {
118119
response.status(statusCode || 200);
119120
(data as Readable).pipe(response);
120-
} else if (data !== null && data !== undefined) {
121-
response.status(statusCode || 200).json(data);
121+
} else if (data !== undefined && (data !== null || isJsonResponse)) {
122+
// allow null response when it is a json response
123+
if (typeof data === 'number' || isJsonResponse) {
124+
// express treats number data as status code so use the json method instead
125+
// or if the response was marked as json then use the json so for example strings are quoted
126+
response.status(statusCode || 200).json(data);
127+
} else {
128+
// do not use json for every type since internally the send will invoke json if needed
129+
// but for string data it will not quote it, so we can send string as plain/text data
130+
response.status(statusCode || 200).send(data);
131+
}
122132
} else {
123133
response.status(statusCode || 204).end();
124134
}

‎packages/runtime/src/routeGeneration/templates/hapi/hapiTemplateService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export class HapiTemplateService extends TemplateService<HapiApiHandlerParameter
127127
return tsoaResponsed.value;
128128
}
129129

130-
const response = data !== null && data !== undefined ? h.response(data).code(200) : h.response('').code(204);
130+
const response = data !== null && data !== undefined ? h.response(data).code(200) : h.response().code(204);
131131

132132
Object.keys(headers).forEach((name: string) => {
133133
response.header(name, headers[name]);

‎packages/runtime/src/routeGeneration/templates/koa/koaTemplateService.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,13 @@ export class KoaTemplateService extends TemplateService<KoaApiHandlerParameters,
8686
const files = Object.values(args).filter(p => p.dataType === 'file' || (p.dataType === 'array' && p.array && p.array.dataType === 'file'));
8787
const contextRequest = context.request as any;
8888
if ((param.dataType === 'file' || (param.dataType === 'array' && param.array && param.array.dataType === 'file')) && files.length > 0) {
89-
if (contextRequest.files[name] === undefined) {
90-
return undefined;
91-
}
92-
93-
const fileArgs = this.validationService.ValidateParam(param, contextRequest.files[name], name, errorFields, false, undefined);
89+
const fileArgs = this.validationService.ValidateParam(param, contextRequest.files?.[name], name, errorFields, false, undefined);
9490
if (param.dataType === 'array') {
9591
return fileArgs;
9692
}
97-
return fileArgs.length === 1 ? fileArgs[0] : fileArgs;
93+
return Array.isArray(fileArgs) && fileArgs.length === 1 ? fileArgs[0] : fileArgs;
9894
}
99-
return this.validationService.ValidateParam(param, contextRequest.body[name], name, errorFields, false, undefined);
95+
return this.validationService.ValidateParam(param, contextRequest.body?.[name], name, errorFields, false, undefined);
10096
}
10197
case 'res':
10298
return async (status: number | undefined, data: any, headers: any): Promise<void> => {

‎tests/esm/integration/express.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ describe('Express Server', () => {
3333
}
3434

3535
if (err) {
36+
verifyResponse(err, res);
3637
reject({
3738
error: err,
3839
response: parsedError,

‎tests/integration/dynamic-controllers-express-server.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,6 +1242,7 @@ describe('Express Server', () => {
12421242
}
12431243

12441244
if (err) {
1245+
verifyResponse(err, res);
12451246
reject({
12461247
error: err,
12471248
response: parsedError,

‎tests/integration/express-router-server.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ describe('Express Router Server', () => {
3939
}
4040

4141
if (err) {
42+
verifyResponse(err, res);
4243
reject({
4344
error: err,
4445
response: parsedError,

‎tests/integration/express-server-custom-multer.spec.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,22 @@ describe('Express Server With custom multer', () => {
4141
});
4242
});
4343

44+
it('cannot post a file with no file', async () => {
45+
const formData = { notAFileAttribute: 'not a file' };
46+
verifyFileUploadRequest(basePath + '/PostTest/File', formData, (_err, res) => {
47+
expect(res.status).to.equal(400);
48+
expect(res.text).to.equal('{"fields":{"someFile":{"message":"\'someFile\' is required"}},"message":"An error occurred during the request.","name":"ValidateError","status":400}');
49+
});
50+
});
51+
52+
it('can post a file with no file', async () => {
53+
const formData = { notAFileAttribute: 'not a file' };
54+
verifyFileUploadRequest(basePath + '/PostTest/FileOptional', formData, (_err, res) => {
55+
expect(res.status).to.equal(200);
56+
expect(res.text).to.equal('no file');
57+
});
58+
});
59+
4460
it('can post multiple files with other form fields', () => {
4561
const formData = {
4662
a: 'b',
@@ -197,6 +213,7 @@ describe('Express Server With custom multer', () => {
197213
}
198214

199215
if (err) {
216+
verifyResponse(err, res);
200217
reject({
201218
error: err,
202219
response: parsedError,

‎tests/integration/express-server-root-security.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ describe('Express Server with api_key Root Security', () => {
6868
}
6969

7070
if (err) {
71+
verifyResponse(err, res);
7172
reject({
7273
error: err,
7374
response: parsedError,

‎tests/integration/express-server.spec.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,7 +1514,7 @@ describe('Express Server', () => {
15141514
const mainResourceId = 'main-123';
15151515

15161516
return verifyGetRequest(basePath + `/SubResourceTest/${mainResourceId}/SubResource`, (_err, res) => {
1517-
expect(res.body).to.equal(mainResourceId);
1517+
expect(res.text).to.equal(mainResourceId);
15181518
});
15191519
});
15201520

@@ -1523,7 +1523,7 @@ describe('Express Server', () => {
15231523
const subResourceId = 'sub-456';
15241524

15251525
return verifyGetRequest(basePath + `/SubResourceTest/${mainResourceId}/SubResource/${subResourceId}`, (_err, res) => {
1526-
expect(res.body).to.equal(`${mainResourceId}:${subResourceId}`);
1526+
expect(res.text).to.equal(`${mainResourceId}:${subResourceId}`);
15271527
});
15281528
});
15291529
});
@@ -1559,6 +1559,22 @@ describe('Express Server', () => {
15591559
});
15601560
});
15611561

1562+
it('cannot post a file with no file', async () => {
1563+
const formData = { notAFileAttribute: 'not a file' };
1564+
verifyFileUploadRequest(basePath + '/PostTest/File', formData, (_err, res) => {
1565+
expect(res.status).to.equal(400);
1566+
expect(res.text).to.equal('{"fields":{"someFile":{"message":"\'someFile\' is required"}},"message":"An error occurred during the request.","name":"ValidateError","status":400}');
1567+
});
1568+
});
1569+
1570+
it('can post a file with no file', async () => {
1571+
const formData = { notAFileAttribute: 'not a file' };
1572+
verifyFileUploadRequest(basePath + '/PostTest/FileOptional', formData, (_err, res) => {
1573+
expect(res.status).to.equal(200);
1574+
expect(res.text).to.equal('no file');
1575+
});
1576+
});
1577+
15621578
it('can post multiple files with other form fields', () => {
15631579
const formData = {
15641580
a: 'b',
@@ -1723,6 +1739,7 @@ describe('Express Server', () => {
17231739
}
17241740

17251741
if (err) {
1742+
verifyResponse(err, res);
17261743
reject({
17271744
error: err,
17281745
response: parsedError,

0 commit comments

Comments
 (0)