From 225c5c3f16073b425740147541e8045d13366fdf Mon Sep 17 00:00:00 2001 From: testn Date: Thu, 7 Oct 2021 21:01:12 +0700 Subject: [PATCH 1/4] Avoid leaking TextRow/BinaryRow object outside the framework Avoid leaking TextRow/BinaryRow object outside the framework by returning a new object every time instead of returning this --- lib/commands/query.js | 2 +- lib/parsers/binary_parser.js | 14 +++++++------- lib/parsers/text_parser.js | 14 +++++++------- .../connection/test-multiple-results.js | 3 +-- test/unit/commands/test-query.js | 8 +++----- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/lib/commands/query.js b/lib/commands/query.js index 344b4abc36..d26eefbdd6 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -238,7 +238,7 @@ class Query extends Command { } let row; try { - row = new this._rowParser( + row = this._rowParser( packet, this._fields[this._resultIndex], this.options, diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js index d20c87cb2d..b15af1164e 100644 --- a/lib/parsers/binary_parser.js +++ b/lib/parsers/binary_parser.js @@ -93,6 +93,8 @@ function compile(fields, options, config) { if (options.rowsAsArray) { parserFn(`const result = new Array(${fields.length});`); + } else { + parserFn("const result = {};"); } const resultTables = {}; @@ -104,7 +106,7 @@ function compile(fields, options, config) { } resultTablesArray = Object.keys(resultTables); for (i = 0; i < resultTablesArray.length; i++) { - parserFn(`this[${helpers.srcEscape(resultTablesArray[i])}] = {};`); + parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`); } } @@ -125,16 +127,16 @@ function compile(fields, options, config) { if (typeof options.nestTables === 'string') { tableName = helpers.srcEscape(fields[i].table); - lvalue = `this[${helpers.srcEscape( + lvalue = `result[${helpers.srcEscape( fields[i].table + options.nestTables + fields[i].name )}]`; } else if (options.nestTables === true) { tableName = helpers.srcEscape(fields[i].table); - lvalue = `this[${tableName}][${fieldName}]`; + lvalue = `result[${tableName}][${fieldName}]`; } else if (options.rowsAsArray) { lvalue = `result[${i.toString(10)}]`; } else { - lvalue = `this[${helpers.srcEscape(fields[i].name)}]`; + lvalue = `result[${helpers.srcEscape(fields[i].name)}]`; } // TODO: this used to be an optimisation ( if column marked as NOT_NULL don't include code to check null @@ -158,9 +160,7 @@ function compile(fields, options, config) { } } - if (options.rowsAsArray) { - parserFn('return result;'); - } + parserFn('return result;'); parserFn('};')('})()'); diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js index eb777d8eb7..c9ba4b07dd 100644 --- a/lib/parsers/text_parser.js +++ b/lib/parsers/text_parser.js @@ -111,6 +111,8 @@ function compile(fields, options, config) { if (options.rowsAsArray) { parserFn(`const result = new Array(${fields.length})`); + } else { + parserFn("const result = {}"); } if (typeof options.typeCast === 'function') { @@ -126,7 +128,7 @@ function compile(fields, options, config) { } resultTablesArray = Object.keys(resultTables); for (i = 0; i < resultTablesArray.length; i++) { - parserFn(`this[${helpers.srcEscape(resultTablesArray[i])}] = {};`); + parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`); } } @@ -136,15 +138,15 @@ function compile(fields, options, config) { fieldName = helpers.srcEscape(fields[i].name); parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`); if (typeof options.nestTables === 'string') { - lvalue = `this[${helpers.srcEscape( + lvalue = `result[${helpers.srcEscape( fields[i].table + options.nestTables + fields[i].name )}]`; } else if (options.nestTables === true) { - lvalue = `this[${helpers.srcEscape(fields[i].table)}][${fieldName}]`; + lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`; } else if (options.rowsAsArray) { lvalue = `result[${i.toString(10)}]`; } else { - lvalue = `this[${fieldName}]`; + lvalue = `result[${fieldName}]`; } const encodingExpr = `CharsetToEncoding[fields[${i}].characterSet]`; const readCode = readCodeFor( @@ -167,9 +169,7 @@ function compile(fields, options, config) { } } - if (options.rowsAsArray) { - parserFn('return result;'); - } + parserFn('return result;'); parserFn('};')('})()'); diff --git a/test/integration/connection/test-multiple-results.js b/test/integration/connection/test-multiple-results.js index ff48b89286..5e6f8a9dd4 100644 --- a/test/integration/connection/test-multiple-results.js +++ b/test/integration/connection/test-multiple-results.js @@ -113,8 +113,7 @@ function do_test(testIndex) { _numResults = 1; } else if (_rows.length > 0) { if ( - _rows.constructor.name === 'Array' && - _rows[0].constructor.name === 'TextRow' + _rows.constructor.name === 'Array' ) { _numResults = 1; } diff --git a/test/unit/commands/test-query.js b/test/unit/commands/test-query.js index c10cd27b57..81c5b87cbf 100644 --- a/test/unit/commands/test-query.js +++ b/test/unit/commands/test-query.js @@ -9,11 +9,9 @@ const testQuery = new Query({}, (err, res) => { assert.equal(res, null); }); -testQuery._rowParser = class FailingRowParser { - constructor() { - throw testError; - } -}; +testQuery._rowParser = function FailingRowParser() { + throw testError; +} testQuery.row({ isEOF: () => false From 0ba4f87eb9a08b43d993c8f3db1a524c96933d8f Mon Sep 17 00:00:00 2001 From: testn Date: Sat, 9 Oct 2021 20:38:55 +0700 Subject: [PATCH 2/4] Convert function into class to cache the field definition --- lib/commands/execute.js | 4 +- lib/commands/query.js | 4 +- lib/parsers/binary_parser.js | 10 +-- lib/parsers/parser_cache.js | 8 +-- lib/parsers/text_parser.js | 104 +++++++++++++++++-------------- test/unit/commands/test-query.js | 8 ++- 6 files changed, 74 insertions(+), 64 deletions(-) diff --git a/lib/commands/execute.js b/lib/commands/execute.js index d5c2d8ab79..8ab7304b5b 100644 --- a/lib/commands/execute.js +++ b/lib/commands/execute.js @@ -85,10 +85,10 @@ class Execute extends Command { if (!packet.isEOF()) { return connection.protocolError('Expected EOF packet'); } - this._rowParser = this.buildParserFromFields( + this._rowParser = new (this.buildParserFromFields( this._fields[this._resultIndex], connection - ); + ))(); return Execute.prototype.row; } } diff --git a/lib/commands/query.js b/lib/commands/query.js index d26eefbdd6..f4c690605d 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -212,7 +212,7 @@ class Query extends Command { if (this._receivedFieldsCount === this._fieldCount) { const fields = this._fields[this._resultIndex]; this.emit('fields', fields); - this._rowParser = getTextParser(fields, this.options, connection.config); + this._rowParser = new (getTextParser(fields, this.options, connection.config))(); return Query.prototype.fieldsEOF; } return Query.prototype.readField; @@ -238,7 +238,7 @@ class Query extends Command { } let row; try { - row = this._rowParser( + row = this._rowParser.next( packet, this._fields[this._resultIndex], this.options, diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js index b15af1164e..f5f64bf360 100644 --- a/lib/parsers/binary_parser.js +++ b/lib/parsers/binary_parser.js @@ -87,10 +87,12 @@ function compile(fields, options, config) { /* eslint-disable no-spaced-func */ /* eslint-disable no-unexpected-multiline */ - parserFn('(function(){')( - 'return function BinaryRow(packet, fields, options, CharsetToEncoding) {' - ); + parserFn('(function(){'); + parserFn('return class BinaryRow {'); + parserFn('constructor() {'); + parserFn('}'); + parserFn('next(packet, fields, options) {'); if (options.rowsAsArray) { parserFn(`const result = new Array(${fields.length});`); } else { @@ -161,7 +163,7 @@ function compile(fields, options, config) { } parserFn('return result;'); - + parserFn('}'); parserFn('};')('})()'); /* eslint-enable no-trailing-spaces */ diff --git a/lib/parsers/parser_cache.js b/lib/parsers/parser_cache.js index 2fa8cf066a..509b5c6736 100644 --- a/lib/parsers/parser_cache.js +++ b/lib/parsers/parser_cache.js @@ -20,13 +20,7 @@ function keyFromFields(type, fields, options, config) { `/${options.dateStrings}`; for (let i = 0; i < fields.length; ++i) { const field = fields[i]; - res += `/${field.name}:${field.columnType}:${field.flags}:${ - field.characterSet - }`; - - if (options.nestTables) { - res += `:${field.table}` - } + res += `/${field.name}:${field.columnType}:${field.length}:${field.schema}:${field.table}:${field.flags}:${field.characterSet}`; } return res; } diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js index c9ba4b07dd..3e7906c767 100644 --- a/lib/parsers/text_parser.js +++ b/lib/parsers/text_parser.js @@ -70,27 +70,6 @@ function readCodeFor(type, charset, encodingExpr, config, options) { } function compile(fields, options, config) { - // node-mysql typeCast compatibility wrapper - // see https://github.com/mysqljs/mysql/blob/96fdd0566b654436624e2375c7b6604b1f50f825/lib/protocol/packets/Field.js - function wrap(field, type, packet, encoding) { - return { - type: type, - length: field.columnLength, - db: field.schema, - table: field.table, - name: field.name, - string: function() { - return packet.readLengthCodedString(encoding); - }, - buffer: function() { - return packet.readLengthCodedBuffer(); - }, - geometry: function() { - return packet.parseGeometryValue(); - } - }; - } - // use global typeCast if current query doesn't specify one if ( typeof config.typeCast === 'function' && @@ -100,41 +79,78 @@ function compile(fields, options, config) { } const parserFn = genFunc(); - let i = 0; /* eslint-disable no-trailing-spaces */ /* eslint-disable no-spaced-func */ /* eslint-disable no-unexpected-multiline */ parserFn('(function () {')( - 'return function TextRow(packet, fields, options, CharsetToEncoding) {' + 'return class TextRow {' ); - if (options.rowsAsArray) { - parserFn(`const result = new Array(${fields.length})`); - } else { - parserFn("const result = {}"); + // constructor method + parserFn('constructor() {'); + // node-mysql typeCast compatibility wrapper + // see https://github.com/mysqljs/mysql/blob/96fdd0566b654436624e2375c7b6604b1f50f825/lib/protocol/packets/Field.js + if (typeof options.typeCast === 'function') { + parserFn('const _this = this;'); + for(let i=0; i { assert.equal(res, null); }); -testQuery._rowParser = function FailingRowParser() { - throw testError; -} +testQuery._rowParser = new class FailingRowParser { + next() { + throw testError; + } +}(); testQuery.row({ isEOF: () => false From d734d08b1778e5bcd4e8034c42b05b2b80772bc2 Mon Sep 17 00:00:00 2001 From: testn Date: Sat, 9 Oct 2021 20:56:41 +0700 Subject: [PATCH 3/4] Remove unnecessary reference to CharsetEncoding --- lib/commands/query.js | 4 +--- lib/parsers/binary_parser.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/commands/query.js b/lib/commands/query.js index f4c690605d..cec75463f8 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -9,7 +9,6 @@ const Command = require('./command.js'); const Packets = require('../packets/index.js'); const getTextParser = require('../parsers/text_parser.js'); const ServerStatus = require('../constants/server_status.js'); -const CharsetToEncoding = require('../constants/charset_encodings.js'); const EmptyPacket = new Packets.Packet(0, Buffer.allocUnsafe(4), 0, 4); @@ -241,8 +240,7 @@ class Query extends Command { row = this._rowParser.next( packet, this._fields[this._resultIndex], - this.options, - CharsetToEncoding + this.options ); } catch (err) { this._localStreamError = err; diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js index f5f64bf360..bbd29596ec 100644 --- a/lib/parsers/binary_parser.js +++ b/lib/parsers/binary_parser.js @@ -74,7 +74,7 @@ function readCodeFor(field, config, options, fieldNum) { if (field.characterSet === Charsets.BINARY) { return 'packet.readLengthCodedBuffer();'; } - return `packet.readLengthCodedString(CharsetToEncoding[fields[${fieldNum}].characterSet])`; + return `packet.readLengthCodedString(fields[${fieldNum}].encoding)`; } } From 01e1bf116facd8f4721fbb2f212a5fc4eb672d6b Mon Sep 17 00:00:00 2001 From: testn Date: Sat, 9 Oct 2021 23:56:49 +0700 Subject: [PATCH 4/4] Missing one file --- lib/parsers/text_parser.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js index 3e7906c767..2036caed6e 100644 --- a/lib/parsers/text_parser.js +++ b/lib/parsers/text_parser.js @@ -95,7 +95,7 @@ function compile(fields, options, config) { parserFn('const _this = this;'); for(let i=0; i