use substitutionValues for inserts and strings

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';

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`.
 

angel_orm/example/main.dart

@@ -22,9 +22,12 @@ class _FakeExecutor extends QueryExecutor {
   const _FakeExecutor();
 
   @override
-  Future<List<List>> query(String query, [returningFields]) async {
+  Future<List<List>> query(
+      String query, Map<String, dynamic> 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<Employee, EmployeeQueryWhere> {
   @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<Employee, EmployeeQueryWhere> {
   List<String> 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<Employee, EmployeeQueryWhere> {
 }
 
 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<SqlExpressionBuilder> get expressionBuilders {
     return [id, firstName, lastName, salary, createdAt, updatedAt];
   }
 
-  final NumericSqlExpressionBuilder<int> id =
+  final NumericSqlExpressionBuilder<int> id;
-      new NumericSqlExpressionBuilder<int>('id');
 
-  final StringSqlExpressionBuilder firstName =
+  final StringSqlExpressionBuilder firstName;
-      new StringSqlExpressionBuilder('first_name');
 
-  final StringSqlExpressionBuilder lastName =
+  final StringSqlExpressionBuilder lastName;
-      new StringSqlExpressionBuilder('last_name');
 
-  final NumericSqlExpressionBuilder<double> salary =
+  final NumericSqlExpressionBuilder<double> salary;
-      new NumericSqlExpressionBuilder<double>('salary');
 
-  final DateTimeSqlExpressionBuilder createdAt =
+  final DateTimeSqlExpressionBuilder createdAt;
-      new DateTimeSqlExpressionBuilder('created_at');
 
-  final DateTimeSqlExpressionBuilder updatedAt =
+  final DateTimeSqlExpressionBuilder updatedAt;
-      new DateTimeSqlExpressionBuilder('updated_at');
 }

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;
 

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<T> {
-  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<T> {
 }
 
 class NumericSqlExpressionBuilder<T extends num>
-    implements SqlExpressionBuilder<T> {
+    extends SqlExpressionBuilder<T> {
-  final String columnName;
   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<T extends num>
   }
 }
 
-class StringSqlExpressionBuilder implements SqlExpressionBuilder<String> {
+class StringSqlExpressionBuilder extends SqlExpressionBuilder<String> {
-  final String columnName;
   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> {
   String compile() {
     if (_raw != null) return _raw;
     if (_value == null) return null;
-    var v = toSql(_value);
+    return "$_op @$substitution";
-    return "$_op $v";
   }
 
   void isEmpty() => equals('');
@@ -164,48 +176,67 @@ class StringSqlExpressionBuilder implements SqlExpressionBuilder<String> {
     _change('!=', value);
   }
 
-  void like(String value) {
+  /// Builds a `LIKE` predicate.
-    _change('LIKE', value);
+  ///
+  /// 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);
+    query.substitutionValues[lowerName] = lower;
-    _raw = "BETWEEN '$l' AND '$u'";
+    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);
+    query.substitutionValues[lowerName] = lower;
-    _raw = "NOT BETWEEN '$l' AND '$u'";
+    query.substitutionValues[upperName] = upper;
+    _raw = "NOT BETWEEN @$lowerName AND @$upperName";
     _hasValue = true;
   }
 
+  String _in(Iterable<String> values) {
+    return 'IN (' +
+        values.map((v) {
+          var name = query.reserveName('${columnName}_in_value');
+          query.substitutionValues[name] = v;
+          return '@$name';
+        }).join(', ') +
+        ')';
+  }
+
   @override
   void isIn(Iterable<String> values) {
-    _raw = 'IN (' +
+    _raw = _in(values);
-        values.map(sanitizeExpression).map((s) => "'$s'").join(', ') +
-        ')';
     _hasValue = true;
   }
 
   @override
   void isNotIn(Iterable<String> values) {
-    _raw = 'NOT IN (' +
+    _raw = 'NOT ' + _in(values);
-        values.map(sanitizeExpression).map((s) => "'$s'").join(', ') +
-        ')';
     _hasValue = true;
   }
 }
 
-class BooleanSqlExpressionBuilder implements SqlExpressionBuilder<bool> {
+class BooleanSqlExpressionBuilder extends SqlExpressionBuilder<bool> {
-  final String columnName;
   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<bool> {
   }
 }
 
-class DateTimeSqlExpressionBuilder implements SqlExpressionBuilder<DateTime> {
+class DateTimeSqlExpressionBuilder extends SqlExpressionBuilder<DateTime> {
-  final NumericSqlExpressionBuilder<int> year =
+  NumericSqlExpressionBuilder<int> _year, _month, _day, _hour, _minute, _second;
-          new NumericSqlExpressionBuilder<int>('year'),
+
-      month = new NumericSqlExpressionBuilder<int>('month'),
-      day = new NumericSqlExpressionBuilder<int>('day'),
-      hour = new NumericSqlExpressionBuilder<int>('hour'),
-      minute = new NumericSqlExpressionBuilder<int>('minute'),
-      second = new NumericSqlExpressionBuilder<int>('second');
-  final String columnName;
   String _raw;
 
-  DateTimeSqlExpressionBuilder(this.columnName);
+  DateTimeSqlExpressionBuilder(Query query, String columnName)
+      : super(query, columnName);
+
+  NumericSqlExpressionBuilder<int> get year =>
+      _year ??= new NumericSqlExpressionBuilder(query, 'year');
+  NumericSqlExpressionBuilder<int> get month =>
+      _month ??= new NumericSqlExpressionBuilder(query, 'month');
+  NumericSqlExpressionBuilder<int> get day =>
+      _day ??= new NumericSqlExpressionBuilder(query, 'day');
+  NumericSqlExpressionBuilder<int> get hour =>
+      _hour ??= new NumericSqlExpressionBuilder(query, 'hour');
+  NumericSqlExpressionBuilder<int> get minute =>
+      _minute ??= new NumericSqlExpressionBuilder(query, 'minute');
+  NumericSqlExpressionBuilder<int> get second =>
+      _second ??= new NumericSqlExpressionBuilder(query, 'second');
 
   @override
   bool get hasValue =>
       _raw?.isNotEmpty == true ||
-      year.hasValue ||
+      _year?.hasValue == true ||
-      month.hasValue ||
+      _month?.hasValue == true ||
-      day.hasValue ||
+      _day?.hasValue == true ||
-      hour.hasValue ||
+      _hour?.hasValue == true ||
-      minute.hasValue ||
+      _minute?.hasValue == true ||
-      second.hasValue;
+      _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<DateTime> {
   String compile() {
     if (_raw?.isNotEmpty == true) return _raw;
     List<String> parts = [];
-    if (year.hasValue) parts.add('YEAR($columnName) ${year.compile()}');
+    if (year?.hasValue == true)
-    if (month.hasValue) parts.add('MONTH($columnName) ${month.compile()}');
+      parts.add('YEAR($columnName) ${year.compile()}');
-    if (day.hasValue) parts.add('DAY($columnName) ${day.compile()}');
+    if (month?.hasValue == true)
-    if (hour.hasValue) parts.add('HOUR($columnName) ${hour.compile()}');
+      parts.add('MONTH($columnName) ${month.compile()}');
-    if (minute.hasValue) parts.add('MINUTE($columnName) ${minute.compile()}');
+    if (day?.hasValue == true) parts.add('DAY($columnName) ${day.compile()}');
-    if (second.hasValue) parts.add('SECOND($columnName) ${second.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 ');
   }

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.indexType: IndexType.none});
-      this.defaultValue});
 }
 
 class PrimaryKey extends Column {

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<T> {
+  final Map<String, dynamic> 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<T> {
 
   Future<List<T>> 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<T> 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<T, Where extends QueryWhere> extends QueryBase<T> {
   final List<JoinBuilder> _joins = [];
+  final Map<String, int> _names = {};
   final List<OrderBy> _orderBy = [];
+
   String _crossJoin, _groupBy;
   int _limit, _offset;
 
@@ -113,8 +122,17 @@ abstract class Query<T, Where extends QueryWhere> extends QueryBase<T> {
   /// 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<T, Where extends QueryWhere> extends QueryBase<T> {
 
     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<T, Where extends QueryWhere> extends QueryBase<T> {
   }
 
   Future<T> 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<List<T>> 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<T, Where extends QueryWhere> extends QueryBase<T> {
 
       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<T, Where extends QueryWhere> extends QueryBase<T> {
 abstract class QueryValues {
   Map<String, dynamic> 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<List<List>> query(String query, [List<String> returningFields]);
+  Future<List<List>> query(
+      String query, Map<String, dynamic> substitutionValues,
+      [List<String> returningFields]);
 
   Future<T> transaction<T>(FutureOr<T> f());
 }

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 <thosakwe@gmail.com>
 homepage: https://github.com/angel-dart/orm

angel_orm/test/to_sql_test.dart

@@ -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'");
-  });
-}

angel_orm_generator/lib/src/orm_build_context.dart

@@ -67,7 +67,8 @@ Future<OrmBuildContext> 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);
     }
 

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';

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';

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';