Merge pull request #718 from gazoakley/feat-source-param-type-ii

feat(execute): Tidy up source param types

Author: sidorares

documentation/Prepared-Statements.md

@@ -36,3 +36,18 @@ Note that you should not use statement after connection reset (`changeUser()` or
 # Configuration
 
 `maxPreparedStatements` : We keep the cached statements in a [lru-cache](https://github.com/isaacs/node-lru-cache). Default size is `16000` but you can use this option to override it. Any statements that are dropped from cache will be `closed`.
+
+# Serialization of bind parameters
+
+The bind parameter values passed to `execute` are serialized JS -> MySQL as:
+
+* `null` -> `NULL`
+* `number` -> `DOUBLE`
+* `boolean` -> `TINY` (0 for false, 1 for true)
+* `object` -> depending on prototype:
+  * `Date` -> `DATETIME`
+  * `JSON` like object - `JSON`
+  * `Buffer` -> `VAR_STRING`
+* Other -> `VAR_STRING`
+
+Passing in `undefined` or a `function` will result in an error.

lib/connection.js

@@ -715,6 +715,18 @@ Connection.prototype.execute = function execute(sql, values, cb) {
   }
   this._resolveNamedPlaceholders(options);
 
+  // check for values containing undefined
+  if (options.values) {
+    options.values.forEach(function(val) {
+      if (val === undefined) {
+        throw new TypeError('Bind parameters must not contain undefined. To pass SQL NULL specify JS null');
+      }
+      if (typeof val === 'function') {
+        throw new TypeError('Bind parameters must not contain function(s). To pass the body of a function as a string call .toString() first')
+      }
+    });
+  }
+
   var executeCommand = new Commands.Execute(options, cb);
   var prepareCommand = new Commands.Prepare(options, function(err, stmt) {
     if (err) {

lib/packets/execute.js

@@ -17,74 +17,59 @@ function isJSON(value) {
     (typeof value.toJSON === 'function' && !Buffer.isBuffer(value));
 }
 
-function toMysqlDateBuffer(value) {
-  var bufferValue = Buffer.allocUnsafe(12);
-  bufferValue.writeUInt8(11, 0);
-  bufferValue.writeUInt16LE(value.getFullYear(), 1);
-  bufferValue.writeUInt8(value.getMonth() + 1, 3);
-  bufferValue.writeUInt8(value.getDate(), 4);
-  bufferValue.writeUInt8(value.getHours(), 5);
-  bufferValue.writeUInt8(value.getMinutes(), 6);
-  bufferValue.writeUInt8(value.getSeconds(), 7);
-  bufferValue.writeUInt32LE(value.getMilliseconds() * 1000, 8);
-  return bufferValue;
-}
-
 /**
  * Converts a value to an object describing type, String/Buffer representation and length
  * @param {*} value
  */
 function toParameter(value, encoding) {
   var type = Types.VAR_STRING;
   var length;
-  var fixed = false;
+  var writer = function (value) {
+    return Packet.prototype.writeLengthCodedString.call(this, value, encoding)
+  }
   if (value !== null) {
     switch (typeof value) {
       case 'undefined':
         throw new TypeError('Bind parameters must not contain undefined');
 
       case 'number':
         type = Types.DOUBLE;
-        fixed = true;
-        var bufferValue = Buffer.allocUnsafe(8);
-        bufferValue.writeDoubleLE(value, 0);
-        value = bufferValue;
+        length = 8;
+        writer = Packet.prototype.writeDouble;
         break;
 
       case 'boolean':
+        value = value | 0;
         type = Types.TINY;
-        fixed = true;
-        var bufferValue = Buffer.allocUnsafe(1);
-        bufferValue.writeInt8(value | 0, 0);
-        value = bufferValue;
+        length = 1;
+        writer = Packet.prototype.writeInt8;
         break;
 
       case 'object':
         if (Object.prototype.toString.call(value) == '[object Date]') {
           type = Types.DATETIME;
-          fixed = true;
-          value = toMysqlDateBuffer(value);
+          length = 12;
+          writer = Packet.prototype.writeDate
         } else if (isJSON(value)) {
-          type = Types.JSON;
           value = JSON.stringify(value);
+          type = Types.JSON;
+        } else if (Buffer.isBuffer(value)) {
+          length = Packet.lengthCodedNumberLength(value.length) + value.length;
+          writer = Packet.prototype.writeLengthCodedBuffer
         }
         break;
+
+      default:
+        value = value.toString();
     }
   } else {
+    value = ''
     type = Types.NULL;
-    value = '';
   }
-  if (fixed) {
-    length = value.length;
-  } else {
-    if (Buffer.isBuffer(value)) {
-      length = Packet.lengthCodedNumberLength(value.length) + value.length;
-    } else {
-      value = value.toString();
-      length = Packet.lengthCodedStringLength(value, encoding);
-    }
+  if (!length) {
+    length = Packet.lengthCodedStringLength(value, encoding);
   }
-  return { type, value, length, fixed };
+  return { value, type, length, writer };
 }
 
 Execute.prototype.toPacket = function() {
@@ -151,14 +136,8 @@ Execute.prototype.toPacket = function() {
 
     // Write parameter values
     parameters.forEach(function (parameter) {
-      if (parameter.type !== Types.NULL) {
-        if (parameter.fixed) {
-          packet.writeBuffer(parameter.value);
-        } else if (Buffer.isBuffer(parameter.value)) {
-          packet.writeLengthCodedBuffer(parameter.value);
-        } else {
-          packet.writeLengthCodedString(parameter.value, self.encoding);
-        }
+    if (parameter.type !== Types.NULL) {
+        parameter.writer.call(packet, parameter.value)
       }
     });
   }

lib/packets/packet.js

@@ -760,6 +760,11 @@ Packet.prototype.writeInt8 = function(n) {
   this.offset++;
 };
 
+Packet.prototype.writeDouble = function(n) {
+  this.buffer.writeDoubleLE(n, this.offset);
+  this.offset += 8;
+}
+
 Packet.prototype.writeBuffer = function(b) {
   b.copy(this.buffer, this.offset);
   this.offset += b.length;
@@ -839,6 +844,18 @@ Packet.prototype.writeLengthCodedNumber = function(n) {
   return this.offset;
 };
 
+Packet.prototype.writeDate = function(d) {
+  this.buffer.writeUInt8(11, this.offset);
+  this.buffer.writeUInt16LE(d.getFullYear(), this.offset + 1);
+  this.buffer.writeUInt8(d.getMonth() + 1, this.offset + 3);
+  this.buffer.writeUInt8(d.getDate(), this.offset + 4);
+  this.buffer.writeUInt8(d.getHours(), this.offset + 5);
+  this.buffer.writeUInt8(d.getMinutes(), this.offset + 6);
+  this.buffer.writeUInt8(d.getSeconds(), this.offset + 7);
+  this.buffer.writeUInt32LE(d.getMilliseconds() * 1000, this.offset + 8);
+  this.offset += 12;
+}
+
 Packet.prototype.writeHeader = function(sequenceId) {
   var offset = this.offset;
   this.offset = 0;

test/integration/connection/test-execute-bind-function.js

@@ -0,0 +1,19 @@
+var common = require('../../common');
+var connection = common.createConnection();
+var assert = require('assert');
+
+var error = null;
+
+try {
+  connection.execute('SELECT ? AS result', [function () {}], function(err, _rows) { });
+} catch (err) {
+  error = err
+  connection.end();
+}
+
+process.on('exit', function() {
+  assert.equal(error.name, 'TypeError');
+  if (!error.message.match(/function/)) {
+    assert.fail('Expected error.message to contain \'function\'')
+  }
+});

test/integration/connection/test-execute-bind-undefined.js

@@ -3,16 +3,17 @@ var connection = common.createConnection();
 var assert = require('assert');
 
 var error = null;
-var foo = {};
 
-connection.execute('SELECT ? AS result', [foo.bar], function(err, _rows) { });
-
-// TODO: Needs to be a better way of catching this exception
-process.on('uncaughtException', (err) => {
-  error = err;
-  process.exit(0);
-});
+try {
+  connection.execute('SELECT ? AS result', [undefined], function(err, _rows) { });
+} catch (err) {
+  error = err
+  connection.end();
+}
 
 process.on('exit', function() {
   assert.equal(error.name, 'TypeError');
+  if (!error.message.match(/undefined/)) {
+    assert.fail('Expected error.message to contain \'undefined\'')
+  }
 });