diff --git a/README.md b/README.md index dc8fc753..16d2527a 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,8 @@ Your model, courtesy of `package:angel_serialize`: ```dart library angel_orm.test.models.car; -import 'package:angel_framework/common.dart'; +import 'package:angel_migration/angel_migration.dart'; +import 'package:angel_model/angel_model.dart'; import 'package:angel_orm/angel_orm.dart'; import 'package:angel_serialize/angel_serialize.dart'; part 'car.g.dart'; diff --git a/angel_orm/CHANGELOG.md b/angel_orm/CHANGELOG.md index fbfa6564..57e7d09a 100644 --- a/angel_orm/CHANGELOG.md +++ b/angel_orm/CHANGELOG.md @@ -1,3 +1,8 @@ +# 2.0.0-dev.15 +* Remove `Column.defaultValue`. +* Deprecate `toSql` and `sanitizeExpression`. +* Refactor builders so that strings are passed through + # 2.0.0-dev.14 * Remove obsolete `@belongsToMany`. diff --git a/angel_orm/example/main.dart b/angel_orm/example/main.dart index ad390cfb..7831bb03 100644 --- a/angel_orm/example/main.dart +++ b/angel_orm/example/main.dart @@ -22,9 +22,12 @@ class _FakeExecutor extends QueryExecutor { const _FakeExecutor(); @override - Future> query(String query, [returningFields]) async { + Future> query( + String query, Map substitutionValues, + [returningFields]) async { var now = new DateTime.now(); - print('_FakeExecutor received query: $query'); + print( + '_FakeExecutor received query: $query and values: $substitutionValues'); return [ [1, 'Rich', 'Person', 100000.0, now, now] ]; @@ -50,8 +53,14 @@ class EmployeeQuery extends Query { @override final QueryValues values = new MapQueryValues(); + EmployeeQueryWhere _where; + + EmployeeQuery() { + _where = new EmployeeQueryWhere(this); + } + @override - final EmployeeQueryWhere where = new EmployeeQueryWhere(); + EmployeeQueryWhere get where => _where; @override String get tableName => 'employees'; @@ -60,6 +69,9 @@ class EmployeeQuery extends Query { List get fields => ['id', 'first_name', 'last_name', 'salary', 'created_at', 'updated_at']; + @override + EmployeeQueryWhere newWhereClause() => new EmployeeQueryWhere(this); + @override Employee deserialize(List row) { return new Employee( @@ -73,26 +85,28 @@ class EmployeeQuery extends Query { } class EmployeeQueryWhere extends QueryWhere { + EmployeeQueryWhere(EmployeeQuery query) + : id = new NumericSqlExpressionBuilder(query, 'id'), + firstName = new StringSqlExpressionBuilder(query, 'first_name'), + lastName = new StringSqlExpressionBuilder(query, 'last_name'), + salary = new NumericSqlExpressionBuilder(query, 'salary'), + createdAt = new DateTimeSqlExpressionBuilder(query, 'created_at'), + updatedAt = new DateTimeSqlExpressionBuilder(query, 'updated_at'); + @override Iterable get expressionBuilders { return [id, firstName, lastName, salary, createdAt, updatedAt]; } - final NumericSqlExpressionBuilder id = - new NumericSqlExpressionBuilder('id'); + final NumericSqlExpressionBuilder id; - final StringSqlExpressionBuilder firstName = - new StringSqlExpressionBuilder('first_name'); + final StringSqlExpressionBuilder firstName; - final StringSqlExpressionBuilder lastName = - new StringSqlExpressionBuilder('last_name'); + final StringSqlExpressionBuilder lastName; - final NumericSqlExpressionBuilder salary = - new NumericSqlExpressionBuilder('salary'); + final NumericSqlExpressionBuilder salary; - final DateTimeSqlExpressionBuilder createdAt = - new DateTimeSqlExpressionBuilder('created_at'); + final DateTimeSqlExpressionBuilder createdAt; - final DateTimeSqlExpressionBuilder updatedAt = - new DateTimeSqlExpressionBuilder('updated_at'); + final DateTimeSqlExpressionBuilder updatedAt; } diff --git a/angel_orm/lib/src/annotations.dart b/angel_orm/lib/src/annotations.dart index e2f735b0..ef2c3c1e 100644 --- a/angel_orm/lib/src/annotations.dart +++ b/angel_orm/lib/src/annotations.dart @@ -2,12 +2,12 @@ const Orm orm = const Orm(); class Orm { /// The name of the table to query. - /// + /// /// Inferred if not present. final String tableName; - + /// Whether to generate migrations for this model. - /// + /// /// Defaults to [:true:]. final bool generateMigrations; diff --git a/angel_orm/lib/src/builder.dart b/angel_orm/lib/src/builder.dart index 91a0dbb5..736682a9 100644 --- a/angel_orm/lib/src/builder.dart +++ b/angel_orm/lib/src/builder.dart @@ -6,7 +6,9 @@ import 'query.dart'; final DateFormat dateYmd = new DateFormat('yyyy-MM-dd'); final DateFormat dateYmdHms = new DateFormat('yyyy-MM-dd HH:mm:ss'); -/// Cleans an input SQL expression of common SQL injection points. +/// The ORM prefers using substitution values, which allow for prepared queries, +/// and prevent SQL injection attacks. +@deprecated String sanitizeExpression(String unsafe) { var buf = new StringBuffer(); var scanner = new StringScanner(unsafe); @@ -30,7 +32,13 @@ String sanitizeExpression(String unsafe) { } abstract class SqlExpressionBuilder { - String get columnName; + final Query query; + final String columnName; + String _substitution; + + SqlExpressionBuilder(this.query, this.columnName); + + String get substitution => _substitution ??= query.reserveName(columnName); bool get hasValue; @@ -46,14 +54,14 @@ abstract class SqlExpressionBuilder { } class NumericSqlExpressionBuilder - implements SqlExpressionBuilder { - final String columnName; + extends SqlExpressionBuilder { bool _hasValue = false; String _op = '='; String _raw; T _value; - NumericSqlExpressionBuilder(this.columnName); + NumericSqlExpressionBuilder(Query query, String columnName) + : super(query, columnName); @override bool get hasValue => _hasValue; @@ -129,20 +137,25 @@ class NumericSqlExpressionBuilder } } -class StringSqlExpressionBuilder implements SqlExpressionBuilder { - final String columnName; +class StringSqlExpressionBuilder extends SqlExpressionBuilder { bool _hasValue = false; String _op = '=', _raw, _value; - StringSqlExpressionBuilder(this.columnName); + StringSqlExpressionBuilder(Query query, String columnName) + : super(query, columnName); @override bool get hasValue => _hasValue; + String get lowerName => '${substitution}_lower'; + + String get upperName => '${substitution}_upper'; + bool _change(String op, String value) { _raw = null; _op = op; _value = value; + query.substitutionValues[substitution] = _value; return _hasValue = true; } @@ -150,8 +163,7 @@ class StringSqlExpressionBuilder implements SqlExpressionBuilder { String compile() { if (_raw != null) return _raw; if (_value == null) return null; - var v = toSql(_value); - return "$_op $v"; + return "$_op @$substitution"; } void isEmpty() => equals(''); @@ -164,48 +176,67 @@ class StringSqlExpressionBuilder implements SqlExpressionBuilder { _change('!=', value); } - void like(String value) { - _change('LIKE', value); + /// Builds a `LIKE` predicate. + /// + /// To prevent injections, the [pattern] is called with a name that + /// will be escaped by the underlying [QueryExecutor]. + /// + /// Example: + /// ```dart + /// carNameBuilder.like((name) => 'Mazda %$name%'); + /// ``` + void like(String Function(String) pattern) { + _raw = 'LIKE \'' + pattern('@$substitution') + '\''; + query.substitutionValues[substitution] = _value; + _hasValue = true; } @override void isBetween(String lower, String upper) { - var l = sanitizeExpression(lower), u = sanitizeExpression(upper); - _raw = "BETWEEN '$l' AND '$u'"; + query.substitutionValues[lowerName] = lower; + query.substitutionValues[upperName] = upper; + _raw = "BETWEEN @$lowerName AND @$upperName"; _hasValue = true; } @override void isNotBetween(String lower, String upper) { - var l = sanitizeExpression(lower), u = sanitizeExpression(upper); - _raw = "NOT BETWEEN '$l' AND '$u'"; + query.substitutionValues[lowerName] = lower; + query.substitutionValues[upperName] = upper; + _raw = "NOT BETWEEN @$lowerName AND @$upperName"; _hasValue = true; } + String _in(Iterable values) { + return 'IN (' + + values.map((v) { + var name = query.reserveName('${columnName}_in_value'); + query.substitutionValues[name] = v; + return '@$name'; + }).join(', ') + + ')'; + } + @override void isIn(Iterable values) { - _raw = 'IN (' + - values.map(sanitizeExpression).map((s) => "'$s'").join(', ') + - ')'; + _raw = _in(values); _hasValue = true; } @override void isNotIn(Iterable values) { - _raw = 'NOT IN (' + - values.map(sanitizeExpression).map((s) => "'$s'").join(', ') + - ')'; + _raw = 'NOT ' + _in(values); _hasValue = true; } } -class BooleanSqlExpressionBuilder implements SqlExpressionBuilder { - final String columnName; +class BooleanSqlExpressionBuilder extends SqlExpressionBuilder { bool _hasValue = false; String _op = '=', _raw; bool _value; - BooleanSqlExpressionBuilder(this.columnName); + BooleanSqlExpressionBuilder(Query query, String columnName) + : super(query, columnName); @override bool get hasValue => _hasValue; @@ -258,28 +289,36 @@ class BooleanSqlExpressionBuilder implements SqlExpressionBuilder { } } -class DateTimeSqlExpressionBuilder implements SqlExpressionBuilder { - final NumericSqlExpressionBuilder year = - new NumericSqlExpressionBuilder('year'), - month = new NumericSqlExpressionBuilder('month'), - day = new NumericSqlExpressionBuilder('day'), - hour = new NumericSqlExpressionBuilder('hour'), - minute = new NumericSqlExpressionBuilder('minute'), - second = new NumericSqlExpressionBuilder('second'); - final String columnName; +class DateTimeSqlExpressionBuilder extends SqlExpressionBuilder { + NumericSqlExpressionBuilder _year, _month, _day, _hour, _minute, _second; + String _raw; - DateTimeSqlExpressionBuilder(this.columnName); + DateTimeSqlExpressionBuilder(Query query, String columnName) + : super(query, columnName); + + NumericSqlExpressionBuilder get year => + _year ??= new NumericSqlExpressionBuilder(query, 'year'); + NumericSqlExpressionBuilder get month => + _month ??= new NumericSqlExpressionBuilder(query, 'month'); + NumericSqlExpressionBuilder get day => + _day ??= new NumericSqlExpressionBuilder(query, 'day'); + NumericSqlExpressionBuilder get hour => + _hour ??= new NumericSqlExpressionBuilder(query, 'hour'); + NumericSqlExpressionBuilder get minute => + _minute ??= new NumericSqlExpressionBuilder(query, 'minute'); + NumericSqlExpressionBuilder get second => + _second ??= new NumericSqlExpressionBuilder(query, 'second'); @override bool get hasValue => _raw?.isNotEmpty == true || - year.hasValue || - month.hasValue || - day.hasValue || - hour.hasValue || - minute.hasValue || - second.hasValue; + _year?.hasValue == true || + _month?.hasValue == true || + _day?.hasValue == true || + _hour?.hasValue == true || + _minute?.hasValue == true || + _second?.hasValue == true; bool _change(String _op, DateTime dt, bool time) { var dateString = time ? dateYmdHms.format(dt) : dateYmd.format(dt); @@ -345,12 +384,17 @@ class DateTimeSqlExpressionBuilder implements SqlExpressionBuilder { String compile() { if (_raw?.isNotEmpty == true) return _raw; List parts = []; - if (year.hasValue) parts.add('YEAR($columnName) ${year.compile()}'); - if (month.hasValue) parts.add('MONTH($columnName) ${month.compile()}'); - if (day.hasValue) parts.add('DAY($columnName) ${day.compile()}'); - if (hour.hasValue) parts.add('HOUR($columnName) ${hour.compile()}'); - if (minute.hasValue) parts.add('MINUTE($columnName) ${minute.compile()}'); - if (second.hasValue) parts.add('SECOND($columnName) ${second.compile()}'); + if (year?.hasValue == true) + parts.add('YEAR($columnName) ${year.compile()}'); + if (month?.hasValue == true) + parts.add('MONTH($columnName) ${month.compile()}'); + if (day?.hasValue == true) parts.add('DAY($columnName) ${day.compile()}'); + if (hour?.hasValue == true) + parts.add('HOUR($columnName) ${hour.compile()}'); + if (minute?.hasValue == true) + parts.add('MINUTE($columnName) ${minute.compile()}'); + if (second?.hasValue == true) + parts.add('SECOND($columnName) ${second.compile()}'); return parts.isEmpty ? null : parts.join(' AND '); } diff --git a/angel_orm/lib/src/migration.dart b/angel_orm/lib/src/migration.dart index d34d8e0e..40615282 100644 --- a/angel_orm/lib/src/migration.dart +++ b/angel_orm/lib/src/migration.dart @@ -26,15 +26,11 @@ class Column { /// Specifies what kind of index this column is, if any. final IndexType indexType; - /// The default value of this field. - final defaultValue; - const Column( {this.isNullable: true, this.length, this.type, - this.indexType: IndexType.none, - this.defaultValue}); + this.indexType: IndexType.none}); } class PrimaryKey extends Column { diff --git a/angel_orm/lib/src/query.dart b/angel_orm/lib/src/query.dart index dbde9da2..97749160 100644 --- a/angel_orm/lib/src/query.dart +++ b/angel_orm/lib/src/query.dart @@ -7,6 +7,8 @@ bool isAscii(int ch) => ch >= $nul && ch <= $del; /// A base class for objects that compile to SQL queries, typically within an ORM. abstract class QueryBase { + final Map substitutionValues = {}; + /// The list of fields returned by this query. /// /// If it's `null`, then this query will perform a `SELECT *`. @@ -22,7 +24,9 @@ abstract class QueryBase { Future> get(QueryExecutor executor) async { var sql = compile(); - return executor.query(sql).then((it) => it.map(deserialize).toList()); + return executor + .query(sql, substitutionValues) + .then((it) => it.map(deserialize).toList()); } Future getOne(QueryExecutor executor) { @@ -47,6 +51,9 @@ class OrderBy { String compile() => descending ? '$key DESC' : '$key ASC'; } +/// The ORM prefers using substitution values, which allow for prepared queries, +/// and prevent SQL injection attacks. +@deprecated String toSql(Object obj, {bool withQuotes: true}) { if (obj is DateTime) { return withQuotes ? "'${dateYmdHms.format(obj)}'" : dateYmdHms.format(obj); @@ -96,7 +103,9 @@ String toSql(Object obj, {bool withQuotes: true}) { /// A SQL `SELECT` query builder. abstract class Query extends QueryBase { final List _joins = []; + final Map _names = {}; final List _orderBy = []; + String _crossJoin, _groupBy; int _limit, _offset; @@ -113,8 +122,17 @@ abstract class Query extends QueryBase { /// This is usually a generated class. QueryValues get values; + /// Preprends the [tableName] to the [String], [s]. String adornWithTableName(String s) => '$tableName.$s'; + /// Returns a unique version of [name], which will not produce a collision within + /// the context of this [query]. + String reserveName(String name) { + var n = _names[name] ??= 0; + _names[name]++; + return n == 0 ? name : '${name}$n'; + } + /// Makes a new [Where] clause. Where newWhereClause() { throw new UnsupportedError( @@ -258,14 +276,15 @@ abstract class Query extends QueryBase { if (_joins.isEmpty) { return executor - .query(sql, fields.map(adornWithTableName).toList()) + .query( + sql, substitutionValues, fields.map(adornWithTableName).toList()) .then((it) => it.map(deserialize).toList()); } else { return executor.transaction(() async { // TODO: Can this be done with just *one* query? var existing = await get(executor); //var sql = compile(preamble: 'SELECT $tableName.id', withFields: false); - return executor.query(sql).then((_) => existing); + return executor.query(sql, substitutionValues).then((_) => existing); }); } } @@ -275,20 +294,21 @@ abstract class Query extends QueryBase { } Future insert(QueryExecutor executor) { - var sql = values.compileInsert(tableName); + var sql = values.compileInsert(this, tableName); if (sql == null) { throw new StateError('No values have been specified for update.'); } else { return executor - .query(sql, fields.map(adornWithTableName).toList()) + .query( + sql, substitutionValues, fields.map(adornWithTableName).toList()) .then((it) => it.isEmpty ? null : deserialize(it.first)); } } Future> update(QueryExecutor executor) async { var sql = new StringBuffer('UPDATE $tableName '); - var valuesClause = values.compileForUpdate(); + var valuesClause = values.compileForUpdate(this); if (valuesClause == null) { throw new StateError('No values have been specified for update.'); @@ -300,12 +320,14 @@ abstract class Query extends QueryBase { if (_joins.isEmpty) { return executor - .query(sql.toString(), fields.map(adornWithTableName).toList()) + .query(sql.toString(), substitutionValues, + fields.map(adornWithTableName).toList()) .then((it) => it.map(deserialize).toList()); } else { // TODO: Can this be done with just *one* query? return executor - .query(sql.toString(), fields.map(adornWithTableName).toList()) + .query(sql.toString(), substitutionValues, + fields.map(adornWithTableName).toList()) .then((it) => get(executor)); } } @@ -319,7 +341,7 @@ abstract class Query extends QueryBase { abstract class QueryValues { Map toMap(); - String compileInsert(String tableName) { + String compileInsert(Query query, String tableName) { var data = toMap(); if (data.isEmpty) return null; @@ -329,14 +351,17 @@ abstract class QueryValues { for (var entry in data.entries) { if (i++ > 0) b.write(', '); - b.write(toSql(entry.value)); + + var name = query.reserveName(entry.key); + query.substitutionValues[name] = entry.value; + b.write('@$name'); } b.write(')'); return b.toString(); } - String compileForUpdate() { + String compileForUpdate(Query query) { var data = toMap(); if (data.isEmpty) return null; var b = new StringBuffer('SET'); @@ -347,7 +372,10 @@ abstract class QueryValues { b.write(' '); b.write(entry.key); b.write('='); - b.write(toSql(entry.value)); + + var name = query.reserveName(entry.key); + query.substitutionValues[name] = entry.value; + b.write('@$name'); } return b.toString(); } @@ -504,7 +532,9 @@ class JoinOn { abstract class QueryExecutor { const QueryExecutor(); - Future> query(String query, [List returningFields]); + Future> query( + String query, Map substitutionValues, + [List returningFields]); Future transaction(FutureOr f()); } diff --git a/angel_orm/pubspec.yaml b/angel_orm/pubspec.yaml index c0d56d14..f89ea208 100644 --- a/angel_orm/pubspec.yaml +++ b/angel_orm/pubspec.yaml @@ -1,5 +1,5 @@ name: angel_orm -version: 2.0.0-dev.14 +version: 2.0.0-dev.15 description: Runtime support for Angel's ORM. Includes base classes for queries. author: Tobe O homepage: https://github.com/angel-dart/orm diff --git a/angel_orm/test/to_sql_test.dart b/angel_orm/test/to_sql_test.dart deleted file mode 100644 index 8e2a6522..00000000 --- a/angel_orm/test/to_sql_test.dart +++ /dev/null @@ -1,17 +0,0 @@ -import 'package:angel_orm/angel_orm.dart'; -import 'package:test/test.dart'; - -void main() { - test('simple', () { - expect(toSql('ABC _!'), "'ABC _!'"); - }); - - test('ignores null byte', () { - expect(toSql('a\x00bc'), "'abc'"); - }); - - test('unicode', () { - expect(toSql('東'), r"'\u6771'"); - expect(toSql('𐐀'), r"'\U00010400'"); - }); -} diff --git a/angel_orm_generator/lib/src/orm_build_context.dart b/angel_orm_generator/lib/src/orm_build_context.dart index a4bd531c..46808d78 100644 --- a/angel_orm_generator/lib/src/orm_build_context.dart +++ b/angel_orm_generator/lib/src/orm_build_context.dart @@ -67,7 +67,8 @@ Future buildOrmContext( } if (column == null && field.name == 'id' && autoIdAndDateFields == true) { - // TODO: This is only for PostgreSQL!!! + // This is only for PostgreSQL, so implementations without a `serial` type + // must handle it accordingly, of course. column = const Column(type: ColumnType.serial); } diff --git a/angel_orm_generator/test/models/customer.dart b/angel_orm_generator/test/models/customer.dart index 12c25b26..1e87623e 100644 --- a/angel_orm_generator/test/models/customer.dart +++ b/angel_orm_generator/test/models/customer.dart @@ -1,5 +1,6 @@ library angel_orm_generator.test.models.customer; +import 'package:angel_migration/angel_migration.dart'; import 'package:angel_model/angel_model.dart'; import 'package:angel_orm/angel_orm.dart'; import 'package:angel_serialize/angel_serialize.dart'; diff --git a/angel_orm_generator/test/models/foot.dart b/angel_orm_generator/test/models/foot.dart index d6234cba..7d4a6822 100644 --- a/angel_orm_generator/test/models/foot.dart +++ b/angel_orm_generator/test/models/foot.dart @@ -1,5 +1,6 @@ library angel_orm_generator.test.models.foot; +import 'package:angel_migration/angel_migration.dart'; import 'package:angel_model/angel_model.dart'; import 'package:angel_orm/angel_orm.dart'; import 'package:angel_serialize/angel_serialize.dart'; diff --git a/angel_orm_generator/test/models/order.dart b/angel_orm_generator/test/models/order.dart index 2cc012dc..1403aaf1 100644 --- a/angel_orm_generator/test/models/order.dart +++ b/angel_orm_generator/test/models/order.dart @@ -1,5 +1,6 @@ library angel_orm_generator.test.models.order; +import 'package:angel_migration/angel_migration.dart'; import 'package:angel_model/angel_model.dart'; import 'package:angel_orm/angel_orm.dart'; import 'package:angel_serialize/angel_serialize.dart';