From eaea8a71004a86d7dabfc44c6c7574e80b6c0027 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Fri, 14 Oct 2022 14:51:19 +0800 Subject: [PATCH] feat: limit database identifier (#908) --- .../database/src/__tests__/collection.test.ts | 47 +++++++++++++++++++ .../__tests__/fields/belongs-to-field.test.ts | 35 ++++++++++++++ .../fields/belongs-to-many-field.test.ts | 45 ++++++++++++++++++ .../__tests__/fields/has-many-field.test.ts | 22 +++++++++ .../__tests__/fields/has-one-field.test.ts | 21 +++++++++ packages/core/database/src/collection.ts | 14 +++++- packages/core/database/src/database.ts | 4 +- .../database/src/errors/identifier-error.ts | 6 +++ .../database/src/fields/belongs-to-field.ts | 4 ++ .../src/fields/belongs-to-many-field.ts | 11 +++++ packages/core/database/src/fields/field.ts | 1 + .../database/src/fields/has-many-field.ts | 7 ++- .../core/database/src/fields/has-one-field.ts | 10 +++- packages/core/database/src/utils.ts | 9 ++++ 14 files changed, 231 insertions(+), 5 deletions(-) create mode 100644 packages/core/database/src/errors/identifier-error.ts diff --git a/packages/core/database/src/__tests__/collection.test.ts b/packages/core/database/src/__tests__/collection.test.ts index 29ad3859a..a3ac75869 100644 --- a/packages/core/database/src/__tests__/collection.test.ts +++ b/packages/core/database/src/__tests__/collection.test.ts @@ -1,6 +1,7 @@ import { Collection } from '../collection'; import { Database } from '../database'; import { mockDatabase } from './index'; +import { IdentifierError } from '../errors/identifier-error'; describe('collection', () => { let db: Database; @@ -258,4 +259,50 @@ describe('collection sync', () => { expect(tableFields['postId']).toBeDefined(); expect(tableFields['tagId']).toBeDefined(); }); + + test('limit table name length', async () => { + const longName = + 'this_is_a_very_long_table_name_that_should_be_truncated_this_is_a_very_long_table_name_that_should_be_truncated'; + + let error; + + try { + const collection = new Collection( + { + name: longName, + fields: [{ type: 'string', name: 'test' }], + }, + { + database: db, + }, + ); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); + + test('limit field name length', async () => { + const longFieldName = + 'this_is_a_very_long_field_name_that_should_be_truncated_this_is_a_very_long_field_name_that_should_be_truncated'; + + let error; + + try { + const collection = new Collection( + { + name: 'test', + fields: [{ type: 'string', name: longFieldName }], + }, + { + database: db, + }, + ); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); }); diff --git a/packages/core/database/src/__tests__/fields/belongs-to-field.test.ts b/packages/core/database/src/__tests__/fields/belongs-to-field.test.ts index 936b2bdd7..a0b3326a7 100644 --- a/packages/core/database/src/__tests__/fields/belongs-to-field.test.ts +++ b/packages/core/database/src/__tests__/fields/belongs-to-field.test.ts @@ -1,5 +1,6 @@ import { Database } from '../../database'; import { mockDatabase } from '../'; +import { IdentifierError } from '../../errors/identifier-error'; describe('belongs to field', () => { let db: Database; @@ -28,12 +29,16 @@ describe('belongs to field', () => { { type: 'belongsTo', name: 'post' }, ], }); + expect(Comment.model.associations.post).toBeUndefined(); + const Post = db.collection({ name: 'posts', fields: [{ type: 'string', name: 'title' }], }); + const association = Comment.model.associations.post; + expect(Comment.model.associations.post).toBeDefined(); expect(association.foreignKey).toBe('postId'); // @ts-ignore @@ -77,11 +82,41 @@ describe('belongs to field', () => { const association = Comment.model.associations.post; expect(association).toBeDefined(); expect(association.foreignKey).toBe('postKey'); + // @ts-ignore expect(association.targetKey).toBe('key'); expect(Comment.model.rawAttributes['postKey']).toBeDefined(); }); + it('should throw error when foreignKey is too long', async () => { + const Post = db.collection({ + name: 'posts', + fields: [{ type: 'string', name: 'key', unique: true }], + }); + + const longForeignKey = 'a'.repeat(128); + + let error; + + try { + const Comment = db.collection({ + name: 'comments1', + fields: [ + { + type: 'belongsTo', + name: 'post', + targetKey: 'key', + foreignKey: longForeignKey, + }, + ], + }); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); + it('custom name and target', async () => { const Comment = db.collection({ name: 'comments', diff --git a/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts b/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts index c58f21567..e92ce8bda 100644 --- a/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts +++ b/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts @@ -1,5 +1,6 @@ import { mockDatabase } from '../'; import { Database } from '../../database'; +import { IdentifierError } from '../../errors/identifier-error'; describe('belongs to many field', () => { let db: Database; @@ -58,4 +59,48 @@ describe('belongs to many field', () => { expect(PostTag.model.rawAttributes['postId']).toBeDefined(); expect(PostTag.model.rawAttributes['tagId']).toBeDefined(); }); + + it('should throw error when foreignKey is too long', async () => { + const Post = db.collection({ + name: 'posts', + fields: [ + { type: 'string', name: 'name' }, + { type: 'belongsToMany', name: 'tags', foreignKey: 'a'.repeat(128) }, + ], + }); + + let error; + try { + const Tag = db.collection({ + name: 'tags', + fields: [{ type: 'string', name: 'name' }], + }); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); + + it('should throw error when through is too long', async () => { + const Post = db.collection({ + name: 'posts', + fields: [ + { type: 'string', name: 'name' }, + { type: 'belongsToMany', name: 'tags', through: 'a'.repeat(128) }, + ], + }); + + let error; + try { + const Tag = db.collection({ + name: 'tags', + fields: [{ type: 'string', name: 'name' }], + }); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); }); diff --git a/packages/core/database/src/__tests__/fields/has-many-field.test.ts b/packages/core/database/src/__tests__/fields/has-many-field.test.ts index 786154aa5..4a3c23c56 100644 --- a/packages/core/database/src/__tests__/fields/has-many-field.test.ts +++ b/packages/core/database/src/__tests__/fields/has-many-field.test.ts @@ -1,6 +1,7 @@ import { Database } from '../../database'; import { mockDatabase } from '../'; import { makeWatchHost } from 'ts-loader/dist/servicesHost'; +import { IdentifierError } from '../../errors/identifier-error'; describe('has many field', () => { let db: Database; @@ -149,4 +150,25 @@ describe('has many field', () => { Comment.removeField('post'); expect(Comment.model.rawAttributes.postId).toBeUndefined(); }); + + it('should throw error when foreignKey is too long', async () => { + const longForeignKey = 'a'.repeat(64); + + const Post = db.collection({ + name: 'posts', + fields: [{ type: 'hasMany', name: 'comments', foreignKey: longForeignKey }], + }); + + let error; + try { + const Comment = db.collection({ + name: 'comments', + fields: [{ type: 'belongsTo', name: 'post' }], + }); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(IdentifierError); + }); }); diff --git a/packages/core/database/src/__tests__/fields/has-one-field.test.ts b/packages/core/database/src/__tests__/fields/has-one-field.test.ts index ca8a40150..000275429 100644 --- a/packages/core/database/src/__tests__/fields/has-one-field.test.ts +++ b/packages/core/database/src/__tests__/fields/has-one-field.test.ts @@ -1,5 +1,6 @@ import { Database } from '../../database'; import { mockDatabase } from '../'; +import { IdentifierError } from '../../errors/identifier-error'; describe('has many field', () => { let db: Database; @@ -64,4 +65,24 @@ describe('has many field', () => { Profile.removeField('user'); expect(Profile.model.rawAttributes.userId).toBeUndefined(); }); + + it('should throw error when foreignKey is too long', async () => { + const longForeignKey = 'a'.repeat(128); + + const User = db.collection({ + name: 'users', + fields: [{ type: 'hasOne', name: 'profile', foreignKey: longForeignKey }], + }); + + let error; + try { + const Profile = db.collection({ + name: 'profiles', + fields: [{ type: 'belongsTo', name: 'user' }], + }); + } catch (e) { + error = e; + } + expect(error).toBeInstanceOf(IdentifierError); + }); }); diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 3244ce530..21da8c7a3 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -7,13 +7,13 @@ import { QueryInterfaceDropTableOptions, SyncOptions, Transactionable, - Utils + Utils, } from 'sequelize'; import { Database } from './database'; import { Field, FieldOptions } from './fields'; import { Model } from './model'; import { Repository } from './repository'; -import { md5 } from './utils'; +import { checkIdentifier, md5 } from './utils'; export type RepositoryType = typeof Repository; @@ -67,8 +67,11 @@ export class Collection< constructor(options: CollectionOptions, context?: CollectionContext) { super(); + this.checkOptions(options); + this.context = context; this.options = options; + this.bindFieldEventListener(); this.modelInit(); this.setFields(options.fields); @@ -76,6 +79,10 @@ export class Collection< this.setSortable(options.sortable); } + private checkOptions(options: CollectionOptions) { + checkIdentifier(options.name); + } + private sequelizeModelOptions() { const { name, tableName } = this.options; return { @@ -160,6 +167,8 @@ export class Collection< } setField(name: string, options: FieldOptions): Field { + checkIdentifier(name); + const { database } = this.context; const field = database.buildField( @@ -169,6 +178,7 @@ export class Collection< collection: this, }, ); + this.removeField(name); this.fields.set(name, field); this.emit('field.afterAdd', field); diff --git a/packages/core/database/src/database.ts b/packages/core/database/src/database.ts index 2d30d4b2d..a3b338a80 100644 --- a/packages/core/database/src/database.ts +++ b/packages/core/database/src/database.ts @@ -14,7 +14,7 @@ import { Sequelize, SyncOptions, Transactionable, - Utils + Utils, } from 'sequelize'; import { SequelizeStorage, Umzug } from 'umzug'; import { Collection, CollectionOptions, RepositoryType } from './collection'; @@ -361,9 +361,11 @@ export class Database extends EventEmitter implements AsyncEmitter { buildField(options, context: FieldContext) { const { type } = options; const Field = this.fieldTypes.get(type); + if (!Field) { throw Error(`unsupported field type ${type}`); } + return new Field(options, context); } diff --git a/packages/core/database/src/errors/identifier-error.ts b/packages/core/database/src/errors/identifier-error.ts new file mode 100644 index 000000000..d958e07c0 --- /dev/null +++ b/packages/core/database/src/errors/identifier-error.ts @@ -0,0 +1,6 @@ +export class IdentifierError extends Error { + constructor(message: string) { + super(message); + this.name = 'IdentifierError'; + } +} diff --git a/packages/core/database/src/fields/belongs-to-field.ts b/packages/core/database/src/fields/belongs-to-field.ts index cb248b66b..eb210cfba 100644 --- a/packages/core/database/src/fields/belongs-to-field.ts +++ b/packages/core/database/src/fields/belongs-to-field.ts @@ -1,6 +1,7 @@ import { omit } from 'lodash'; import { BelongsToOptions as SequelizeBelongsToOptions, Utils } from 'sequelize'; import { BaseRelationFieldOptions, RelationField } from './relation-field'; +import { checkIdentifier } from '../utils'; export class BelongsToField extends RelationField { static type = 'belongsTo'; @@ -42,10 +43,13 @@ export class BelongsToField extends RelationField { this.options.foreignKey = association.foreignKey; } + checkIdentifier(this.options.foreignKey); + if (!this.options.sourceKey) { // @ts-ignore this.options.sourceKey = association.sourceKey; } + this.collection.addIndex([this.options.foreignKey]); return true; } diff --git a/packages/core/database/src/fields/belongs-to-many-field.ts b/packages/core/database/src/fields/belongs-to-many-field.ts index 4ef70a5d7..e5ea07cc2 100644 --- a/packages/core/database/src/fields/belongs-to-many-field.ts +++ b/packages/core/database/src/fields/belongs-to-many-field.ts @@ -2,6 +2,7 @@ import { omit } from 'lodash'; import { BelongsToManyOptions as SequelizeBelongsToManyOptions, Utils } from 'sequelize'; import { Collection } from '../collection'; import { MultipleRelationFieldOptions, RelationField } from './relation-field'; +import { checkIdentifier } from '../utils'; export class BelongsToManyField extends RelationField { get through() { @@ -23,6 +24,7 @@ export class BelongsToManyField extends RelationField { database.addPendingField(this); return false; } + const through = this.through; let Through: Collection; @@ -33,6 +35,7 @@ export class BelongsToManyField extends RelationField { Through = database.collection({ name: through, }); + Object.defineProperty(Through.model, 'isThrough', { value: true }); } @@ -49,15 +52,23 @@ export class BelongsToManyField extends RelationField { if (!this.options.foreignKey) { this.options.foreignKey = association.foreignKey; } + + checkIdentifier(this.options.foreignKey); + if (!this.options.sourceKey) { this.options.sourceKey = association.sourceKey; } + if (!this.options.otherKey) { this.options.otherKey = association.otherKey; } + + checkIdentifier(this.options.otherKey); + if (!this.options.through) { this.options.through = this.through; } + Through.addIndex([this.options.foreignKey]); Through.addIndex([this.options.otherKey]); return true; diff --git a/packages/core/database/src/fields/field.ts b/packages/core/database/src/fields/field.ts index bc2c4cf9c..fe9f5b0ef 100644 --- a/packages/core/database/src/fields/field.ts +++ b/packages/core/database/src/fields/field.ts @@ -9,6 +9,7 @@ import { } from 'sequelize'; import { Collection } from '../collection'; import { Database } from '../database'; +import { checkIdentifier } from '../utils'; export interface FieldContext { database: Database; diff --git a/packages/core/database/src/fields/has-many-field.ts b/packages/core/database/src/fields/has-many-field.ts index 3f14b4093..faeb09a7f 100644 --- a/packages/core/database/src/fields/has-many-field.ts +++ b/packages/core/database/src/fields/has-many-field.ts @@ -5,10 +5,11 @@ import { ForeignKeyOptions, HasManyOptions, HasManyOptions as SequelizeHasManyOptions, - Utils + Utils, } from 'sequelize'; import { Collection } from '../collection'; import { MultipleRelationFieldOptions, RelationField } from './relation-field'; +import { checkIdentifier } from '../utils'; export interface HasManyFieldOptions extends HasManyOptions { /** @@ -98,6 +99,7 @@ export class HasManyField extends RelationField { as: this.name, foreignKey: this.foreignKey, }); + // inverse relation // this.TargetModel.belongsTo(collection.model); @@ -107,6 +109,9 @@ export class HasManyField extends RelationField { if (!this.options.foreignKey) { this.options.foreignKey = association.foreignKey; } + + checkIdentifier(this.options.foreignKey); + if (!this.options.sourceKey) { // @ts-ignore this.options.sourceKey = association.sourceKey; diff --git a/packages/core/database/src/fields/has-one-field.ts b/packages/core/database/src/fields/has-one-field.ts index 436fc7cf7..a44fc0ac2 100644 --- a/packages/core/database/src/fields/has-one-field.ts +++ b/packages/core/database/src/fields/has-one-field.ts @@ -5,10 +5,11 @@ import { ForeignKeyOptions, HasOneOptions, HasOneOptions as SequelizeHasOneOptions, - Utils + Utils, } from 'sequelize'; import { Collection } from '../collection'; import { BaseRelationFieldOptions, RelationField } from './relation-field'; +import { checkIdentifier } from '../utils'; export interface HasOneFieldOptions extends HasOneOptions { /** @@ -92,21 +93,28 @@ export class HasOneField extends RelationField { database.addPendingField(this); return false; } + const association = collection.model.hasOne(Target, { constraints: false, ...omit(this.options, ['name', 'type', 'target']), as: this.name, foreignKey: this.foreignKey, }); + // 建立关系之后从 pending 列表中删除 database.removePendingField(this); + if (!this.options.foreignKey) { this.options.foreignKey = association.foreignKey; } + + checkIdentifier(this.options.foreignKey); + if (!this.options.sourceKey) { // @ts-ignore this.options.sourceKey = association.sourceKey; } + let tcoll: Collection; if (this.target === collection.name) { tcoll = collection; diff --git a/packages/core/database/src/utils.ts b/packages/core/database/src/utils.ts index b0654011f..a0aa8a4ac 100644 --- a/packages/core/database/src/utils.ts +++ b/packages/core/database/src/utils.ts @@ -1,5 +1,6 @@ import crypto from 'crypto'; import { Model } from './model'; +import { IdentifierError } from './errors/identifier-error'; type HandleAppendsQueryOptions = { templateModel: any; @@ -49,3 +50,11 @@ export async function handleAppendsQuery(options: HandleAppendsQueryOptions) { export function md5(value: string) { return crypto.createHash('md5').update(value).digest('hex'); } + +const MAX_IDENTIFIER_LENGTH = 63; + +export function checkIdentifier(value: string) { + if (value.length > MAX_IDENTIFIER_LENGTH) { + throw new IdentifierError(`Identifier ${value} is too long`); + } +}