Skip to content

Commit 01485d5

Browse files
sidoraressidorares
authored
Merge pull request #1402 from testn/fix-parser
Avoid leaking TextRow/BinaryRow object outside the framework
2 parents f0812c4 + 01e1bf1 commit 01485d5

File tree

7 files changed

+87
-82
lines changed

7 files changed

+87
-82
lines changed

‎lib/commands/execute.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ class Execute extends Command {
8585
if (!packet.isEOF()) {
8686
return connection.protocolError('Expected EOF packet');
8787
}
88-
this._rowParser = this.buildParserFromFields(
88+
this._rowParser = new (this.buildParserFromFields(
8989
this._fields[this._resultIndex],
9090
connection
91-
);
91+
))();
9292
return Execute.prototype.row;
9393
}
9494
}

‎lib/commands/query.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const Command = require('./command.js');
99
const Packets = require('../packets/index.js');
1010
const getTextParser = require('../parsers/text_parser.js');
1111
const ServerStatus = require('../constants/server_status.js');
12-
const CharsetToEncoding = require('../constants/charset_encodings.js');
1312

1413
const EmptyPacket = new Packets.Packet(0, Buffer.allocUnsafe(4), 0, 4);
1514

@@ -213,7 +212,7 @@ class Query extends Command {
213212
if (this._receivedFieldsCount === this._fieldCount) {
214213
const fields = this._fields[this._resultIndex];
215214
this.emit('fields', fields);
216-
this._rowParser = getTextParser(fields, this.options, connection.config);
215+
this._rowParser = new (getTextParser(fields, this.options, connection.config))();
217216
return Query.prototype.fieldsEOF;
218217
}
219218
return Query.prototype.readField;
@@ -240,11 +239,10 @@ class Query extends Command {
240239
}
241240
let row;
242241
try {
243-
row = new this._rowParser(
242+
row = this._rowParser.next(
244243
packet,
245244
this._fields[this._resultIndex],
246-
this.options,
247-
CharsetToEncoding
245+
this.options
248246
);
249247
} catch (err) {
250248
this._localStreamError = err;

‎lib/parsers/binary_parser.js

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ function readCodeFor(field, config, options, fieldNum) {
7474
if (field.characterSet === Charsets.BINARY) {
7575
return 'packet.readLengthCodedBuffer();';
7676
}
77-
return `packet.readLengthCodedString(CharsetToEncoding[fields[${fieldNum}].characterSet])`;
77+
return `packet.readLengthCodedString(fields[${fieldNum}].encoding)`;
7878
}
7979
}
8080

@@ -87,12 +87,16 @@ function compile(fields, options, config) {
8787
/* eslint-disable no-spaced-func */
8888
/* eslint-disable no-unexpected-multiline */
8989

90-
parserFn('(function(){')(
91-
'return function BinaryRow(packet, fields, options, CharsetToEncoding) {'
92-
);
90+
parserFn('(function(){');
91+
parserFn('return class BinaryRow {');
92+
parserFn('constructor() {');
93+
parserFn('}');
9394

95+
parserFn('next(packet, fields, options) {');
9496
if (options.rowsAsArray) {
9597
parserFn(`const result = new Array(${fields.length});`);
98+
} else {
99+
parserFn("const result = {};");
96100
}
97101

98102
const resultTables = {};
@@ -104,7 +108,7 @@ function compile(fields, options, config) {
104108
}
105109
resultTablesArray = Object.keys(resultTables);
106110
for (i = 0; i < resultTablesArray.length; i++) {
107-
parserFn(`this[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
111+
parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
108112
}
109113
}
110114

@@ -125,16 +129,16 @@ function compile(fields, options, config) {
125129

126130
if (typeof options.nestTables === 'string') {
127131
tableName = helpers.srcEscape(fields[i].table);
128-
lvalue = `this[${helpers.srcEscape(
132+
lvalue = `result[${helpers.srcEscape(
129133
fields[i].table + options.nestTables + fields[i].name
130134
)}]`;
131135
} else if (options.nestTables === true) {
132136
tableName = helpers.srcEscape(fields[i].table);
133-
lvalue = `this[${tableName}][${fieldName}]`;
137+
lvalue = `result[${tableName}][${fieldName}]`;
134138
} else if (options.rowsAsArray) {
135139
lvalue = `result[${i.toString(10)}]`;
136140
} else {
137-
lvalue = `this[${helpers.srcEscape(fields[i].name)}]`;
141+
lvalue = `result[${helpers.srcEscape(fields[i].name)}]`;
138142
}
139143

140144
// TODO: this used to be an optimisation ( if column marked as NOT_NULL don't include code to check null
@@ -158,10 +162,8 @@ function compile(fields, options, config) {
158162
}
159163
}
160164

161-
if (options.rowsAsArray) {
162-
parserFn('return result;');
163-
}
164-
165+
parserFn('return result;');
166+
parserFn('}');
165167
parserFn('};')('})()');
166168

167169
/* eslint-enable no-trailing-spaces */

‎lib/parsers/parser_cache.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,7 @@ function keyFromFields(type, fields, options, config) {
2020
`/${options.dateStrings}`;
2121
for (let i = 0; i < fields.length; ++i) {
2222
const field = fields[i];
23-
res += `/${field.name}:${field.columnType}:${field.flags}:${
24-
field.characterSet
25-
}`;
26-
27-
if (options.nestTables) {
28-
res += `:${field.table}`
29-
}
23+
res += `/${field.name}:${field.columnType}:${field.length}:${field.schema}:${field.table}:${field.flags}:${field.characterSet}`;
3024
}
3125
return res;
3226
}

‎lib/parsers/text_parser.js

Lines changed: 63 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -70,27 +70,6 @@ function readCodeFor(type, charset, encodingExpr, config, options) {
7070
}
7171

7272
function compile(fields, options, config) {
73-
// node-mysql typeCast compatibility wrapper
74-
// see https://github.com/mysqljs/mysql/blob/96fdd0566b654436624e2375c7b6604b1f50f825/lib/protocol/packets/Field.js
75-
function wrap(field, type, packet, encoding) {
76-
return {
77-
type: type,
78-
length: field.columnLength,
79-
db: field.schema,
80-
table: field.table,
81-
name: field.name,
82-
string: function() {
83-
return packet.readLengthCodedString(encoding);
84-
},
85-
buffer: function() {
86-
return packet.readLengthCodedBuffer();
87-
},
88-
geometry: function() {
89-
return packet.parseGeometryValue();
90-
}
91-
};
92-
}
93-
9473
// use global typeCast if current query doesn't specify one
9574
if (
9675
typeof config.typeCast === 'function' &&
@@ -100,77 +79,110 @@ function compile(fields, options, config) {
10079
}
10180

10281
const parserFn = genFunc();
103-
let i = 0;
10482

10583
/* eslint-disable no-trailing-spaces */
10684
/* eslint-disable no-spaced-func */
10785
/* eslint-disable no-unexpected-multiline */
10886
parserFn('(function () {')(
109-
'return function TextRow(packet, fields, options, CharsetToEncoding) {'
87+
'return class TextRow {'
11088
);
11189

112-
if (options.rowsAsArray) {
113-
parserFn(`const result = new Array(${fields.length})`);
90+
// constructor method
91+
parserFn('constructor() {');
92+
// node-mysql typeCast compatibility wrapper
93+
// see https://github.com/mysqljs/mysql/blob/96fdd0566b654436624e2375c7b6604b1f50f825/lib/protocol/packets/Field.js
94+
if (typeof options.typeCast === 'function') {
95+
parserFn('const _this = this;');
96+
for(let i=0; i<fields.length; ++i) {
97+
const field = fields[i];
98+
const encodingExpr = helpers.srcEscape(field.encoding);
99+
const readCode = readCodeFor(
100+
fields[i].columnType,
101+
fields[i].characterSet,
102+
encodingExpr,
103+
config,
104+
options
105+
);
106+
parserFn(`this.wrap${i} = {
107+
type: ${helpers.srcEscape(typeNames[field.columnType])},
108+
length: ${helpers.srcEscape(field.columnLength)},
109+
db: ${helpers.srcEscape(field.schema)},
110+
table: ${helpers.srcEscape(field.table)},
111+
name: ${helpers.srcEscape(field.name)},
112+
string: function() {
113+
return _this.packet.readLengthCodedString(${encodingExpr});
114+
},
115+
buffer: function() {
116+
return _this.packet.readLengthCodedBuffer();
117+
},
118+
geometry: function() {
119+
return _this.packet.parseGeometryValue();
120+
},
121+
readNext: function() {
122+
return _this.${readCode};
123+
}
124+
};`);
125+
}
114126
}
127+
parserFn('}');
115128

116-
if (typeof options.typeCast === 'function') {
117-
parserFn(`const wrap = ${wrap.toString()}`);
129+
// next method
130+
parserFn('next(packet, fields, options) {');
131+
parserFn("this.packet = packet;");
132+
if (options.rowsAsArray) {
133+
parserFn(`const result = new Array(${fields.length});`);
134+
} else {
135+
parserFn("const result = {};");
118136
}
119137

120138
const resultTables = {};
121139
let resultTablesArray = [];
122140

123141
if (options.nestTables === true) {
124-
for (i = 0; i < fields.length; i++) {
142+
for (let i=0; i < fields.length; i++) {
125143
resultTables[fields[i].table] = 1;
126144
}
127145
resultTablesArray = Object.keys(resultTables);
128-
for (i = 0; i < resultTablesArray.length; i++) {
129-
parserFn(`this[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
146+
for (let i=0; i < resultTablesArray.length; i++) {
147+
parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
130148
}
131149
}
132150

133151
let lvalue = '';
134152
let fieldName = '';
135-
for (i = 0; i < fields.length; i++) {
153+
for (let i = 0; i < fields.length; i++) {
136154
fieldName = helpers.srcEscape(fields[i].name);
137155
parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
138156
if (typeof options.nestTables === 'string') {
139-
lvalue = `this[${helpers.srcEscape(
157+
lvalue = `result[${helpers.srcEscape(
140158
fields[i].table + options.nestTables + fields[i].name
141159
)}]`;
142160
} else if (options.nestTables === true) {
143-
lvalue = `this[${helpers.srcEscape(fields[i].table)}][${fieldName}]`;
161+
lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`;
144162
} else if (options.rowsAsArray) {
145163
lvalue = `result[${i.toString(10)}]`;
146164
} else {
147-
lvalue = `this[${fieldName}]`;
165+
lvalue = `result[${fieldName}]`;
148166
}
149-
const encodingExpr = `CharsetToEncoding[fields[${i}].characterSet]`;
150-
const readCode = readCodeFor(
151-
fields[i].columnType,
152-
fields[i].characterSet,
153-
encodingExpr,
154-
config,
155-
options
156-
);
157167
if (typeof options.typeCast === 'function') {
158-
parserFn(
159-
`${lvalue} = options.typeCast(wrap(fields[${i}], ${helpers.srcEscape(
160-
typeNames[fields[i].columnType]
161-
)}, packet, ${encodingExpr}), function() { return ${readCode};})`
162-
);
168+
parserFn(`${lvalue} = options.typeCast(this.wrap${i}, this.wrap${i}.readNext);`);
163169
} else if (options.typeCast === false) {
164170
parserFn(`${lvalue} = packet.readLengthCodedBuffer();`);
165171
} else {
172+
const encodingExpr = `fields[${i}].encoding`;
173+
const readCode = readCodeFor(
174+
fields[i].columnType,
175+
fields[i].characterSet,
176+
encodingExpr,
177+
config,
178+
options
179+
);
166180
parserFn(`${lvalue} = ${readCode};`);
167181
}
168182
}
169183

170-
if (options.rowsAsArray) {
171-
parserFn('return result;');
172-
}
173-
184+
parserFn('return result;');
185+
parserFn('}');
174186
parserFn('};')('})()');
175187

176188
/* eslint-enable no-trailing-spaces */

‎test/integration/connection/test-multiple-results.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,7 @@ function do_test(testIndex) {
113113
_numResults = 1;
114114
} else if (_rows.length > 0) {
115115
if (
116-
_rows.constructor.name === 'Array' &&
117-
_rows[0].constructor.name === 'TextRow'
116+
_rows.constructor.name === 'Array'
118117
) {
119118
_numResults = 1;
120119
}

‎test/unit/commands/test-query.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ const testQuery = new Query({}, (err, res) => {
99
assert.equal(res, null);
1010
});
1111

12-
testQuery._rowParser = class FailingRowParser {
13-
constructor() {
12+
testQuery._rowParser = new class FailingRowParser {
13+
next() {
1414
throw testError;
1515
}
16-
};
16+
}();
1717

1818
testQuery.row({
1919
isEOF: () => false

0 commit comments

Comments
 (0)