diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index bc80f3b832..61d0fe1216 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -30,7 +30,7 @@ jobs: fail-fast: false matrix: node-version: [16.x, 18.x, 20.x] - mysql-version: ["mysql:8.0.18", "mysql:8.0.22", "mysql:5.7"] + mysql-version: ["mysql:8.0.18", "mysql:8.0.22", "mysql:8.0.33", "mysql:5.7"] use-compression: [0] use-tls: [0] mysql_connection_url_key: [""] @@ -38,12 +38,12 @@ jobs: include: # 20.x - node-version: "20.x" - mysql-version: "mysql:8.0.18" + mysql-version: "mysql:8.0.33" use-compression: 1 use-tls: 0 use-builtin-test-runner: 1 - node-version: "20.x" - mysql-version: "mysql:8.0.18" + mysql-version: "mysql:8.0.33" use-compression: 0 use-tls: 1 use-builtin-test-runner: 1 diff --git a/README.md b/README.md index be692ab413..17cd42fa0d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -## Node MySQL 2 +## MySQL 2 [![Greenkeeper badge](https://badges.greenkeeper.io/sidorares/node-mysql2.svg)](https://greenkeeper.io/) [![NPM Version][npm-image]][npm-url] diff --git a/lib/commands/prepare.js b/lib/commands/prepare.js index d451088a14..03cb308122 100644 --- a/lib/commands/prepare.js +++ b/lib/commands/prepare.js @@ -73,10 +73,19 @@ class Prepare extends Command { return Prepare.prototype.readField; } return this.prepareDone(connection); - } readParameter(packet, connection) { + // there might be scenarios when mysql server reports more parameters than + // are actually present in the array of parameter definitions. + // if EOF packet is received we switch to "read fields" state if there are + // any fields reported by the server, otherwise we finish the command. + if (packet.isEOF()) { + if (this.fieldCount > 0) { + return Prepare.prototype.readField; + } + return this.prepareDone(connection); + } const def = new Packets.ColumnDefinition(packet, connection.clientEncoding); this.parameterDefinitions.push(def); if (this.parameterDefinitions.length === this.parameterCount) { @@ -86,6 +95,9 @@ class Prepare extends Command { } readField(packet, connection) { + if (packet.isEOF()) { + return this.prepareDone(connection); + } const def = new Packets.ColumnDefinition(packet, connection.clientEncoding); this.fields.push(def); if (this.fields.length === this.fieldCount) { diff --git a/lib/connection.js b/lib/connection.js index 2a0214cfad..2a33c5aefc 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -489,14 +489,23 @@ class Connection extends EventEmitter { } this._bumpSequenceId(packet.numPackets); } - const done = this._command.execute(packet, this); - if (done) { - this._command = this._commands.shift(); - if (this._command) { - this.sequenceId = 0; - this.compressedSequenceId = 0; - this.handlePacket(); + try { + if (this._fatalError) { + // skip remaining packets after client is in the error state + return; + } + const done = this._command.execute(packet, this); + if (done) { + this._command = this._commands.shift(); + if (this._command) { + this.sequenceId = 0; + this.compressedSequenceId = 0; + this.handlePacket(); + } } + } catch (err) { + this._handleFatalError(err); + this.stream.destroy(); } } diff --git a/test/builtin-runner/regressions/2052.test.mjs b/test/builtin-runner/regressions/2052.test.mjs index 79d746bc80..2bc5a83c83 100644 --- a/test/builtin-runner/regressions/2052.test.mjs +++ b/test/builtin-runner/regressions/2052.test.mjs @@ -1,29 +1,147 @@ -import { describe, it } from 'node:test'; +import { describe, it, beforeEach, afterEach } from 'node:test'; import assert from 'node:assert'; -// import common from '../../common.js'; - -describe('Prepare result when CLIENT_OPTIONAL_RESULTSET_METADATA is set or metadata_follows is unset', () => { - it('should not throw an exception', (t, done) => { - assert(true); - done(null); - /* - const connection = common.createConnection({ - database: 'mysql', +import common from '../../common.js'; +import PrepareCommand from '../../../lib/commands/prepare.js'; +import packets from '../../../lib/packets/index.js'; + +describe( + 'Unit test - prepare result with number of parameters incorrectly reported by the server', + { timeout: 1000 }, + () => { + it('should report 0 actual parameters when 1 placeholder is used in ORDER BY ?', (t, done) => { + const connection = { + constructor: { + statementKey: () => 0, + }, + _handshakePacket: {}, + _resetSequenceId: () => {}, + _statements: new Map(), + serverEncoding: 'utf8', + clientEncoding: 'utf8', + config: { + charsetNumber: 33, + }, + writePacket: (packet) => { + // client -> server COM_PREPARE + assert.equal( + packet.buffer.toString('hex'), + '000000001673656c656374202a2066726f6d207573657273206f72646572206279203f' + ); + }, + }; + const prepareCommand = new PrepareCommand( + { sql: 'select * from users order by ?' }, + (err, result) => { + assert.equal(err, null); + debugger; + assert.equal(result.parameters.length, 0); + assert.equal(result.columns.length, 51); + assert.equal(result.id, 1); + done(null); + } + ); + + prepareCommand.execute(null, connection); + const headerPacket = new packets.Packet( + 0, + Buffer.from('0000000000010000003300010000000005000002', 'hex'), + 0, + 20 + ); + prepareCommand.execute(headerPacket, connection); + const paramsEofPacket = new packets.Packet(0, Buffer.from('00000000fe000002002b000004', 'hex'), 0, 11); + prepareCommand.execute(paramsEofPacket, connection); + for (let i = 0; i < 51; ++i) { + const columnDefinitionPacket = new packets.Packet( + 0, + Buffer.from( + '0000000003646566056d7973716c0475736572047573657204486f737404486f73740ce000fc030000fe034000000005000005', 'hex' + ), + 0, + 47 + ); + prepareCommand.execute(columnDefinitionPacket, connection); + } + const columnsEofPacket = new packets.Packet(0, Buffer.from('00000000fe000002002b000004', 'hex'), 0, 11); + prepareCommand.execute(columnsEofPacket, connection); }); + } +); + +describe('E2E Prepare result with number of parameters incorrectly reported by the server', { timeout: 1000 }, () => { + let connection; + + function isNewerThan8_0_22() { + const { serverVersion } = connection._handshakePacket; + const [major, minor, patch] = serverVersion.split('.').map((x) => parseInt(x, 10)); + if (major > 8) { + return true; + } + if (major === 8 && minor > 0) { + return true; + } + if (major === 8 && minor === 0 && patch >= 22) { + return true; + } + return false; + } + beforeEach((t, done) => { + connection = common.createConnection({ + database: 'mysql', + }); connection.on('error', (err) => { done(err); }); + connection.on('connect', () => { + done(null); + }); + }); + afterEach((t, done) => { + connection.end(err => { + done(null) + }); + }); + + // see https://github.com/sidorares/node-mysql2/issues/2052#issuecomment-1620318928 + it('should report 0 actual parameters when 1 placeholder is used in ORDER BY ?', (t, done) => { connection.prepare('select * from user order by ?', (err, stmt) => { - console.log(err); if (err) { if (!err.fatal) { connection.end(); } done(err); + } else { + if(isNewerThan8_0_22()) { + // mysql 8.0.22 and newer report 0 parameters + assert.equal(stmt.parameters.length, 0); + } else { + // mariadb, mysql 8.0.21 and older report 1 parameter + assert.equal(stmt.parameters.length, 1); + } + done(null); + } + }); + }); + + it('should report 1 actual parameters when 2 placeholders used in ORDER BY?', (t, done) => { + connection.prepare('select * from user where user.User like ? order by ?', (err, stmt) => { + if (err) { + if (!err.fatal) { + connection.end(); + } + done(err); + } else { + if(isNewerThan8_0_22()) { + // mysql 8.0.22 and newer report 1 parameter + assert.equal(stmt.parameters.length, 1); + } else { + // mariadb, mysql 8.0.21 and older report 2 parameters + assert.equal(stmt.parameters.length, 2); + } + done(null); } }); - */ }); }); \ No newline at end of file