Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[CONJS-263] ensure respecting server collation when no charset/collat…
…ion is set
  • Loading branch information
rusher committed Jul 24, 2023
1 parent 05d5155 commit df28830
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 23 deletions.
16 changes: 1 addition & 15 deletions lib/cmd/handshake/auth/handshake.js
Expand Up @@ -39,19 +39,6 @@ class Handshake extends PluginAuth {
}

let handshake = new InitialHandshake(packet, info);

// handle default collation.
if (opts.collation) {
// collation has been set using charset.
// If server use same charset, use server collation.
if (!opts.charset || info.collation.charset !== opts.collation.charset) {
info.collation = opts.collation;
}
} else if (info.collation.charset !== 'utf8' || info.collation.maxLength === 3) {
// if not utf8mb4 and no configuration, force to use UTF8MB4_UNICODE_CI
info.collation = Collations.fromIndex(224);
}

ClientCapabilities.init(opts, info);

if (opts.ssl) {
Expand Down Expand Up @@ -108,8 +95,7 @@ class Handshake extends PluginAuth {
}
out.writeInt32(Number(info.clientCapabilities & BigInt(0xffffffff)));
out.writeInt32(1024 * 1024 * 1024); // max packet size
out.writeInt8(info.collation.index);

out.writeInt8(opts.collation ? opts.collation.index : info.collation.index);
for (let i = 0; i < 19; i++) {
out.writeInt8(0);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/cmd/handshake/ssl-request.js
Expand Up @@ -17,7 +17,7 @@ module.exports.send = function sendSSLRequest(cmd, out, info, opts) {
out.startPacket(cmd);
out.writeInt32(Number(info.clientCapabilities & BigInt(0xffffffff)));
out.writeInt32(1024 * 1024 * 1024); // max packet size
out.writeInt8(info.collation.index);
out.writeInt8(opts.collation ? opts.collation.index : info.collation.index);
for (let i = 0; i < 19; i++) {
out.writeInt8(0);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/config/connection-options.js
Expand Up @@ -42,9 +42,10 @@ class ConnectionOptions {
"')\n" +
"(collation looks like 'UTF8MB4_UNICODE_CI', charset like 'utf8')."
);
} else {
this.charset = opts.charset;
}
}
if (this.collation === undefined) throw new RangeError("Unknown charset '" + opts.charset + "'");
} else if (opts.collation && typeof opts.collation === 'string') {
this.collation = Collations.fromName(opts.collation.toUpperCase());
if (this.collation === undefined) throw new RangeError("Unknown collation '" + opts.collation + "'");
Expand Down
31 changes: 30 additions & 1 deletion lib/connection.js
Expand Up @@ -365,6 +365,7 @@ class Connection extends EventEmitter {
let prom = Promise.resolve();
// re-execute init query / session query timeout
prom
.then(conn.handleCharset.bind(conn))
.then(conn.handleTimezone.bind(conn))
.then(conn.executeInitQuery.bind(conn))
.then(conn.executeSessionTimeout.bind(conn))
Expand Down Expand Up @@ -656,6 +657,18 @@ class Connection extends EventEmitter {
return Promise.resolve();
}

/**
* set charset to utf8
* @returns {Promise<void>}
* @private
*/
handleCharset() {
if (this.opts.collation) return Promise.resolve();
return new Promise(
this.query.bind(this, new CommandParameter(`SET NAMES ${this.opts.charset ? this.opts.charset : 'utf8mb4'}`))
);
}

/**
* Asking server timezone if not set in case of 'auto'
* @returns {Promise<void>}
Expand Down Expand Up @@ -864,6 +877,7 @@ class Connection extends EventEmitter {
const conn = this;
this.status = Status.INIT_CMD;
this.executeSessionVariableQuery()
.then(conn.handleCharset.bind(conn))
.then(this.handleTimezone.bind(this))
.then(this.checkServerVersion.bind(this))
.then(this.executeInitQuery.bind(this))
Expand Down Expand Up @@ -891,6 +905,7 @@ class Connection extends EventEmitter {
} else {
conn.authFailHandler.call(conn, err);
}
return Promise.reject(err);
});
}

Expand Down Expand Up @@ -1248,14 +1263,28 @@ class Connection extends EventEmitter {
if (this.status < Status.CLOSING) {
this.addCommand = this.addCommandEnable;
}
let conn = this;
this.addCommand(
new ChangeUser(
cmdParam,
this.opts,
(res) => {
if (this.status < Status.CLOSING && this.opts.pipelining) this.addCommand = this.addCommandEnablePipeline;
if (cmdParam.opts && cmdParam.opts.collation) this.opts.collation = cmdParam.opts.collation;
resolve(res);
conn
.handleCharset()
.then(() => resolve(res))
.catch((err) => {
if (!err.fatal) {
const res = () => {
conn.authFailHandler.call(conn, err);
};
conn.end(res, res);
} else {
conn.authFailHandler.call(conn, err);
}
reject(err);
});
},
this.authFailHandler.bind(this, reject),
this.getSocket.bind(this)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/test-connection-opts.js
Expand Up @@ -14,7 +14,7 @@ describe('connection option', () => {
done(new Error('must have thrown error!'));
})
.catch((err) => {
assert(err.message.includes('Unknown charset'));
assert(err.message.includes('Unknown'));
done();
});
});
Expand Down
8 changes: 4 additions & 4 deletions test/integration/test-debug.js
Expand Up @@ -216,7 +216,7 @@ describe('debug', () => {
.then(() => {
const serverVersion = conn.serverVersion();
const data = fs.readFileSync(tmpLogFile, 'utf8');
let range = [8000, 10500];
let range = [9500, 12000];
assert(
data.length > range[0] && data.length < range[1],
'wrong data length : ' +
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('debug', () => {
const conn = await base.createConnection({ debug: true });
const res = await conn.query("SELECT '1'");
conn.end();
const range = [3200, 5000];
const range = [3700, 5500];
assert(
data.length > range[0] && data.length < range[1],
'wrong data length : ' +
Expand Down Expand Up @@ -309,7 +309,7 @@ describe('debug', () => {
setTimeout(() => {
const data = fs.readFileSync(tmpLogFile, 'utf8');
const serverVersion = conn.serverVersion();
const range = [6500, 9000 + (Conf.baseConfig.ssl ? 800 : 0)];
const range = [7500, 10000 + (Conf.baseConfig.ssl ? 800 : 0)];
assert(
data.length > range[0] && data.length < range[1],
'wrong data length : ' +
Expand Down Expand Up @@ -360,7 +360,7 @@ describe('debug', () => {
const serverVersion = conn.serverVersion();
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql' || process.env.srv === 'skysql-ha')
compress = false;
const range = compress ? [60, 150] : [60, 140];
const range = compress ? [90, 180] : [90, 170];
const data = fs.readFileSync(tmpLogFile, 'utf8');
assert.isTrue(data.includes('PING'));
assert.isTrue(data.includes('QUIT'));
Expand Down

0 comments on commit df28830

Please sign in to comment.