Skip to content

Commit

Permalink
Merge pull request #1402 from testn/fix-parser
Browse files Browse the repository at this point in the history 
Avoid leaking TextRow/BinaryRow object outside the framework
  • Loading branch information
@sidorares
sidorares authored Oct 12, 2021
2 parents f0812c4 + 01e1bf1 commit 01485d5
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 82 deletions.
4 changes: 2 additions & 2 deletions lib/commands/execute.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
8 changes: 3 additions & 5 deletions lib/commands/query.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -213,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;
Expand All @@ -240,11 +239,10 @@ class Query extends Command {
}
let row;
try {
row = new this._rowParser(
row = this._rowParser.next(
packet,
this._fields[this._resultIndex],
this.options,
CharsetToEncoding
this.options
);
} catch (err) {
this._localStreamError = err;
Expand Down
26 changes: 14 additions & 12 deletions lib/parsers/binary_parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)`;
}
}

Expand All @@ -87,12 +87,16 @@ 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 {
parserFn("const result = {};");
}

const resultTables = {};
Expand All @@ -104,7 +108,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])}] = {};`);
}
}

Expand All @@ -125,16 +129,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
Expand All @@ -158,10 +162,8 @@ function compile(fields, options, config) {
}
}

if (options.rowsAsArray) {
parserFn('return result;');
}

parserFn('return result;');
parserFn('}');
parserFn('};')('})()');

/* eslint-enable no-trailing-spaces */
Expand Down
8 changes: 1 addition & 7 deletions lib/parsers/parser_cache.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
114 changes: 63 additions & 51 deletions lib/parsers/text_parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' &&
Expand All @@ -100,77 +79,110 @@ 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})`);
// 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<fields.length; ++i) {
const field = fields[i];
const encodingExpr = helpers.srcEscape(field.encoding);
const readCode = readCodeFor(
fields[i].columnType,
fields[i].characterSet,
encodingExpr,
config,
options
);
parserFn(`this.wrap${i} = {
type: ${helpers.srcEscape(typeNames[field.columnType])},
length: ${helpers.srcEscape(field.columnLength)},
db: ${helpers.srcEscape(field.schema)},
table: ${helpers.srcEscape(field.table)},
name: ${helpers.srcEscape(field.name)},
string: function() {
return _this.packet.readLengthCodedString(${encodingExpr});
},
buffer: function() {
return _this.packet.readLengthCodedBuffer();
},
geometry: function() {
return _this.packet.parseGeometryValue();
},
readNext: function() {
return _this.${readCode};
}
};`);
}
}
parserFn('}');

if (typeof options.typeCast === 'function') {
parserFn(`const wrap = ${wrap.toString()}`);
// next method
parserFn('next(packet, fields, options) {');
parserFn("this.packet = packet;");
if (options.rowsAsArray) {
parserFn(`const result = new Array(${fields.length});`);
} else {
parserFn("const result = {};");
}

const resultTables = {};
let resultTablesArray = [];

if (options.nestTables === true) {
for (i = 0; i < fields.length; i++) {
for (let i=0; i < fields.length; i++) {
resultTables[fields[i].table] = 1;
}
resultTablesArray = Object.keys(resultTables);
for (i = 0; i < resultTablesArray.length; i++) {
parserFn(`this[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
for (let i=0; i < resultTablesArray.length; i++) {
parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
}
}

let lvalue = '';
let fieldName = '';
for (i = 0; i < fields.length; i++) {
for (let i = 0; i < fields.length; i++) {
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(
fields[i].columnType,
fields[i].characterSet,
encodingExpr,
config,
options
);
if (typeof options.typeCast === 'function') {
parserFn(
`${lvalue} = options.typeCast(wrap(fields[${i}], ${helpers.srcEscape(
typeNames[fields[i].columnType]
)}, packet, ${encodingExpr}), function() { return ${readCode};})`
);
parserFn(`${lvalue} = options.typeCast(this.wrap${i}, this.wrap${i}.readNext);`);
} else if (options.typeCast === false) {
parserFn(`${lvalue} = packet.readLengthCodedBuffer();`);
} else {
const encodingExpr = `fields[${i}].encoding`;
const readCode = readCodeFor(
fields[i].columnType,
fields[i].characterSet,
encodingExpr,
config,
options
);
parserFn(`${lvalue} = ${readCode};`);
}
}

if (options.rowsAsArray) {
parserFn('return result;');
}

parserFn('return result;');
parserFn('}');
parserFn('};')('})()');

/* eslint-enable no-trailing-spaces */
Expand Down
3 changes: 1 addition & 2 deletions test/integration/connection/test-multiple-results.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions test/unit/commands/test-query.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ const testQuery = new Query({}, (err, res) => {
assert.equal(res, null);
});

testQuery._rowParser = class FailingRowParser {
constructor() {
testQuery._rowParser = new class FailingRowParser {
next() {
throw testError;
}
};
}();

testQuery.row({
isEOF: () => false
Expand Down

0 comments on commit 01485d5

Please sign in to comment.