fix: Restrict header types, fix array handling

lukeautry · Dec 30, 2020 · edafcc0 · edafcc0
1 parent 2c3543b
commit edafcc0
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 49 deletions.
diff --git a/packages/cli/src/swagger/specGenerator2.ts b/packages/cli/src/swagger/specGenerator2.ts
@@ -202,7 +202,7 @@ export class SpecGenerator2 extends SpecGenerator {
         if (res.headers.dataType === 'refObject' || res.headers.dataType === 'nestedObjectLiteral') {
           res.headers.properties.forEach((each: Tsoa.Property) => {
             headers[each.name] = {
-              type: this.getSwaggerType(each.type).type,
+              ...this.getSwaggerType(each.type),
               description: each.description,
             };
           });

diff --git a/packages/cli/src/swagger/specGenerator3.ts b/packages/cli/src/swagger/specGenerator3.ts
@@ -303,16 +303,16 @@ export class SpecGenerator3 extends SpecGenerator {
       }
 
       if (res.headers) {
-        const headers = {};
+        const headers: { [name: string]: Swagger.Header3 } = {};
         if (res.headers.dataType === 'refObject') {
           headers[res.headers.refName] = {
             schema: this.getSwaggerTypeForReferenceType(res.headers) as Swagger.Schema3,
             description: res.headers.description,
-          } as Swagger.Header3;
+          };
         } else if (res.headers.dataType === 'nestedObjectLiteral') {
           res.headers.properties.forEach((each: Tsoa.Property) => {
             headers[each.name] = {
-              schema: this.getSwaggerType(each.type),
+              schema: this.getSwaggerType(each.type) as Swagger.Schema3,
               description: each.description,
               required: each.required,
             };

diff --git a/packages/runtime/src/decorators/response.ts b/packages/runtime/src/decorators/response.ts
@@ -1,10 +1,12 @@
-export function SuccessResponse<HeaderType = {}>(name: string | number, description?: string): Function {
+import { IsValidHeader } from '../utils/isHeaderType';
+
+export function SuccessResponse<HeaderType extends IsValidHeader<HeaderType> = {}>(name: string | number, description?: string): Function {
   return () => {
     return;
   };
 }
 
-export function Response<ExampleType, HeaderType = {}>(name: string | number, description?: string, example?: ExampleType): Function {
+export function Response<ExampleType, HeaderType extends IsValidHeader<HeaderType> = {}>(name: string | number, description?: string, example?: ExampleType): Function {
   return () => {
     return;
   };

diff --git a/packages/runtime/src/interfaces/response.ts b/packages/runtime/src/interfaces/response.ts
@@ -1,3 +1,5 @@
+import { IsValidHeader } from '../utils/isHeaderType';
+
 export type HttpStatusCodeLiteral =
   | 100
   | 101
@@ -58,4 +60,4 @@ export type HttpStatusCodeLiteral =
   | 510
   | 511;
 
-export type TsoaResponse<T extends HttpStatusCodeLiteral, U, V extends {} = {}> = (status: T, data: U, headers?: V) => any;
+export type TsoaResponse<T extends HttpStatusCodeLiteral, BodyType, HeaderType extends IsValidHeader<HeaderType> = {}> = (status: T, data: BodyType, headers?: HeaderType) => any;
diff --git a/packages/runtime/src/utils/isHeaderType.ts b/packages/runtime/src/utils/isHeaderType.ts
@@ -0,0 +1,9 @@
+/**
+ * Checks for a supported header type
+ * key types can be `never` to support `{}` or `number` to support
+ */
+export type IsValidHeader<Header> = keyof Header extends string
+  ? Header[keyof Header] extends string | string[] | undefined
+    ? {}
+    : 'Header values must be string or string[]'
+  : 'Header names must be of type string';
diff --git a/tests/fixtures/controllers/commonResponseHeaderClassController.ts b/tests/fixtures/controllers/commonResponseHeaderClassController.ts
@@ -10,14 +10,14 @@ class CommonResponseHeader {
   CommonLink: string;
 
   /**
-   * b common link str
+   * b common link str[]
    */
-  CommonLinkB: string;
+  CommonLinkB: string[];
 
   /**
-   * c common link number, optional
+   * c common link string, optional
    */
-  CommonLinkC?: number;
+  CommonLinkC?: string;
 }
 
 @Route('CommonResponseHeaderClass')

diff --git a/tests/fixtures/controllers/commonResponseHeaderObjectController.ts b/tests/fixtures/controllers/commonResponseHeaderObjectController.ts
@@ -1,11 +1,14 @@
 import { Controller, Get, Route, Response } from '@tsoa/runtime';
 
 @Route('CommonResponseHeaderObject')
-@Response<null, {
-  objectA: string;
-  objectB: number;
-  objectC?: string;
-}>(200, 'Ok')
+@Response<
+  null,
+  {
+    objectA: string;
+    objectB: string[];
+    objectC?: string;
+  }
+>(200, 'Ok')
 export class CommonResponseHeaderObjectController extends Controller {
   @Get('Response1')
   public async handler(): Promise<void> {

diff --git a/tests/fixtures/controllers/incorrectResponseHeaderController.ts b/tests/fixtures/controllers/incorrectResponseHeaderController.ts
@@ -1,7 +1,7 @@
-import { Controller, Get, Response, Route } from "@tsoa/runtime";
+import { Controller, Get, Response, Route } from '@tsoa/runtime';
 
 @Route('IncorrectResponseHeader')
-@Response<null, null>(200)
+@Response<null, any>(200)
 export class IncorrectResponseHeaderController extends Controller {
   @Get()
   public async handler(): Promise<void> {

diff --git a/tests/fixtures/controllers/invalidHeaderController.ts b/tests/fixtures/controllers/invalidHeaderController.ts
@@ -3,7 +3,7 @@ import { Get, Res, Route, TsoaResponse } from '@tsoa/runtime';
 @Route('/')
 export class InvalidHeaderTestController {
   @Get('/path')
-  public async getWithInvalidHeader(@Res() notFound: TsoaResponse<404, void, 'asd'>): Promise<void> {
+  public async getWithInvalidHeader(@Res() notFound: TsoaResponse<404, void, 'Header names must be of type string'>): Promise<void> {
     return;
   }
 }
diff --git a/tests/fixtures/controllers/responseHeaderController.ts b/tests/fixtures/controllers/responseHeaderController.ts
@@ -3,21 +3,21 @@ import { Controller, Get, Route, SuccessResponse, Response, TsoaResponse, Res }
 /**
  * response header's description
  */
-class ResponseHeader {
+interface ResponseHeader {
   /**
    * a link string
    */
   Link: string;
 
   /**
-   * b link str
+   * b link str[]
    */
-  LinkB: string;
+  LinkB: string[];
 
   /**
-   * c link number, optional
+   * c link string, optional
    */
-  LinkC?: number;
+  LinkC?: string;
 }
 
 @Route('ResponseHeader')
@@ -29,7 +29,7 @@ export class ResponseHeaderController extends Controller {
   }
 
   @Get('SuccessResponseWithObject')
-  @SuccessResponse<{ linkA: string; linkB: string; linkOpt?: string }>(200, 'xxx')
+  @SuccessResponse<{ linkA: string; linkB: string[]; linkOpt?: string }>(200, 'xxx')
   public async handler2(): Promise<void> {
     return;
   }
@@ -41,7 +41,7 @@ export class ResponseHeaderController extends Controller {
   }
 
   @Get('ResponseWithObject')
-  @Response<null, { linkC: number; linkD: string; linkOpt?: number }>(200, 'yyy')
+  @Response<null, { linkC: string; linkD: string[]; linkOpt?: string }>(200, 'yyy')
   public async handler4(): Promise<void> {
     return;
   }
@@ -52,7 +52,7 @@ export class ResponseHeaderController extends Controller {
   }
 
   @Get('TsoaResponseWithObject')
-  public async handler6(@Res() res: TsoaResponse<200, null, { linkE: string; linkF: number; linkOpt?: string }>): Promise<void> {
+  public async handler6(@Res() res: TsoaResponse<200, null, { linkE: string; linkF: string[]; linkOpt?: string }>): Promise<void> {
     return;
   }
 }
diff --git a/tests/unit/swagger/pathGeneration/getRoutes.spec.ts b/tests/unit/swagger/pathGeneration/getRoutes.spec.ts
@@ -184,12 +184,12 @@ describe('GET route generation', () => {
     expect(() => {
       new MetadataGenerator('./fixtures/controllers/invalidHeaderController.ts').Generate();
     }).to.throw(
-      "Unable to parse Header Type 'asd'\nAt: fixtures/controllers/invalidHeaderController.ts:6:6.\nThis was caused by 'TsoaResponse<404, void, 'asd'>' \n in 'InvalidHeaderTestController.getWithInvalidHeader'",
+      "Unable to parse Header Type 'Header names must be of type string'\nAt: fixtures/controllers/invalidHeaderController.ts:6:6.\nThis was caused by 'TsoaResponse<404, void, 'Header names must be of type string'>' \n in 'InvalidHeaderTestController.getWithInvalidHeader'",
     );
 
     expect(() => {
       new MetadataGenerator('./fixtures/controllers/incorrectResponseHeaderController.ts').Generate();
-    }).to.throw("Unable to parse Header Type null\nAt: fixtures/controllers/incorrectResponseHeaderController.ts:4:4.\nThis was caused by 'Response<null, null>(200)'");
+    }).to.throw("Unable to parse Header Type any\nAt: fixtures/controllers/incorrectResponseHeaderController.ts:4:4.\nThis was caused by 'Response<null, any>(200)'");
   });
 
   it('should generate a path description from jsdoc comment', () => {

diff --git a/tests/unit/swagger/schemaDetails.spec.ts b/tests/unit/swagger/schemaDetails.spec.ts
@@ -197,12 +197,13 @@ describe('Schema details generation', () => {
                   description: 'a link string',
                 },
                 LinkB: {
-                  type: 'string',
-                  description: 'b link str',
+                  type: 'array',
+                  items: { type: 'string' },
+                  description: 'b link str[]',
                 },
                 LinkC: {
-                  type: 'number',
-                  description: 'c link number, optional',
+                  type: 'string',
+                  description: 'c link string, optional',
                 },
               });
             });
@@ -214,7 +215,8 @@ describe('Schema details generation', () => {
                 description: undefined,
               },
               linkB: {
-                type: 'string',
+                type: 'array',
+                items: { type: 'string' },
                 description: undefined,
               },
               linkOpt: {
@@ -224,15 +226,16 @@ describe('Schema details generation', () => {
             });
             expect(responseSpec.paths['/ResponseHeader/ResponseWithObject'].get?.responses?.[200]?.headers).to.deep.eq({
               linkC: {
-                type: 'number',
+                type: 'string',
                 description: undefined,
               },
               linkD: {
-                type: 'string',
+                type: 'array',
+                items: { type: 'string' },
                 description: undefined,
               },
               linkOpt: {
-                type: 'number',
+                type: 'string',
                 description: undefined,
               },
             });
@@ -242,7 +245,8 @@ describe('Schema details generation', () => {
                 description: undefined,
               },
               linkF: {
-                type: 'number',
+                type: 'array',
+                items: { type: 'string' },
                 description: undefined,
               },
               linkOpt: {
@@ -265,12 +269,13 @@ describe('Schema details generation', () => {
                   description: 'a common link string',
                 },
                 CommonLinkB: {
-                  type: 'string',
-                  description: 'b common link str',
+                  type: 'array',
+                  items: { type: 'string' },
+                  description: 'b common link str[]',
                 },
                 CommonLinkC: {
-                  type: 'number',
-                  description: 'c common link number, optional',
+                  type: 'string',
+                  description: 'c common link string, optional',
                 },
               });
             });
@@ -288,7 +293,8 @@ describe('Schema details generation', () => {
                   description: undefined,
                 },
                 objectB: {
-                  type: 'number',
+                  type: 'array',
+                  items: { type: 'string' },
                   description: undefined,
                 },
                 objectC: {

diff --git a/tests/unit/swagger/schemaDetails3.spec.ts b/tests/unit/swagger/schemaDetails3.spec.ts
@@ -455,7 +455,7 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
               },
               linkB: {
                 required: true,
-                schema: { type: 'string' },
+                schema: { type: 'array', items: { type: 'string' } },
                 description: undefined,
               },
               linkOpt: {
@@ -467,17 +467,17 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
             expect(responseSpec.paths['/ResponseHeader/ResponseWithObject'].get?.responses?.[200]?.headers).to.deep.eq({
               linkC: {
                 required: true,
-                schema: { type: 'number', format: 'double' },
+                schema: { type: 'string' },
                 description: undefined,
               },
               linkD: {
                 required: true,
-                schema: { type: 'string' },
+                schema: { type: 'array', items: { type: 'string' } },
                 description: undefined,
               },
               linkOpt: {
                 required: false,
-                schema: { type: 'number', format: 'double' },
+                schema: { type: 'string' },
                 description: undefined,
               },
             });
@@ -489,7 +489,7 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
               },
               linkF: {
                 required: true,
-                schema: { type: 'number', format: 'double' },
+                schema: { type: 'array', items: { type: 'string' } },
                 description: undefined,
               },
               linkOpt: {
@@ -530,7 +530,7 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
                 },
                 objectB: {
                   required: true,
-                  schema: { type: 'number', format: 'double' },
+                  schema: { type: 'array', items: { type: 'string' } },
                   description: undefined,
                 },
                 objectC: {