Feat(#488): Added the benchmark option based on getTime

Sushant
Commit 9960398c authored Jan 07, 2016 by Sushant
Showing with 108 additions and 42 deletions
changelog.md
docs/docs/migrations.md
lib/dialects/abstract/query-generator.js
lib/dialects/mssql/query-generator.js
lib/dialects/mssql/query.js
lib/dialects/mysql/query.js
lib/dialects/postgres/query-generator.js
lib/dialects/postgres/query.js
lib/dialects/sqlite/query-generator.js
lib/dialects/sqlite/query.js
lib/model.js
package.json
test/integration/dialects/mysql/query-generator.test.js
test/unit/sql/offset-limit.test.js
test/unit/sql/select.test.js
--- a/changelog.md
+++ b/changelog.md
-# Future
+# 3.17.1
+- [FIXED] Reverted benchmarking feature since it does not compile on Node v4.0
+# 3.17.0
+- [CRITICAL] Fixed injection vulnerability for order/limit
 - [FIXED] MySQL throws error when null GEOMETRY data results in empty buffer [#4953](https://github.com/sequelize/sequelize/issues/4953)
- [ADDED] Support for benchmarking the execution time for SQL queries [#488](https://github.com/sequelize/sequelize/issues/488)
 # 3.16.0
 - [ADDED] PostgreSQL tsrange (Range of timestamp without time zone) data type support.

--- a/docs/docs/migrations.md
+++ b/docs/docs/migrations.md
@@ -214,7 +214,7 @@ queryInterface.changeColumn(
  {
    type: Sequelize.FLOAT,
    allowNull: false,
-    default: 0.0
+    defaultValue: 0.0
  }
 )
 ```

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -1765,12 +1765,12 @@ var QueryGenerator = {
  addLimitAndOffset: function(options, model) {
    var fragment = '';
    if (options.offset && !options.limit) {
-      fragment += ' LIMIT ' + options.offset + ', ' + 18440000000000000000;
+      fragment += ' LIMIT ' + this.escape(options.offset) + ', ' + 10000000000000;
    } else if (options.limit) {
      if (options.offset) {
-        fragment += ' LIMIT ' + options.offset + ', ' + options.limit;
+        fragment += ' LIMIT ' + this.escape(options.offset) + ', ' + this.escape(options.limit);
      } else {
-        fragment += ' LIMIT ' + options.limit;
+        fragment += ' LIMIT ' + this.escape(options.limit);
      }
    }

--- a/lib/dialects/mssql/query-generator.js
+++ b/lib/dialects/mssql/query-generator.js
@@ -590,11 +590,11 @@ var QueryGenerator = {
      }
      if (options.offset || options.limit) {
-        fragment += ' OFFSET ' + offset + ' ROWS';
+        fragment += ' OFFSET ' + this.escape(offset) + ' ROWS';
      }
      if (options.limit) {
-        fragment += ' FETCH NEXT ' + options.limit + ' ROWS ONLY';
+        fragment += ' FETCH NEXT ' + this.escape(options.limit) + ' ROWS ONLY';
      }
    }

--- a/lib/dialects/mssql/query.js
+++ b/lib/dialects/mssql/query.js
@@ -36,7 +36,7 @@ Query.prototype.run = function(sql, parameters) {
  if (!benchmark) {
    this.sequelize.log('Executing (' + (this.connection.uuid || 'default') + '): ' + this.sql, this.options);
  } else {
-    var queryBegin = (new Date()).getTime();
+    var queryBegin = Date.now();
  }
  var promise = new Utils.Promise(function(resolve, reject) {
@@ -72,7 +72,7 @@ Query.prototype.run = function(sql, parameters) {
        var request = new self.connection.lib.Request(self.sql, function(err) {
          if (benchmark) {
-            self.sequelize.log('Executed (' + (self.connection.uuid || 'default') + '): ' + self.sql + ' in ' + ((new Date()).getTime() - queryBegin) + 'ms', self.options);
+            self.sequelize.log('Executed (' + (self.connection.uuid || 'default') + '): ' + self.sql + ' in ' + (Date.now() - queryBegin) + 'ms', self.options);
          }
          if (err) {

--- a/lib/dialects/mysql/query.js
+++ b/lib/dialects/mysql/query.js
@@ -32,14 +32,14 @@ Query.prototype.run = function(sql, parameters) {
  if (!benchmark) {
    this.sequelize.log('Executing (' + (this.connection.uuid || 'default') + '): ' + this.sql, this.options);
  } else {
-    var queryBegin = (new Date()).getTime();
+    var queryBegin = Date.now();
  }
  var promise = new Utils.Promise(function(resolve, reject) {
    self.connection.query(self.sql, function(err, results) {
      if (benchmark) {
-        self.sequelize.log('Executed (' + (self.connection.uuid || 'default') + '): ' + self.sql + ' in ' + ((new Date()).getTime() - queryBegin) + 'ms', self.options);
+        self.sequelize.log('Executed (' + (self.connection.uuid || 'default') + '): ' + self.sql + ' in ' + (Date.now() - queryBegin) + 'ms', self.options);
      }
      if (err) {

--- a/lib/dialects/postgres/query-generator.js
+++ b/lib/dialects/postgres/query-generator.js
@@ -423,8 +423,8 @@ var QueryGenerator = {
  addLimitAndOffset: function(options) {
    var fragment = '';
-    if (options.limit) fragment += ' LIMIT ' + options.limit;
+    if (options.limit) fragment += ' LIMIT ' + this.escape(options.limit);
-    if (options.offset) fragment += ' OFFSET ' + options.offset;
+    if (options.offset) fragment += ' OFFSET ' + this.escape(options.offset);
    return fragment;
  },

--- a/lib/dialects/postgres/query.js
+++ b/lib/dialects/postgres/query.js
@@ -68,7 +68,7 @@ Query.prototype.run = function(sql, parameters) {
  if (!benchmark) {
    this.sequelize.log('Executing (' + (this.client.uuid || 'default') + '): ' + this.sql, this.options);
  } else {
-    var queryBegin = (new Date()).getTime();
+    var queryBegin = Date.now();
  }
  var promise = new Promise(function(resolve, reject) {
@@ -91,7 +91,7 @@ Query.prototype.run = function(sql, parameters) {
    query.on('end', function(result) {
      if (benchmark) {
-        self.sequelize.log('Executed (' + (self.client.uuid || 'default') + '): ' + self.sql + ' in ' + ((new Date()).getTime() - queryBegin) + 'ms', self.options);
+        self.sequelize.log('Executed (' + (self.client.uuid || 'default') + '): ' + self.sql + ' in ' + (Date.now() - queryBegin) + 'ms', self.options);
      }
      if (receivedError) {

--- a/lib/dialects/sqlite/query-generator.js
+++ b/lib/dialects/sqlite/query-generator.js
@@ -88,21 +88,6 @@ var QueryGenerator = {
    return !!value ? 1 : 0;
  },
-  addLimitAndOffset: function(options){
-    var fragment = '';
-    if (options.offset && !options.limit) {
-      fragment += ' LIMIT ' + options.offset + ', ' + 10000000000000;
-    } else if (options.limit) {
-      if (options.offset) {
-        fragment += ' LIMIT ' + options.offset + ', ' + options.limit;
-      } else {
-        fragment += ' LIMIT ' + options.limit;
-      }
-    }
-    return fragment;
-  },
  addColumnQuery: function(table, key, dataType) {
    var query = 'ALTER TABLE <%= table %> ADD <%= attribute %>;'
      , attributes = {};

--- a/lib/dialects/sqlite/query.js
+++ b/lib/dialects/sqlite/query.js
@@ -90,7 +90,7 @@ Query.prototype.run = function(sql, parameters) {
  if (!benchmark) {
    this.sequelize.log('Executing (' + (this.database.uuid || 'default') + '): ' + this.sql, this.options);
  } else {
-    var queryBegin = (new Date()).getTime();
+    var queryBegin = Date.now();
  }
  promise = new Promise(function(resolve) {
@@ -104,7 +104,7 @@ Query.prototype.run = function(sql, parameters) {
            var afterExecute = function(err, results) {
              if (benchmark) {
-                self.sequelize.log('Executed (' + (self.database.uuid || 'default') + '): ' + self.sql + ' in ' + ((new Date()).getTime() - queryBegin) + 'ms', self.options);
+                self.sequelize.log('Executed (' + (self.database.uuid || 'default') + '): ' + self.sql + ' in ' + (Date.now() - queryBegin) + 'ms', self.options);
              }
              if (err) {

--- a/lib/model.js
+++ b/lib/model.js
@@ -1022,8 +1022,8 @@ Model.prototype.dropSchema = function(schema) {
 * while the schema will be prepended to the table name for mysql and sqlite - `'schema.tablename'`.
 *
 * @param {String} schema The name of the schema
- * @param {Object}   [options]
+ * @param {Object} [options]
- * @param {String}   [options.schemaDelimiter='.'] The character(s) that separates the schema name from the table name
+ * @param {String} [options.schemaDelimiter='.'] The character(s) that separates the schema name from the table name
 * @param {Function} [options.logging=false] A function that gets executed while running the query to log the sql.
 * @param {Boolean}  [options.benchmark=false] Print query execution time in milliseconds when logging SQL.
 * @return {this}
@@ -1506,7 +1506,7 @@ Model.prototype.find = Model.prototype.findOne;
 * @param {Object}          [options.where] A hash of search attributes.
 * @param {Function}        [options.logging=false] A function that gets executed while running the query to log the sql.
 * @param {DataType|String} [options.dataType] The type of the result. If `field` is a field in this Model, the default will be the type of that field, otherwise defaults to float.
- * @param {Boolean}         [options.distinct] Applies DISTINCT to the field being aggregated over
+ * @param {boolean}         [options.distinct] Applies DISTINCT to the field being aggregated over
 * @param {Transaction}     [options.transaction] Transaction to run query under
 * @param {Boolean}         [options.plain] When `true`, the first returned value of `aggregateFunction` is cast to `dataType` and returned. If additional attributes are specified, along with `group` clauses, set `plain` to `false` to return all values of all returned rows.  Defaults to `true`
 * @param {Boolean}         [options.benchmark=false] Print query execution time in milliseconds when logging SQL.
@@ -2239,7 +2239,7 @@ Model.prototype.truncate = function(options) {
 * @param  {Boolean}      [options.force=false]           Delete instead of setting deletedAt to current timestamp (only applicable if `paranoid` is enabled)
 * @param  {Boolean}      [options.truncate=false]        If set to true, dialects that support it will use TRUNCATE instead of DELETE FROM. If a table is truncated the where and limit options are ignored
 * @param  {Boolean}      [options.cascade=false]         Only used in conjunction with TRUNCATE. Truncates  all tables that have foreign-key references to the named table, or to any tables added to the group due to CASCADE.
- * @param  {Transaction}  [options.transaction]           Transaction to run query under
+ * @param  {Transaction}  [options.transaction] Transaction to run query under
 * @param  {Function}     [options.logging=false]         A function that gets executed while running the query to log the sql.
 * @param  {Boolean}      [options.benchmark=false]       Print query execution time in milliseconds when logging SQL.
 * @return {Promise<Integer>} The number of destroyed rows

--- a/package.json
+++ b/package.json
 {
  "name": "sequelize",
  "description": "Multi dialect ORM for Node.JS/io.js",
-  "version": "3.16.0",
+  "version": "3.17.1",
  "author": "Sascha Depold <sascha@depold.com>",
  "contributors": [
    {

--- a/test/integration/dialects/mysql/query-generator.test.js
+++ b/test/integration/dialects/mysql/query-generator.test.js
@@ -332,7 +332,7 @@ if (Support.dialectIsMySQL()) {
        }, {
          title: 'uses default limit if only offset is specified',
          arguments: ['myTable', {offset: 2}],
-          expectation: 'SELECT * FROM `myTable` LIMIT 2, 18440000000000000000;',
+          expectation: 'SELECT * FROM `myTable` LIMIT 2, 10000000000000;',
          context: QueryGenerator
        }, {
          title: 'multiple where arguments',

--- a/test/unit/sql/offset-limit.test.js
+++ b/test/unit/sql/offset-limit.test.js
+'use strict';
+/* jshint -W110 */
+var Support   = require(__dirname + '/../support')
+  , util = require('util')
+  , expectsql = Support.expectsql
+  , current   = Support.sequelize
+  , sql       = current.dialect.QueryGenerator;
+// Notice: [] will be replaced by dialect specific tick/quote character when there is not dialect specific expectation but only a default expectation
+suite(Support.getTestDialectTeaser('SQL'), function() {
+  suite('offset/limit', function () {
+    var testsql = function (options, expectation) {
+      var model = options.model;
+      test(util.inspect(options, {depth: 2}), function () {
+        return expectsql(
+          sql.addLimitAndOffset(
+            options,
+            model
+          ),
+          expectation
+        );
+      });
+    };
+    testsql({
+      limit: 10,
+      order: [
+        ['email', 'DESC'] // for MSSQL
+      ]
+    }, {
+      default: ' LIMIT 10',
+      mssql: ' OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY'
+    });
+    testsql({
+      limit: 10,
+      offset: 20,
+      order: [
+        ['email', 'DESC'] // for MSSQL
+      ]
+    }, {
+      default: ' LIMIT 20, 10',
+      postgres: ' LIMIT 10 OFFSET 20',
+      mssql: ' OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY'
+    });
+    testsql({
+      limit: "';DELETE FROM user",
+      order: [
+        ['email', 'DESC'] // for MSSQL
+      ]
+    }, {
+      default: " LIMIT ''';DELETE FROM user'",
+      mysql: " LIMIT '\\';DELETE FROM user'",
+      mssql: " OFFSET 0 ROWS FETCH NEXT N''';DELETE FROM user' ROWS ONLY"
+    });
+    testsql({
+      limit: 10,
+      offset: "';DELETE FROM user",
+      order: [
+        ['email', 'DESC'] // for MSSQL
+      ]
+    }, {
+      sqlite: " LIMIT ''';DELETE FROM user', 10",
+      postgres: " LIMIT 10 OFFSET ''';DELETE FROM user'",
+      mysql: " LIMIT '\\';DELETE FROM user', 10",
+      mssql: " OFFSET N''';DELETE FROM user' ROWS FETCH NEXT 10 ROWS ONLY"
+    });
+  });
+});
--- a/test/unit/sql/select.test.js
+++ b/test/unit/sql/select.test.js
@@ -36,10 +36,14 @@ suite(Support.getTestDialectTeaser('SQL'), function() {
      ],
      where: {
        email: 'jon.snow@gmail.com'
-      }
+      },
+      order: [
+        ['email', 'DESC']
+      ],
+      limit: 10
    }, {
-      default: "SELECT [email], [first_name] AS [firstName] FROM [User] WHERE [User].[email] = 'jon.snow@gmail.com';",
+      default: "SELECT [email], [first_name] AS [firstName] FROM [User] WHERE [User].[email] = 'jon.snow@gmail.com' ORDER BY [email] DESC LIMIT 10;",
-      mssql: "SELECT [email], [first_name] AS [firstName] FROM [User] WHERE [User].[email] = N'jon.snow@gmail.com';"
+      mssql: "SELECT [email], [first_name] AS [firstName] FROM [User] WHERE [User].[email] = N'jon.snow@gmail.com' ORDER BY [email] DESC OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY;"
    });
    testsql({