Merge pull request mysqljs#618 from seanmonstar/change-user-packet

always pass handshakeInitializationPacket to ChangeUser

Author: seanmonstar

.travis.yml

@@ -3,3 +3,15 @@ node_js:
   - '0.6'
   - '0.8'
   - '0.10'
+
+before_script:
+- "mysql -e 'create database node_mysql;'"
+
+env:
+  - MYSQL_DATABASE=node_mysql
+
+mysql:
+  adapter: mysql2
+  username: root
+  encoding: utf8
+  database: node_mysql

lib/protocol/Protocol.js

@@ -106,25 +106,30 @@ Protocol.prototype._enqueue = function(sequence) {
 
   if (this._queue.length === 1) {
     this._parser.resetPacketNumber();
-    sequence.start();
+    this._startSequence(sequence);
   }
 
   return sequence;
 };
 
 Protocol.prototype._validateEnqueue = function(sequence) {
   var err;
-  var prefix = 'Cannot enqueue ' + sequence.constructor.name + ' after ';
+  var prefix = 'Cannot enqueue ' + sequence.constructor.name;
+  var prefixBefore = prefix + ' before ';
+  var prefixAfter = prefix + ' after ';
 
   if (this._quitSequence) {
-    err      = new Error(prefix + 'invoking quit.');
+    err      = new Error(prefixAfter + 'invoking quit.');
     err.code = 'PROTOCOL_ENQUEUE_AFTER_QUIT';
   } else if (this._destroyed) {
-    err      = new Error(prefix + 'being destroyed.');
+    err      = new Error(prefixAfter + 'being destroyed.');
     err.code = 'PROTOCOL_ENQUEUE_AFTER_DESTROY';
   } else if (this._handshakeSequence && sequence.constructor === Sequences.Handshake) {
-    err      = new Error(prefix + 'already enqueuing a Handshake.');
+    err      = new Error(prefixAfter + 'already enqueuing a Handshake.');
     err.code = 'PROTOCOL_ENQUEUE_HANDSHAKE_TWICE';
+  } else if (!this._handshakeSequence && sequence.constructor === Sequences.ChangeUser) {
+    err      = new Error(prefixBefore + 'a Handshake.');
+    err.code = 'PROTOCOL_ENQUEUE_BEFORE_HANDSHAKE';
   } else {
     return true;
   }
@@ -222,12 +227,15 @@ Protocol.prototype._dequeue = function() {
 
   this._parser.resetPacketNumber();
 
-  if (sequence.constructor == Sequences.ChangeUser) {
+  this._startSequence(sequence);
+};
+
+Protocol.prototype._startSequence = function(sequence) {
+  if (sequence.constructor === Sequences.ChangeUser) {
     sequence.start(this._handshakeInitializationPacket);
-    return;
+  } else {
+    sequence.start();
   }
-
-  sequence.start();
 };
 
 Protocol.prototype.handleNetworkError = function(err) {

test/integration/connection/test-change-user-empty-queue.js

@@ -0,0 +1,16 @@
+var common     = require('../../common');
+var connection = common.createConnection();
+var assert     = require('assert');
+
+connection.query('CREATE DATABASE ' + common.testDatabase, function(err) {
+  if (err && err.code !== 'ER_DB_CREATE_EXISTS') throw err;
+
+  // wait till protocol._queue is empty
+  setTimeout(function() {
+    connection.changeUser({database: common.testDatabase}, function(err) {
+      assert.ifError(err);
+    });
+
+    connection.end();
+  }, 2000);
+});

test/integration/connection/test-change-user-fatal-error.js

@@ -7,18 +7,16 @@ var common     = require('../../common');
 var connection = common.createConnection();
 var assert     = require('assert');
 
-if (common.isTravis()) {
-  return console.log('skipping - travis mysql does not support this test');
-}
 
 var err;
-connection.changeUser({user: 'does-not-exist'}, function(_err) {
+// XXX: user: 'does-not-exist' doesn't cause any error...
+connection.changeUser({database: 'does-not-exist'}, function(_err) {
   err = _err;
   connection.end();
 });
 
 process.on('exit', function() {
   if (process.env.NO_GRANT == '1' && err === null) return;
-  assert.equal(err.code, 'ER_ACCESS_DENIED_ERROR');
+  assert.equal(err.code, 'ER_BAD_DB_ERROR');
   assert.equal(err.fatal, true);
 });

test/integration/connection/test-change-user.js

@@ -2,10 +2,6 @@ var common     = require('../../common');
 var connection = common.createConnection();
 var assert     = require('assert');
 
-if (common.isTravis()) {
-  return console.log('skipping - travis mysql does not support this test');
-}
-
 connection.query('CREATE DATABASE ' + common.testDatabase, function(err) {
   if (err && err.code !== 'ER_DB_CREATE_EXISTS') throw err;
 });