Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generics: Use %-encoded escaped names to prevent clashes #475

Closed
wants to merge 1 commit into from
Closed

Generics: Use %-encoded escaped names to prevent clashes #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 41 additions & 92 deletions src/metadataGeneration/typeResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,25 +190,10 @@ export class TypeResolver {
this.typeArgumentsToContext(typeReference, typeReference.typeName, this.context);
}

referenceType = this.getReferenceType(typeReference.typeName as ts.EntityName, this.extractEnum, typeReference.typeArguments);
referenceType = this.getReferenceType(typeReference);

this.current.AddReferenceType(referenceType);

// We do a hard assert in the test mode so we can catch bad ref names (https://github.com/lukeautry/tsoa/issues/398).
// The goal is to avoid producing these names before the code is ever merged to master (via extensive test coverage)
// and therefore this validation does not have to run for the users
if (process.env.NODE_ENV === 'tsoa_test') {
// This regex allows underscore, hyphen, and period since those are valid in SwaggerEditor
const symbolsRegex = /[!$%^&*()+|~=`{}\[\]:";'<>?,\/]/;
if (symbolsRegex.test(referenceType.refName)) {
throw new Error(
`Problem with creating refName ${referenceType.refName} since we should not allow symbols in ref names ` +
`because it would cause invalid swagger.yaml to be created. This is due to the swagger rule ` +
`"ref values must be RFC3986-compliant percent-encoded URIs."`,
);
}
}

return referenceType;
}

Expand Down Expand Up @@ -340,30 +325,38 @@ export class TypeResolver {
} as Tsoa.EnumerateType;
}

private getReferenceType(type: ts.EntityName, extractEnum = true, genericTypes?: ts.NodeArray<ts.TypeNode>): Tsoa.ReferenceType {
const typeName = this.resolveFqTypeName(type);
const refNameWithGenerics = this.getTypeName(typeName, genericTypes);
private getReferenceType(node: ts.TypeReferenceNode | ts.ExpressionWithTypeArguments): Tsoa.ReferenceType {
let type: ts.EntityName;
if (ts.isTypeReferenceNode(node)) {
type = node.typeName;
} else if (ts.isExpressionWithTypeArguments(node)) {
type = node.expression as ts.EntityName;
} else {
throw new GenerateMetadataError(`Can't resolve Reference type.`);
}

const name = this.contextualizedName(node.getText());

try {
const existingType = localReferenceTypeCache[refNameWithGenerics];
const existingType = localReferenceTypeCache[name];
if (existingType) {
return existingType;
}

const referenceEnumType = this.getEnumerateType(type, true) as Tsoa.ReferenceType;
if (referenceEnumType) {
localReferenceTypeCache[refNameWithGenerics] = referenceEnumType;
localReferenceTypeCache[name] = referenceEnumType;
return referenceEnumType;
}

if (inProgressTypes[refNameWithGenerics]) {
return this.createCircularDependencyResolver(refNameWithGenerics);
if (inProgressTypes[name]) {
return this.createCircularDependencyResolver(name);
}

inProgressTypes[refNameWithGenerics] = true;
inProgressTypes[name] = true;

const modelType = this.getModelTypeDeclaration(type);
const properties = this.getModelProperties(modelType, genericTypes);
const properties = this.getModelProperties(modelType);
const additionalProperties = this.getModelAdditionalProperties(modelType);
const inheritedProperties = this.getModelInheritedProperties(modelType) || [];
const example = this.getNodeExample(modelType);
Expand All @@ -373,87 +366,43 @@ export class TypeResolver {
dataType: 'refObject',
description: this.getNodeDescription(modelType),
properties: inheritedProperties,
refName: refNameWithGenerics,
refName: this.getRefTypeName(name),
} as Tsoa.ReferenceType;

referenceType.properties = (referenceType.properties as Tsoa.Property[]).concat(properties);
localReferenceTypeCache[refNameWithGenerics] = referenceType;
localReferenceTypeCache[name] = referenceType;

if (example) {
referenceType.example = example;
}
return referenceType;
} catch (err) {
// tslint:disable-next-line:no-console
console.error(`There was a problem resolving type of '${this.getTypeName(typeName, genericTypes)}'.`);
console.error(`There was a problem resolving type of '${name}'.`);
throw err;
}
}

private resolveFqTypeName(type: ts.EntityName): string {
if (type.kind === ts.SyntaxKind.Identifier) {
return (type as ts.Identifier).text;
}

const qualifiedType = type as ts.QualifiedName;
return this.resolveFqTypeName(qualifiedType.left) + '.' + (qualifiedType.right as ts.Identifier).text;
}

private getTypeName(typeName: string, genericTypes?: ts.NodeArray<ts.TypeNode>): string {
if (!genericTypes || !genericTypes.length) {
return typeName;
}

const resolvedName = genericTypes
.map(genericType => this.context[genericType.getText()] || genericType)
.reduce(
(acc, generic) => {
if (ts.isTypeReferenceNode(generic) && generic.typeArguments && generic.typeArguments.length > 0) {
const typeNameSection = this.getTypeName(generic.typeName.getText(), generic.typeArguments);
acc.push(typeNameSection);
return acc;
} else {
const typeNameSection = this.getAnyTypeName(generic);
acc.push(typeNameSection);
return acc;
}
},
[] as string[],
);

const finalName = typeName + resolvedName.join('');

return finalName;
private getRefTypeName(name: string): string {
return encodeURIComponent(
name
.replace(/<|>/g, '_')
.replace(/ /g, '')
.replace(/,/g, '.')
.replace(/\'(.*)\'|\"(.*)\'/g, '$1')
.replace(/&/g, '~AND')
.replace(/\[\]/g, 'Array'),
);
}

private getAnyTypeName(typeNode: ts.TypeNode): string {
const primitiveType = syntaxKindMap[typeNode.kind];
if (primitiveType) {
return primitiveType;
}

if (typeNode.kind === ts.SyntaxKind.ArrayType) {
const arrayType = typeNode as ts.ArrayTypeNode;
return this.getAnyTypeName(arrayType.elementType) + 'Array';
}

if (typeNode.kind === ts.SyntaxKind.UnionType) {
return 'object';
}

if (typeNode.kind !== ts.SyntaxKind.TypeReference) {
throw new GenerateMetadataError(`Unknown type: ${ts.SyntaxKind[typeNode.kind]}.`);
}

const typeReference = typeNode as ts.TypeReferenceNode;
try {
return (typeReference.typeName as ts.Identifier).text;
} catch (e) {
// idk what would hit this? probably needs more testing
// tslint:disable-next-line:no-console
console.error(e);
return typeNode.toString();
}
private contextualizedName(name: string): string {
return Object.entries(this.context).reduce((acc, [key, entry]) => {
return acc
.replace(new RegExp(`<\s*${key}\s*>`, 'g'), `<${entry.getText()}>`)
.replace(new RegExp(`<\s*${key}\s*,`, 'g'), `<${entry.getText()},`)
.replace(new RegExp(`,\s*${key}\s*>`, 'g'), `,${entry.getText()}>`)
.replace(new RegExp(`<\s*${key}\s*<`, 'g'), `<${entry.getText()}<`);
}, name);
}

private createCircularDependencyResolver(refName: string) {
Expand Down Expand Up @@ -587,7 +536,7 @@ export class TypeResolver {
return modelTypes[0];
}

private getModelProperties(node: UsableDeclaration, genericTypes?: ts.NodeArray<ts.TypeNode>): Tsoa.Property[] {
private getModelProperties(node: UsableDeclaration): Tsoa.Property[] {
const isIgnored = (e: ts.TypeElement | ts.ClassElement) => {
const ignore = isExistJSDocTag(e, tag => tag.tagName.text === 'ignore');
return ignore;
Expand Down Expand Up @@ -766,7 +715,7 @@ export class TypeResolver {
resetCtx = this.typeArgumentsToContext(t, baseEntityName, this.context);
}

const referenceType = this.getReferenceType(baseEntityName);
const referenceType = this.getReferenceType(t);
if (referenceType.properties) {
referenceType.properties.forEach(property => properties.push(property));
}
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/controllers/getController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class GetTestController extends Controller {
@Query() stringParam: string,
@Query() numberParam: number,
@Query() optionalStringParam = '',
) {
): Promise<TestModel> {
const model = new ModelService().getModel();
model.optionalString = optionalStringParam;
model.numberValue = numberPathParam;
Expand Down
32 changes: 16 additions & 16 deletions tests/unit/swagger/definitionsGeneration/definitions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,19 +391,19 @@ describe('Definition generation', () => {
expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`);
},
genericMultiNested: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequestGenericRequestTypeAliasModel1', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequest_GenericRequest_TypeAliasModel1__', `for property ${propertyName}.$ref`);
},
genericNestedArrayKeyword1: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequestArrayTypeAliasModel1', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequest_Array_TypeAliasModel1__', `for property ${propertyName}.$ref`);
},
genericNestedArrayCharacter1: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequestTypeAliasModel1Array', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequest_TypeAliasModel1Array_', `for property ${propertyName}.$ref`);
},
genericNestedArrayKeyword2: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequestArrayTypeAliasModel2', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequest_Array_TypeAliasModel2__', `for property ${propertyName}.$ref`);
},
genericNestedArrayCharacter2: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequestTypeAliasModel2Array', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/definitions/GenericRequest_TypeAliasModel2Array_', `for property ${propertyName}.$ref`);
},
and: (propertyName, propertySchema) => {
expect(propertySchema.type).to.eq('object', `for property ${propertyName}`);
Expand Down Expand Up @@ -679,7 +679,7 @@ describe('Definition generation', () => {
});

it('should generate different definitions for a generic model', () => {
const genericDefinition = getValidatedDefinition('GenericModelTestModel', currentSpec).properties;
const genericDefinition = getValidatedDefinition('GenericModel_TestModel_', currentSpec).properties;

if (!genericDefinition) {
throw new Error(`There were no properties on model.`);
Expand All @@ -693,7 +693,7 @@ describe('Definition generation', () => {
expect(property.$ref).to.equal('#/definitions/TestModel');
});
it('should generate different definitions for a generic model array', () => {
const definition = getValidatedDefinition('GenericModelTestModelArray', currentSpec).properties;
const definition = getValidatedDefinition('GenericModel_TestModelArray_', currentSpec).properties;

if (!definition) {
throw new Error(`There were no properties on model.`);
Expand All @@ -715,7 +715,7 @@ describe('Definition generation', () => {
expect((property.items as Swagger.Schema).$ref).to.equal('#/definitions/TestModel');
});
it('should generate different definitions for a generic primitive', () => {
const definition = getValidatedDefinition('GenericModelstring', currentSpec).properties;
const definition = getValidatedDefinition('GenericModel_string_', currentSpec).properties;

if (!definition) {
throw new Error(`There were no properties on model.`);
Expand All @@ -729,7 +729,7 @@ describe('Definition generation', () => {
expect(property.type).to.equal('string');
});
it('should generate different definitions for a generic primitive array', () => {
const definition = getValidatedDefinition('GenericModelstringArray', currentSpec).properties;
const definition = getValidatedDefinition('GenericModel_stringArray_', currentSpec).properties;

if (!definition) {
throw new Error(`There were no properties on model.`);
Expand All @@ -751,33 +751,33 @@ describe('Definition generation', () => {
expect((property.items as Swagger.Schema).type).to.equal('string');
});
it('should propagate generics', () => {
const definition = getValidatedDefinition('GenericModelTestModelArray', currentSpec).properties;
const definition = getValidatedDefinition('GenericModel_TestModelArray_', currentSpec).properties;

expect(definition!.result).to.deep.equal({ items: { $ref: '#/definitions/TestModel' }, type: 'array', description: undefined, format: undefined, default: undefined });
expect(definition!.union).to.deep.equal({ type: 'object', description: undefined, format: undefined, default: undefined, 'x-nullable': true });
expect(definition!.nested).to.deep.equal({ $ref: '#/definitions/GenericRequestTestModelArray', description: undefined, format: undefined, 'x-nullable': true });
expect(definition!.nested).to.deep.equal({ $ref: '#/definitions/GenericRequest_TestModelArray_', description: undefined, format: undefined, 'x-nullable': true });

const ref = getValidatedDefinition('GenericRequestTestModelArray', currentSpec).properties;
const ref = getValidatedDefinition('GenericRequest_TestModelArray_', currentSpec).properties;
expect(ref!.name).to.deep.equal({ type: 'string', description: undefined, format: undefined, default: undefined });
expect(ref!.value).to.deep.equal({ items: { $ref: '#/definitions/TestModel' }, type: 'array', description: undefined, format: undefined, default: undefined });
});
it('should not propagate dangling context', () => {
const definition = getValidatedDefinition('DanglingContextnumber', currentSpec).properties;
const definition = getValidatedDefinition('DanglingContext_number_', currentSpec).properties;

expect(definition!.number).to.deep.equal({ type: 'number', format: 'double', description: undefined, default: undefined });
expect(definition!.shouldBeString!.$ref).to.deep.equal('#/definitions/TSameNameDifferentValue');
});
it('should check heritage clauses for type args', () => {
const definition = getValidatedDefinition('GenericModelTestModelArray', currentSpec).properties;
const definition = getValidatedDefinition('GenericModel_TestModelArray_', currentSpec).properties;

expect(definition!.heritageCheck).to.deep.equal({
$ref: '#/definitions/ThingContainerWithTitleTestModelArray',
$ref: '#/definitions/ThingContainerWithTitle_TestModelArray_',
description: undefined,
format: undefined,
'x-nullable': true,
});

const ref = getValidatedDefinition('ThingContainerWithTitleTestModelArray', currentSpec).properties;
const ref = getValidatedDefinition('ThingContainerWithTitle_TestModelArray_', currentSpec).properties;
expect(ref!.title).to.deep.equal({ type: 'string', description: undefined, format: undefined, default: undefined });
expect(ref!.t).to.deep.equal({
default: undefined,
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/swagger/schemaDetails3.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,19 +428,19 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
expect(propertySchema.nullable).to.eq(true, `for property ${propertyName}.nullable`);
},
genericMultiNested: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequestGenericRequestTypeAliasModel1', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequest_GenericRequest_TypeAliasModel1__', `for property ${propertyName}.$ref`);
},
genericNestedArrayKeyword1: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequestArrayTypeAliasModel1', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequest_Array_TypeAliasModel1__', `for property ${propertyName}.$ref`);
},
genericNestedArrayCharacter1: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequestTypeAliasModel1Array', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequest_TypeAliasModel1Array_', `for property ${propertyName}.$ref`);
},
genericNestedArrayKeyword2: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequestArrayTypeAliasModel2', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequest_Array_TypeAliasModel2__', `for property ${propertyName}.$ref`);
},
genericNestedArrayCharacter2: (propertyName, propertySchema) => {
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequestTypeAliasModel2Array', `for property ${propertyName}.$ref`);
expect(propertySchema.$ref).to.eq('#/components/schemas/GenericRequest_TypeAliasModel2Array_', `for property ${propertyName}.$ref`);
},
and: (propertyName, propertySchema) => {
expect(propertySchema).to.deep.include(
Expand Down