6935 remove order string syntax (#7220)

* Refactored support for ORDER BY clause so SQL injection is not possible.

* Adds sublime text files to gitignore.
Modified changelog to accurately represent changes to the order property.
Fixed lint issues.

* Added rejection with error where Order must be type of Array or instance of sequelize literal.

* Removed support for options.order accepting strings.
@TODO Look into plain objects with property raw
@TODO Update documentation and tests

* Removed all direct lodash and replaced them with Utils.
ie. `_.isPlainObject` is now `Utils._.isPlainObject`
find: `(?s)(?<!Utils\.)_\.`
replace : `Utils._.`

* Updates documentation for findAll order.

* Added removeTicks to addTicks in quoteIdentifier for each dialect.

* Added documentation for order by only an array.

* Added breaking change to changelog and renamed error to be more general.

@TODO: TESTS!

* Fixes bad code when checking if the order is valid.

* Updated unit tests.

* Should incorporate all integration tests.

* Update changelog.md capitalize Sequelize

* Changes everything in query generator to use lodash outside of utils.

* Instead of checking if the order object is an instance of a Utils member we just see if `_.isSequelizeMethod` is truthy.

* Removing escapeId because it is no longer used.

* Instead of removing ticks, we should escape them.

* Instead of removing ticks, we should escape them. (reverted from commit e4dade0a18be85dfaf8f91f1210bc0b5bedcb31a)

* Removes support for `._isSequelizeMethod` because it can be spoofed.  Instead, use instanceof.

lib/dialects/abstract/query-generator.js

@@ -479,7 +479,7 @@ const QueryGenerator = {
     const fieldsSql = options.fields.map(field => {
       if (typeof field === 'string') {
         return this.quoteIdentifier(field);
-      } else if (field._isSequelizeMethod) {
+      } else if (field instanceof Utils.SequelizeMethod) {
         return this.handleSequelizeMethod(field);
       } else {
         let result = '';
@@ -635,7 +635,7 @@ const QueryGenerator = {
 
       for (i = 0; i < len - 1; i++) {
         item = obj[i];
-        if (item._modelAttribute || _.isString(item) || item._isSequelizeMethod || 'raw' in item) {
+        if (item._modelAttribute || _.isString(item) || item instanceof Utils.SequelizeMethod || 'raw' in item) {
           break;
         }
 
@@ -668,7 +668,7 @@ const QueryGenerator = {
       // add 1st string as quoted, 2nd as unquoted raw
       let sql = (i > 0 ? this.quoteIdentifier(tableNames.join(connector)) + '.' : (_.isString(obj[0]) && parent ? this.quoteIdentifier(parent.name) + '.' : '')) + this.quote(obj[i], parent, force);
       if (i < len - 1) {
-        if (obj[i + 1]._isSequelizeMethod) {
+        if (obj[i + 1] instanceof Utils.SequelizeMethod) {
           sql += this.handleSequelizeMethod(obj[i + 1]);
         } else {
           sql += ' ' + obj[i + 1];
@@ -677,7 +677,7 @@ const QueryGenerator = {
       return sql;
     } else if (obj._modelAttribute) {
       return this.quoteTable(obj.Model.name) + '.' + obj.fieldName;
-    } else if (obj._isSequelizeMethod) {
+    } else if (obj instanceof Utils.SequelizeMethod) {
       return this.handleSequelizeMethod(obj);
     } else if (_.isObject(obj) && 'raw' in obj) {
       return obj.raw;
@@ -707,7 +707,7 @@ const QueryGenerator = {
     options = options || {};
 
     if (value !== null && value !== undefined) {
-      if (value._isSequelizeMethod) {
+      if (value instanceof Utils.SequelizeMethod) {
         return this.handleSequelizeMethod(value);
       } else {
         if (field && field.type) {
@@ -1029,7 +1029,7 @@ const QueryGenerator = {
     return attributes && attributes.map(attr => {
       let addTable = true;
 
-      if (attr._isSequelizeMethod) {
+      if (attr instanceof Utils.SequelizeMethod) {
         return this.handleSequelizeMethod(attr);
       }
       if (Array.isArray(attr)) {
@@ -1038,7 +1038,7 @@ const QueryGenerator = {
         }
         attr = attr.slice();
 
-        if (attr[0]._isSequelizeMethod) {
+        if (attr[0] instanceof Utils.SequelizeMethod) {
           attr[0] = this.handleSequelizeMethod(attr[0]);
           addTable = false;
         } else if (attr[0].indexOf('(') === -1 && attr[0].indexOf(')') === -1) {
@@ -1089,16 +1089,15 @@ const QueryGenerator = {
         let verbatim = false;
 
         if (Array.isArray(attr) && attr.length === 2) {
-          if (attr[0]._isSequelizeMethod) {
-            if (attr[0] instanceof Utils.Literal ||
-              attr[0] instanceof Utils.Cast ||
-              attr[0] instanceof Utils.Fn
-            ) {
-              verbatim = true;
-            }
+          if (attr[0] instanceof Utils.SequelizeMethod && (
+            attr[0] instanceof Utils.Literal ||
+            attr[0] instanceof Utils.Cast ||
+            attr[0] instanceof Utils.Fn
+          )) {
+            verbatim = true;
           }
 
-          attr = attr.map(attr => attr._isSequelizeMethod ? this.handleSequelizeMethod(attr) : attr);
+          attr = attr.map(attr => attr instanceof Utils.SequelizeMethod ? this.handleSequelizeMethod(attr) : attr);
 
           attrAs = attr[1];
           attr = attr[0];
@@ -1496,7 +1495,7 @@ const QueryGenerator = {
     ];
 
     const validateOrder = order => {
-      if (order._isSequelizeMethod) {
+      if (order instanceof Utils.SequelizeMethod) {
         return;
       }
 
@@ -1523,7 +1522,7 @@ const QueryGenerator = {
 
         mainQueryOrder.push(this.quote(order, model, false, '->'));
       }
-    } else if (options.order._isSequelizeMethod){
+    } else if (options.order instanceof Utils.SequelizeMethod){
       const sql = this.quote(options.order, model, false, '->');
       if (subQuery) {
         subQueryOrder.push(sql);
@@ -1682,13 +1681,13 @@ const QueryGenerator = {
       let value = smth.logic;
       let key;
 
-      if (smth.attribute._isSequelizeMethod) {
+      if (smth.attribute instanceof Utils.SequelizeMethod) {
         key = this.getWhereConditions(smth.attribute, tableName, factory, options, prepend);
       } else {
         key = this.quoteTable(smth.attribute.Model.name) + '.' + this.quoteIdentifier(smth.attribute.field || smth.attribute.fieldName);
       }
 
-      if (value && value._isSequelizeMethod) {
+      if (value && value instanceof Utils.SequelizeMethod) {
         value = this.getWhereConditions(value, tableName, factory, options, prepend);
 
         result = (value === 'NULL') ? key + ' IS NULL' : [key, value].join(smth.comparator);
@@ -1708,7 +1707,7 @@ const QueryGenerator = {
     } else if (smth instanceof Utils.Literal) {
       result = smth.val;
     } else if (smth instanceof Utils.Cast) {
-      if (smth.val._isSequelizeMethod) {
+      if (smth.val instanceof Utils.SequelizeMethod) {
         result = this.handleSequelizeMethod(smth.val, tableName, factory, options, prepend);
       } else if (_.isPlainObject(smth.val)) {
         result = this.whereItemsQuery(smth.val);
@@ -1719,7 +1718,7 @@ const QueryGenerator = {
       result = 'CAST(' + result + ' AS ' + smth.type.toUpperCase() + ')';
     } else if (smth instanceof Utils.Fn) {
       result = smth.fn + '(' + smth.args.map(arg => {
-        if (arg._isSequelizeMethod) {
+        if (arg instanceof Utils.SequelizeMethod) {
           return this.handleSequelizeMethod(arg, tableName, factory, options, prepend);
         } else if (_.isPlainObject(arg)) {
           return this.whereItemsQuery(arg);
@@ -1875,7 +1874,7 @@ const QueryGenerator = {
       }
     }
 
-    if (value && value._isSequelizeMethod && !(key !== undefined && value instanceof Utils.Fn)) {
+    if (value && value instanceof Utils.SequelizeMethod && !(key !== undefined && value instanceof Utils.Fn)) {
       return this.handleSequelizeMethod(value);
     }
 
@@ -2134,7 +2133,7 @@ const QueryGenerator = {
 
     if (key) {
       let prefix = true;
-      if (key._isSequelizeMethod) {
+      if (key instanceof Utils.SequelizeMethod) {
         key = this.handleSequelizeMethod(key);
       } else if (Utils.isColString(key)) {
         key = key.substr(1, key.length - 2).split('.');
@@ -2186,7 +2185,7 @@ const QueryGenerator = {
       prepend = true;
     }
 
-    if (smth && smth._isSequelizeMethod === true) { // Checking a property is cheaper than a lot of instanceof calls
+    if (smth && smth instanceof Utils.SequelizeMethod) { // Checking a property is cheaper than a lot of instanceof calls
       result = this.handleSequelizeMethod(smth, tableName, factory, options, prepend);
     } else if (_.isPlainObject(smth)) {
       return this.whereItemsQuery(smth, {

lib/instance-validator.js

@@ -319,7 +319,7 @@ class InstanceValidator {
     }
 
     if (rawAttribute.type === DataTypes.STRING || rawAttribute.type instanceof DataTypes.STRING || rawAttribute.type === DataTypes.TEXT || rawAttribute.type instanceof DataTypes.TEXT) {
-      if (Array.isArray(value) || (_.isObject(value) && !value._isSequelizeMethod) && !Buffer.isBuffer(value)) {
+      if (Array.isArray(value) || (_.isObject(value) && !(value instanceof Utils.SequelizeMethod)) && !Buffer.isBuffer(value)) {
         error = new sequelizeError.ValidationErrorItem(`${field} cannot be an array or an object`, 'string violation', field, value);
         this.errors.push(error);
       }

lib/model.js

@@ -2403,7 +2403,7 @@ class Model {
       throw new Error('Missing where or truncate attribute in the options parameter of model.destroy.');
     }
 
-    if (!options.truncate && !_.isPlainObject(options.where) && !_.isArray(options.where) && options.where._isSequelizeMethod !== true) {
+    if (!options.truncate && !_.isPlainObject(options.where) && !_.isArray(options.where) && !(options.where instanceof Utils.SequelizeMethod)) {
       throw new Error('Expected plain object, array or sequelize method in the options.where parameter of model.destroy.');
     }
 
@@ -2560,7 +2560,7 @@ class Model {
       throw new Error('Missing where attribute in the options parameter passed to update.');
     }
 
-    if (!_.isPlainObject(options.where) && !_.isArray(options.where) && options.where._isSequelizeMethod !== true) {
+    if (!_.isPlainObject(options.where) && !_.isArray(options.where) && !(options.where instanceof Utils.SequelizeMethod)) {
       throw new Error('Expected plain object, array or sequelize method in the options.where parameter of model.update.');
     }
 
@@ -2885,7 +2885,7 @@ class Model {
       if (this.constructor._hasDefaultValues) {
         defaults = _.mapValues(this.constructor._defaultValues, valueFn => {
           const value = valueFn();
-          return value && value._isSequelizeMethod ? value : _.cloneDeep(value);
+          return value && value instanceof Utils.SequelizeMethod ? value : _.cloneDeep(value);
         });
       }
 
@@ -3159,7 +3159,7 @@ class Model {
             }
 
             // Convert date fields to real date objects
-            if (this.constructor._hasDateAttributes && this.constructor._isDateAttribute(key) && !!value && !value._isSequelizeMethod) {
+            if (this.constructor._hasDateAttributes && this.constructor._isDateAttribute(key) && !!value && !(value instanceof Utils.SequelizeMethod)) {
               // Dont parse DATEONLY to new Date, keep them as string
               if (this.rawAttributes[key].type instanceof DataTypes.DATEONLY) {
                 if (originalValue && originalValue === value) {
@@ -3184,7 +3184,7 @@ class Model {
           }
 
           // Convert boolean-ish values to booleans
-          if (this.constructor._hasBooleanAttributes && this.constructor._isBooleanAttribute(key) && value !== null && value !== undefined && !value._isSequelizeMethod) {
+          if (this.constructor._hasBooleanAttributes && this.constructor._isBooleanAttribute(key) && value !== null && value !== undefined && !(value instanceof Utils.SequelizeMethod)) {
             if (Buffer.isBuffer(value) && value.length === 1) {
               // Bit fields are returned as buffers
               value = value[0];

lib/utils.js

@@ -455,8 +455,12 @@ exports.removeTicks = removeTicks;
  * Please do not use these functions directly, use Sequelize.fn and Sequelize.col instead.
  * @private
  */
-class Fn {
+class SequelizeMethod {}
+exports.SequelizeMethod = SequelizeMethod;
+
+class Fn extends SequelizeMethod {
   constructor(fn, args) {
+    super();
     this.fn = fn;
     this.args = args;
   }
@@ -466,8 +470,9 @@ class Fn {
 }
 exports.Fn = Fn;
 
-class Col {
+class Col extends SequelizeMethod {
   constructor(col) {
+    super();
     if (arguments.length > 1) {
       col = this.sliceArgs(arguments);
     }
@@ -476,23 +481,26 @@ class Col {
 }
 exports.Col = Col;
 
-class Cast {
+class Cast extends SequelizeMethod {
   constructor(val, type) {
+    super();
     this.val = val;
     this.type = (type || '').trim();
   }
 }
 exports.Cast = Cast;
 
-class Literal {
+class Literal extends SequelizeMethod {
   constructor(val) {
+    super();
     this.val = val;
   }
 }
 exports.Literal = Literal;
 
-class Json {
+class Json extends SequelizeMethod {
   constructor(conditionsOrPath, value) {
+    super();
     if (_.isObject(conditionsOrPath)) {
       this.conditions = conditionsOrPath;
     } else {
@@ -505,8 +513,9 @@ class Json {
 }
 exports.Json = Json;
 
-class Where {
+class Where extends SequelizeMethod {
   constructor(attribute, comparator, logic) {
+    super();
     if (logic === undefined) {
       logic = comparator;
       comparator = '=';
@@ -519,11 +528,4 @@ class Where {
 }
 exports.Where = Where;
 
-Where.prototype._isSequelizeMethod =
-Literal.prototype._isSequelizeMethod =
-Cast.prototype._isSequelizeMethod =
-Fn.prototype._isSequelizeMethod =
-Col.prototype._isSequelizeMethod =
-Json.prototype._isSequelizeMethod = true;
-
 exports.validateParameter = parameterValidator;