From 7b6261054c13c21a072cdcc377f53d2182107f44 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Mon, 11 Oct 2021 15:30:16 +1100 Subject: [PATCH 1/7] test for mysql 8.0.22 bind parameters regression - expected to fail until a fix --- test/integration/connection/test-execute-type-casting.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/connection/test-execute-type-casting.js b/test/integration/connection/test-execute-type-casting.js index 02c416f790..8c4ae4cceb 100644 --- a/test/integration/connection/test-execute-type-casting.js +++ b/test/integration/connection/test-execute-type-casting.js @@ -38,8 +38,8 @@ connection.query('select 1', waitConnectErr => { let row; connection.execute( - `SELECT * FROM ${table} WHERE id = ?;`, - [1], + `SELECT * FROM ${table} WHERE id = ? LIMIT ?`, + [1, 1], (err, rows) => { if (err) { throw err; From 26c122648c5c095d7dc8987050e9ecd7be120d87 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Mon, 11 Oct 2021 17:36:02 +1100 Subject: [PATCH 2/7] use column types from parameters in the prepared statement rather than inferring from input variables --- lib/commands/execute.js | 2 +- lib/packets/execute.js | 87 +++++++++++++++++++++++++++-------------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/lib/commands/execute.js b/lib/commands/execute.js index d5c2d8ab79..79c4ea1107 100644 --- a/lib/commands/execute.js +++ b/lib/commands/execute.js @@ -39,7 +39,7 @@ class Execute extends Command { this.options = Object.assign({}, connection.config, this._executeOptions); this._setTimeout(); const executePacket = new Packets.Execute( - this.statement.id, + this.statement, this.parameters, connection.config.charsetNumber, connection.config.timezone diff --git a/lib/packets/execute.js b/lib/packets/execute.js index c612f66e12..cff1df9563 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -2,6 +2,7 @@ const CursorType = require('../constants/cursor'); const CommandCodes = require('../constants/commands'); +const FieldFlags = require('../constants/field_flags.js'); const Types = require('../constants/types'); const Packet = require('../packets/packet'); const CharsetToEncoding = require('../constants/charset_encodings.js'); @@ -18,49 +19,71 @@ function isJSON(value) { * Converts a value to an object describing type, String/Buffer representation and length * @param {*} value */ -function toParameter(value, encoding, timezone) { - let type = Types.VAR_STRING; +function toParameter(value, encoding, timezone, parameterHints) { + let type = parameterHints.columnType; let length; let writer = function(value) { // eslint-disable-next-line no-invalid-this 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'); + let unsigned = parameterHints.flags & FieldFlags.UNSIGNED; - case 'number': - type = Types.DOUBLE; - length = 8; - writer = Packet.prototype.writeDouble; - break; + if (value !== null) { + if (typeof value == 'undefined') { + throw new TypeError('Bind parameters must not contain undefined'); + } - case 'boolean': + console.log(parameterHints.columnType, parameterHints.columnName) + switch(parameterHints.columnType) { + case Types.TINY: value = value | 0; - type = Types.TINY; length = 1; - writer = Packet.prototype.writeInt8; + writer = unsigned ? Packet.prototype.writeInt8 : Packet.prototype.writeUInt8; break; - - case 'object': - if (Object.prototype.toString.call(value) === '[object Date]') { - type = Types.DATETIME; + case Types.YEAR: + case Types.SHORT: + value = value | 0; + length = 2; + writer = unsigned ? Packet.prototype.writeInt16 : Packet.prototype.writeSInt16; + break; + case Types.LONG: + case Types.INT24: // in binary protocol int24 is encoded in 4 bytes int32 + value = value | 0; + length = 4; + writer = unsigned ? packet.writeInt32() : packet.writeSInt32(); + + case Types.DATETIME: + if (!(Object.prototype.toString.call(value) === '[object Date]')) { length = 12; writer = function(value) { // eslint-disable-next-line no-invalid-this return Packet.prototype.writeDate.call(this, value, timezone); }; - } else if (isJSON(value)) { - value = JSON.stringify(value); - type = Types.JSON; - } else if (Buffer.isBuffer(value)) { - length = Packet.lengthCodedNumberLength(value.length) + value.length; - writer = Packet.prototype.writeLengthCodedBuffer; + } else { + // if parameter is not a date serialize it as string by default + value = value.toString(); + type = Types.VAR_STRING; } break; - + case Types.FLOAT: + length = 4; + writer = Packet.prototype.writeFloat; + break; + case Types.DOUBLE: + length = 8; + writer = Packet.prototype.writeDouble; + break; + case Types.JSON: + value = JSON.stringify(value); + type = parameterHints.type; + break; + case Types.LONGLONG: + // this should also cover anything that serializes to a string ( BigInt etc ) + type = Types.VAR_STRING; + value = value.toString(); + break; default: + type = Types.VAR_STRING; value = value.toString(); } } else { @@ -70,18 +93,24 @@ function toParameter(value, encoding, timezone) { if (!length) { length = Packet.lengthCodedStringLength(value, encoding); } + console.log({ value, type, length, writer }) return { value, type, length, writer }; } class Execute { - constructor(id, parameters, charsetNumber, timezone) { - this.id = id; + constructor(statement, parameters, charsetNumber, timezone) { + this.statement = statement; this.parameters = parameters; this.encoding = CharsetToEncoding[charsetNumber]; this.timezone = timezone; } toPacket() { + + if (this.statement.parameters.length > 0 && this.parameters.length !== this.statement.parameters.length ) { + throw new TypeError(`Incorrect number of bind parameters, expected ${this.statement.parameters.length} but supplied ${this.parameters.length}`); + } + // TODO: don't try to calculate packet length in advance, allocate some big buffer in advance (header + 256 bytes?) // and copy + reallocate if not enough // 0 + 4 - length, seqId @@ -95,8 +124,8 @@ class Execute { length += Math.floor((this.parameters.length + 7) / 8); length += 1; // new-params-bound-flag length += 2 * this.parameters.length; // type byte for each parameter if new-params-bound-flag is set - parameters = this.parameters.map(value => - toParameter(value, this.encoding, this.timezone) + parameters = this.parameters.map((value, index) => + toParameter(value, this.encoding, this.timezone, this.statement.parameters[index]) ); length += parameters.reduce( (accumulator, parameter) => accumulator + parameter.length, From 51c2e43a9085532317105fa41d5bfd9743bf3900 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Mon, 11 Oct 2021 17:38:49 +1100 Subject: [PATCH 3/7] lint --- lib/packets/execute.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/packets/execute.js b/lib/packets/execute.js index cff1df9563..e4c7a7f7b4 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -7,14 +7,6 @@ const Types = require('../constants/types'); const Packet = require('../packets/packet'); const CharsetToEncoding = require('../constants/charset_encodings.js'); -function isJSON(value) { - return ( - Array.isArray(value) || - value.constructor === Object || - (typeof value.toJSON === 'function' && !Buffer.isBuffer(value)) - ); -} - /** * Converts a value to an object describing type, String/Buffer representation and length * @param {*} value From 830026ec6a5787b9bf90ef9e19fe4832507295cc Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Mon, 11 Oct 2021 21:31:23 +1100 Subject: [PATCH 4/7] fix serialization of statement id --- lib/packets/execute.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/packets/execute.js b/lib/packets/execute.js index e4c7a7f7b4..b26600e8ee 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -18,14 +18,13 @@ function toParameter(value, encoding, timezone, parameterHints) { // eslint-disable-next-line no-invalid-this return Packet.prototype.writeLengthCodedString.call(this, value, encoding); }; - let unsigned = parameterHints.flags & FieldFlags.UNSIGNED; + const unsigned = parameterHints.flags & FieldFlags.UNSIGNED; if (value !== null) { - if (typeof value == 'undefined') { + if (typeof value === 'undefined') { throw new TypeError('Bind parameters must not contain undefined'); } - console.log(parameterHints.columnType, parameterHints.columnName) switch(parameterHints.columnType) { case Types.TINY: value = value | 0; @@ -42,8 +41,8 @@ function toParameter(value, encoding, timezone, parameterHints) { case Types.INT24: // in binary protocol int24 is encoded in 4 bytes int32 value = value | 0; length = 4; - writer = unsigned ? packet.writeInt32() : packet.writeSInt32(); - + writer = unsigned ? Packet.prototype.writeInt32() : Packet.prototype.writeSInt32(); + break; case Types.DATETIME: if (!(Object.prototype.toString.call(value) === '[object Date]')) { length = 12; @@ -85,7 +84,6 @@ function toParameter(value, encoding, timezone, parameterHints) { if (!length) { length = Packet.lengthCodedStringLength(value, encoding); } - console.log({ value, type, length, writer }) return { value, type, length, writer }; } @@ -128,7 +126,7 @@ class Execute { const packet = new Packet(0, buffer, 0, length); packet.offset = 4; packet.writeInt8(CommandCodes.STMT_EXECUTE); - packet.writeInt32(this.id); + packet.writeInt32(this.statement.id); packet.writeInt8(CursorType.NO_CURSOR); // flags packet.writeInt32(1); // iteration-count, always 1 if (parameters) { From 1385eeb77db750c4e44b516521cfac52d646fee7 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Mon, 11 Oct 2021 21:39:42 +1100 Subject: [PATCH 5/7] fix check for number of parameters --- lib/packets/execute.js | 2 +- test/integration/connection/test-signed-tinyint.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/packets/execute.js b/lib/packets/execute.js index b26600e8ee..31592a169e 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -97,7 +97,7 @@ class Execute { toPacket() { - if (this.statement.parameters.length > 0 && this.parameters.length !== this.statement.parameters.length ) { + if (this.statement.parameters && this.parameters.length !== this.statement.parameters.length ) { throw new TypeError(`Incorrect number of bind parameters, expected ${this.statement.parameters.length} but supplied ${this.parameters.length}`); } diff --git a/test/integration/connection/test-signed-tinyint.js b/test/integration/connection/test-signed-tinyint.js index 3a365b0456..10660c8a27 100644 --- a/test/integration/connection/test-signed-tinyint.js +++ b/test/integration/connection/test-signed-tinyint.js @@ -12,7 +12,7 @@ connection.query( connection.query('INSERT INTO signed_ints values (-3, -120, 500)'); connection.query('INSERT INTO signed_ints values (3, -110, -500)'); -connection.execute('SELECT * from signed_ints', [5], (err, _rows) => { +connection.execute('SELECT * from signed_ints', [], (err, _rows) => { if (err) { throw err; } From 9e3a5ae1fa89b846fc1cd7e0c40c428fdca89a24 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Tue, 12 Oct 2021 08:07:42 +1100 Subject: [PATCH 6/7] infer input type from Date, Buffer and object parameters --- lib/packets/execute.js | 32 ++++++++++++++++++- .../connection/test-execute-bind-boolean.js | 2 +- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/packets/execute.js b/lib/packets/execute.js index 31592a169e..4cdf84f1cf 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -7,6 +7,14 @@ const Types = require('../constants/types'); const Packet = require('../packets/packet'); const CharsetToEncoding = require('../constants/charset_encodings.js'); +function isJSON(value) { + return ( + Array.isArray(value) || + value.constructor === Object || + (typeof value.toJSON === 'function' && !Buffer.isBuffer(value)) + ); +} + /** * Converts a value to an object describing type, String/Buffer representation and length * @param {*} value @@ -25,6 +33,28 @@ function toParameter(value, encoding, timezone, parameterHints) { throw new TypeError('Bind parameters must not contain undefined'); } + // currently exception for Date and plain object input parameters - even if statement expects another type we'll still send date/stringified json + // this might change in the future + if (Object.prototype.toString.call(value) === '[object Date]') { + type = Types.DATETIME; + length = 12; + writer = function(value) { + // eslint-disable-next-line no-invalid-this + return Packet.prototype.writeDate.call(this, value, timezone); + }; + return { value, type, length, writer }; + } else if (isJSON(value)) { + value = JSON.stringify(value); + type = Types.JSON; + length = Packet.lengthCodedStringLength(value, encoding); + return { value, type, length, writer }; + } else if (Buffer.isBuffer(value)) { + length = Packet.lengthCodedNumberLength(value.length) + value.length; + writer = Packet.prototype.writeLengthCodedBuffer; + type = Types.VAR_STRING; + return { value, type, length, writer }; + } + switch(parameterHints.columnType) { case Types.TINY: value = value | 0; @@ -97,7 +127,7 @@ class Execute { toPacket() { - if (this.statement.parameters && this.parameters.length !== this.statement.parameters.length ) { + if ((this.statement.parameters.length > 0 && !this.parameters) || (this.parameters && this.parameters.length !== this.statement.parameters.length)) { throw new TypeError(`Incorrect number of bind parameters, expected ${this.statement.parameters.length} but supplied ${this.parameters.length}`); } diff --git a/test/integration/connection/test-execute-bind-boolean.js b/test/integration/connection/test-execute-bind-boolean.js index 39ec561803..36af266b81 100644 --- a/test/integration/connection/test-execute-bind-boolean.js +++ b/test/integration/connection/test-execute-bind-boolean.js @@ -18,5 +18,5 @@ connection.execute( ); process.on('exit', () => { - assert.deepEqual(rows, [{ trueValue: 1, falseValue: 0 }]); + assert.deepEqual(rows, [{ trueValue: 'true', falseValue: 'false' }]); }); From 04cee652cf5ab3ed4df2657609458b077abbb49b Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Tue, 12 Oct 2021 08:12:40 +1100 Subject: [PATCH 7/7] lint --- lib/packets/execute.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/packets/execute.js b/lib/packets/execute.js index 4cdf84f1cf..83a586ba85 100644 --- a/lib/packets/execute.js +++ b/lib/packets/execute.js @@ -43,12 +43,16 @@ function toParameter(value, encoding, timezone, parameterHints) { return Packet.prototype.writeDate.call(this, value, timezone); }; return { value, type, length, writer }; - } else if (isJSON(value)) { + } + + if (isJSON(value)) { value = JSON.stringify(value); type = Types.JSON; length = Packet.lengthCodedStringLength(value, encoding); return { value, type, length, writer }; - } else if (Buffer.isBuffer(value)) { + } + + if (Buffer.isBuffer(value)) { length = Packet.lengthCodedNumberLength(value.length) + value.length; writer = Packet.prototype.writeLengthCodedBuffer; type = Types.VAR_STRING;