From 1ace562db276fe4d1d49b5659d197b8388b6ef6a Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Thu, 9 Dec 2021 12:42:52 +0100 Subject: [PATCH 1/8] CONJS-181 - fix local infile file name validation for windows #183 --- lib/cmd/resultset.js | 2 +- lib/misc/parse.js | 11 +++++-- test/integration/test-local-infile.js | 42 ++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/cmd/resultset.js b/lib/cmd/resultset.js index d5120de7..1659f42e 100644 --- a/lib/cmd/resultset.js +++ b/lib/cmd/resultset.js @@ -516,7 +516,7 @@ class ResultSet extends Command { this.sql, false, info, - '45034', + 'HY000', Errors.ER_LOCAL_INFILE_WRONG_FILENAME ); diff --git a/lib/misc/parse.js b/lib/misc/parse.js index 3b680d8d..ba4fea04 100644 --- a/lib/misc/parse.js +++ b/lib/misc/parse.js @@ -1005,9 +1005,13 @@ module.exports.splitRewritableNamedParameterQuery = function (sql, initialValues * @returns {boolean} is filename corresponding to query */ module.exports.validateFileName = function (sql, parameters, fileName) { + // in case of windows, file name in query are escaped + // so for example LOAD DATA LOCAL INFILE 'C:\\Temp\\myFile.txt' ... + // but server return 'C:\Temp\myFile.txt' + // so with regex escaped, must test LOAD DATA LOCAL INFILE 'C:\\\\Temp\\\\myFile.txt' let queryValidator = new RegExp( "^(\\s*\\/\\*([^\\*]|\\*[^\\/])*\\*\\/)*\\s*LOAD\\s+DATA\\s+((LOW_PRIORITY|CONCURRENT)\\s+)?LOCAL\\s+INFILE\\s+'" + - fileName + + fileName.replace(/\\/g, '\\\\\\\\').replace('.', '\\.') + "'", 'i' ); @@ -1019,7 +1023,10 @@ module.exports.validateFileName = function (sql, parameters, fileName) { 'i' ); if (queryValidator.test(sql) && parameters.length > 0) { - return parameters[0].toLowerCase() === fileName.toLowerCase(); + if (Array.isArray(parameters)) { + return parameters[0].toLowerCase() === fileName.toLowerCase(); + } + return parameters.toLowerCase() === fileName.toLowerCase(); } } return false; diff --git a/test/integration/test-local-infile.js b/test/integration/test-local-infile.js index 2c2660f6..57c19690 100644 --- a/test/integration/test-local-infile.js +++ b/test/integration/test-local-infile.js @@ -178,7 +178,7 @@ describe('local-infile', () => { await conn.beginTransaction(); await conn.query( "LOAD DATA LOCAL INFILE '" + - smallFileName.replace(/\\/g, '/') + + smallFileName.replace(/\\/g, '\\\\') + "' INTO TABLE smallLocalInfile FIELDS TERMINATED BY ',' (id, test)" ); const rows2 = await conn.query('SELECT * FROM smallLocalInfile'); @@ -189,6 +189,46 @@ describe('local-infile', () => { conn.end(); }); + it('small local infile with parameter', async function () { + const self = this; + const rows = await shareConn.query('select @@local_infile'); + if ( + rows[0]['@@local_infile'] === 0 || + (!shareConn.info.isMariaDB() && shareConn.info.hasMinVersion(8, 0, 0)) || + process.env.srv === 'skysql' || + process.env.srv === 'skysql-ha' + ) { + return self.skip(); + } + await new Promise(function (resolve, reject) { + fs.writeFile(smallFileName, '1,hello\n2,world\n', 'utf8', function (err) { + if (err) reject(err); + else resolve(); + }); + }); + const conn = await base.createConnection({ permitLocalInfile: true }); + await conn.query('DROP TABLE IF EXISTS smallLocalInfile'); + await conn.query('CREATE TABLE smallLocalInfile(id int, test varchar(100))'); + await conn.beginTransaction(); + await conn.query( + "LOAD DATA LOCAL INFILE ? INTO TABLE smallLocalInfile FIELDS TERMINATED BY ',' (id, test)", + smallFileName + ); + await conn.query( + "LOAD DATA LOCAL INFILE ? INTO TABLE smallLocalInfile FIELDS TERMINATED BY ',' (id, test)", + [smallFileName] + ); + + const rows2 = await conn.query('SELECT * FROM smallLocalInfile'); + assert.deepEqual(rows2, [ + { id: 1, test: 'hello' }, + { id: 2, test: 'world' }, + { id: 1, test: 'hello' }, + { id: 2, test: 'world' } + ]); + conn.end(); + }); + it('small local infile with non supported node.js encoding', async function () { const self = this; const rows = await shareConn.query('select @@local_infile'); From 5d0930a4a128c6c6f031e9ea1d338acfff239566 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Thu, 9 Dec 2021 12:51:19 +0100 Subject: [PATCH 2/8] misc - ensuring test stream close --- test/integration/test-batch.js | 15 +++++++++++++-- types/index.d.ts | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/integration/test-batch.js b/test/integration/test-batch.js index 01aebc8f..328898f7 100644 --- a/test/integration/test-batch.js +++ b/test/integration/test-batch.js @@ -645,6 +645,8 @@ describe('batch', () => { assert.equal(err.code, 'ER_NO_SUCH_TABLE'); clearTimeout(timeout); conn.end(); + stream1.close(); + stream2.close(); } }; @@ -703,10 +705,14 @@ describe('batch', () => { }; const bigBatchErrorWithStreams = async (useCompression, useBulk) => { + const streams = []; const values = []; for (let i = 0; i < 1000000; i++) { - if (i % 100000 === 0) values.push([i, fs.createReadStream(fileName), i * 2]); - else values.push([i, str, i * 2]); + if (i % 100000 === 0) { + const st = fs.createReadStream(fileName); + streams.push(st); + values.push([i, st, i * 2]); + } else values.push([i, str, i * 2]); } const conn = await base.createConnection({ @@ -725,6 +731,9 @@ describe('batch', () => { assert.deepEqual(rows, [{ 1: 1 }]); conn.end(); clearTimeout(timeout); + for (const element of streams) { + element.close(); + } } }; @@ -1001,6 +1010,8 @@ describe('batch', () => { assert.equal(err.code, 'ER_NO_SUCH_TABLE'); clearTimeout(timeout); conn.end(); + stream1.close(); + stream2.close(); } }; diff --git a/types/index.d.ts b/types/index.d.ts index 1566e576..a3c59628 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -674,7 +674,7 @@ export interface PoolCluster { remove(pattern: string): void; getConnection(pattern?: string, selector?: string): Promise; - on(ev: 'remove', callback: (nodekey: string) => void): PoolCluster + on(ev: 'remove', callback: (nodekey: string) => void): PoolCluster; } export interface UpsertResult { From 16d45cd7b3d2201a252f9199afbb08f794ea2a69 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Thu, 9 Dec 2021 15:31:34 +0100 Subject: [PATCH 3/8] CONJS-183 - change default connection timeout value to 1000ms --- documentation/connection-options.md | 2 +- documentation/promise-api.md | 2 +- lib/config/connection-options.js | 2 +- test/integration/test-pool.js | 2 -- test/unit/config/test-options.js | 2 +- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/documentation/connection-options.md b/documentation/connection-options.md index 6b03d1e9..dddf13a9 100644 --- a/documentation/connection-options.md +++ b/documentation/connection-options.md @@ -21,7 +21,7 @@ | **database** | Default database to use when establishing the connection | *string* | | **socketPath** | Permit connecting to the database via Unix domain socket or named pipe, if the server allows it| *string* | | **compress** | Compress exchanges with database using gzip. This can give you better performance when accessing a database in a different location. |*boolean*| false| -| **connectTimeout** | Connection timeout in milliseconds |*integer* | 10 000| +| **connectTimeout** | Connection timeout in milliseconds (default changed from 10000 to 1000 in 2.5.6) |*integer* | 1000| | **socketTimeout** | Socket timeout in milliseconds after the connection is established |*integer* | 0| | **rowsAsArray** | Return result-sets as array, rather than a JSON object. This is a faster way to get results. For more information, see the [Promise](../README.md#querysql-values---promise) and [Callback](callback-api.md#querysql-values-callback---emoitter) query implementations.|*boolean* | false| | **maxAllowedPacket** | permit to indicate server global variable [max_allowed_packet](https://mariadb.com/kb/en/library/server-system-variables/#max_allowed_packet) value to ensure efficient batching. default is 4Mb. see [batch documentation](./batch.md)|*integer* | 4196304| diff --git a/documentation/promise-api.md b/documentation/promise-api.md index 71f38a0a..23357d18 100644 --- a/documentation/promise-api.md +++ b/documentation/promise-api.md @@ -334,7 +334,7 @@ Specific options for pools are : |option|description|type|default| |---:|---|:---:|:---:| -| **`acquireTimeout`** | Timeout to get a new connection from pool in ms. |*integer* | 10000 | +| **`acquireTimeout`** | Timeout to get a new connection from pool in ms. In order to have connection error information, must be higher than connectTimeout |*integer* | 10000 | | **`connectionLimit`** | Maximum number of connection in pool. |*integer* | 10 | | **`idleTimeout`** | Indicate idle time after which a pool connection is released. Value must be lower than [@@wait_timeout](https://mariadb.com/kb/en/library/server-system-variables/#wait_timeout). In seconds (0 means never release) |*integer* | 1800 | | **`minimumIdle`** | Permit to set a minimum number of connection in pool. **Recommendation is to use fixed pool, so not setting this value**.|*integer* | *set to connectionLimit value* | diff --git a/lib/config/connection-options.js b/lib/config/connection-options.js index e8092fc5..a1df42be 100644 --- a/lib/config/connection-options.js +++ b/lib/config/connection-options.js @@ -54,7 +54,7 @@ class ConnectionOptions { // connection options this.initSql = opts.initSql; - this.connectTimeout = opts.connectTimeout === undefined ? 10000 : opts.connectTimeout; + this.connectTimeout = opts.connectTimeout === undefined ? 1000 : opts.connectTimeout; this.connectAttributes = opts.connectAttributes || false; this.compress = opts.compress || false; this.rsaPublicKey = opts.rsaPublicKey; diff --git a/test/integration/test-pool.js b/test/integration/test-pool.js index 8a850afd..9040beea 100644 --- a/test/integration/test-pool.js +++ b/test/integration/test-pool.js @@ -238,7 +238,6 @@ describe('Pool', () => { err.errno === 1524 || err.errno === 1045 || err.errno === 1698 || - err.errno === 45028 || err.errno === 45025 || err.errno === 45044, err.message @@ -252,7 +251,6 @@ describe('Pool', () => { err.errno === 1524 || err.errno === 1045 || err.errno === 1698 || - err.errno === 45028 || err.errno === 45025 || err.errno === 45044, err.message diff --git a/test/unit/config/test-options.js b/test/unit/config/test-options.js index 6f3a389b..d8ede75d 100644 --- a/test/unit/config/test-options.js +++ b/test/unit/config/test-options.js @@ -17,7 +17,7 @@ describe('test options', () => { database: undefined, collation: Collations.fromName('UTF8MB4_UNICODE_CI'), initSql: undefined, - connectTimeout: 10000, + connectTimeout: 1000, connectAttributes: false, compress: false, rsaPublicKey: undefined, From d17378a13c46207c881533435a22dbfb2577bae0 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Thu, 9 Dec 2021 15:42:15 +0100 Subject: [PATCH 4/8] misc - ensure test reliability --- test/integration/test-pool-callback.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/integration/test-pool-callback.js b/test/integration/test-pool-callback.js index 5a389252..07b36e62 100644 --- a/test/integration/test-pool-callback.js +++ b/test/integration/test-pool-callback.js @@ -212,19 +212,26 @@ describe('Pool callback', () => { it('pool query timeout', function (done) { if (process.env.srv === 'skysql' || process.env.srv === 'skysql-ha') this.skip(); this.timeout(5000); + let errorNo = 0; const pool = base.createPoolCallback({ connectionLimit: 1, acquireTimeout: 500 }); const initTime = Date.now(); - pool.query('SELECT SLEEP(?)', 2, () => { + pool.query('SELECT SLEEP(?)', 4, () => { pool.end(); + if (errorNo === 3) { + done(); + } else { + done(new Error(`error expeced 3, but was ${errorNo}`)); + } }); pool.query('SELECT 1', (err, res) => { assert(err.message.includes('retrieve connection from pool timeout')); assert.equal(err.sqlState, 'HY000'); assert.equal(err.errno, 45028); assert.equal(err.code, 'ER_GET_CONNECTION_TIMEOUT'); + errorNo += 1; }); pool.query('SELECT 2', (err) => { assert(err.message.includes('retrieve connection from pool timeout')); @@ -236,6 +243,7 @@ describe('Pool callback', () => { elapse >= 499 && elapse < 550, 'elapse time was ' + elapse + ' but must be just after 500' ); + errorNo += 1; }); setTimeout(() => { pool.query('SELECT 3', (err) => { @@ -248,7 +256,7 @@ describe('Pool callback', () => { elapse >= 698 && elapse < 750, 'elapse time was ' + elapse + ' but must be just after 700' ); - done(); + errorNo += 1; }); }, 200); }); From c8eb320e83fd7cc1d1d3766d00b037d971bc0271 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Thu, 9 Dec 2021 17:31:17 +0100 Subject: [PATCH 5/8] misc - correct character_set_client unexpect error parsing OK_Packet #177 --- lib/cmd/command.js | 2 +- lib/cmd/resultset.js | 2 +- lib/io/packet.js | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/cmd/command.js b/lib/cmd/command.js index 0e9c1c4c..bc248dcc 100644 --- a/lib/cmd/command.js +++ b/lib/cmd/command.js @@ -113,7 +113,7 @@ class Command extends EventEmitter { this.emit('end'); } - static parseOkPacket(packet, out, opts, info) { + parseOkPacket(packet, out, opts, info) { packet.skip(1); //skip header const affectedRows = packet.readUnsignedLength(); diff --git a/lib/cmd/resultset.js b/lib/cmd/resultset.js index 1659f42e..1351b4e0 100644 --- a/lib/cmd/resultset.js +++ b/lib/cmd/resultset.js @@ -138,7 +138,7 @@ class ResultSet extends Command { * @returns {*} null or {Resultset.readResponsePacket} in case of multi-result-set */ readOKPacket(packet, out, opts, info) { - const okPacket = Command.parseOkPacket(packet, out, opts, info); + const okPacket = this.parseOkPacket(packet, out, opts, info); this._rows.push(okPacket); if (info.status & ServerStatus.MORE_RESULTS_EXISTS) { diff --git a/lib/io/packet.js b/lib/io/packet.js index fdb1c62a..5745b9cf 100644 --- a/lib/io/packet.js +++ b/lib/io/packet.js @@ -285,6 +285,12 @@ class Packet { return this.buf.toString('ascii', this.pos - len, this.pos); } + readStringLength() { + throw new Error( + 'code is normally superseded by Node encoder or Iconv depending on charset used' + ); + } + readStringLengthEncoded(encoding) { const len = this.readUnsignedLength(); if (len === null) return null; From 77c01fdb2ec0473826af3d7651d8e23977675347 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Mon, 20 Dec 2021 11:55:13 +0100 Subject: [PATCH 6/8] misc - documentation improvement indicating that connection.release() is async --- documentation/promise-api.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/documentation/promise-api.md b/documentation/promise-api.md index 23357d18..5dd5212d 100644 --- a/documentation/promise-api.md +++ b/documentation/promise-api.md @@ -1158,6 +1158,8 @@ When a connection is given back to the pool, any remaining transactions will be Creates a new [Connection](#connection-api) object with an additional release method. Calling connection.release() will give back connection to pool. +connection.release() is an async method returning an empty promise success + Connection must be given back to pool using this connection.release() method. **Example:** @@ -1165,14 +1167,14 @@ Connection must be given back to pool using this connection.release() method. ```javascript const mariadb = require('mariadb'); const pool = mariadb.createPool({ host: 'mydb.com', user:'myUser' }); -pool.getConnection() - .then(conn => { - console.log("connected ! connection id is " + conn.threadId); - conn.release(); //release to pool - }) - .catch(err => { - console.log("not connected due to error: " + err); - }); + +try { + const conn = await pool.getConnection() + console.log("connected ! connection id is " + conn.threadId); + await conn.release(); //release to pool +} catch (err) { + console.log("not connected due to error: " + err); +}; ``` ## `pool.query(sql[, values])` -> `Promise` From 3d608406c6bd9277e3f556830588e4aa943b24ca Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Mon, 20 Dec 2021 14:13:59 +0100 Subject: [PATCH 7/8] misc - stored procedure tests async --- test/integration/test-call.js | 121 +++++++++++++--------------------- 1 file changed, 46 insertions(+), 75 deletions(-) diff --git a/test/integration/test-call.js b/test/integration/test-call.js index 53d68d62..a9acd10c 100644 --- a/test/integration/test-call.js +++ b/test/integration/test-call.js @@ -5,94 +5,65 @@ const base = require('../base.js'); const { assert } = require('chai'); describe('stored procedure', () => { - before(function (done) { + before(async function () { if (process.env.srv === 'skysql' || process.env.srv === 'skysql-ha') this.skip(); - shareConn - .query('CREATE PROCEDURE stmtSimple (IN p1 INT, IN p2 INT) begin SELECT p1 + p2 t; end') - .then(() => { - done(); - }) - .catch(done); + await shareConn.query( + 'CREATE PROCEDURE stmtSimple (IN p1 INT, IN p2 INT) begin SELECT p1 + p2 t; end' + ); }); - after((done) => { - shareConn.query('DROP PROCEDURE IF EXISTS stmtOutParam').catch((err) => {}); - shareConn.query('DROP PROCEDURE IF EXISTS stmtSimple').catch((err) => {}); - shareConn - .query('DROP FUNCTION IF EXISTS stmtSimpleFunct') - .then(() => { - done(); - }) - .catch((err) => {}); + after(async () => { + await shareConn.query('DROP PROCEDURE IF EXISTS stmtOutParam'); + await shareConn.query('DROP PROCEDURE IF EXISTS stmtSimple'); + await shareConn.query('DROP FUNCTION IF EXISTS stmtSimpleFunct'); }); - it('simple call query', function (done) { - shareConn - .query('call stmtSimple(?,?)', [2, 2]) - .then((rows) => testRes(rows, done)) - .catch(done); + it('simple call query', async () => { + const rows = await shareConn.query('call stmtSimple(?,?)', [2, 2]); + await testRes(rows); }); - it('simple call query using compression', function (done) { - base - .createConnection({ compress: true }) - .then((conn) => { - const finish = (err) => { - conn.end(); - done(err); - }; - conn - .query('call stmtSimple(?,?)', [2, 2]) - .then((rows) => testRes(rows, finish)) - .catch(finish); - }) - .catch(done); + it('simple call query using compression', async () => { + const conn = await base.createConnection({ compress: true }); + try { + const rows = await conn.query('call stmtSimple(?,?)', [2, 2]); + await testRes(rows); + } finally { + conn.end(); + } }); - it('simple function', function (done) { - shareConn.query( + it('simple function', async function () { + await shareConn.query( 'CREATE FUNCTION stmtSimpleFunct ' + '(p1 INT, p2 INT) RETURNS INT NO SQL\nBEGIN\nRETURN p1 + p2;\n end' ); - shareConn - .query('SELECT stmtSimpleFunct(?,?) t', [2, 2]) - .then((rows) => { - assert.equal(rows.length, 1); - assert.equal(rows[0].t, 4); - done(); - }) - .catch(done); + const rows = await shareConn.query('SELECT stmtSimpleFunct(?,?) t', [2, 2]); + assert.equal(rows.length, 1); + assert.equal(rows[0].t, 4); }); - it('call with out parameter query', function (done) { - shareConn.query('CREATE PROCEDURE stmtOutParam (IN p1 INT, INOUT p2 INT) begin SELECT p1; end'); - shareConn - .query('call stmtOutParam(?,?)', [2, 3]) - .then(() => { - done(new Error('must not be possible since output parameter is not a variable')); - }) - .catch((err) => { - assert.ok( - err.message.includes('is not a variable or NEW pseudo-variable in BEFORE trigger') - ); - done(); - }); + it('call with out parameter query', async () => { + await shareConn.query( + 'CREATE PROCEDURE stmtOutParam (IN p1 INT, INOUT p2 INT) begin SELECT p1; end' + ); + try { + await shareConn.query('call stmtOutParam(?,?)', [2, 3]); + throw new Error('must not be possible since output parameter is not a variable'); + } catch (err) { + assert.ok(err.message.includes('is not a variable or NEW pseudo-variable in BEFORE trigger')); + } }); - - function testRes(res, done) { - assert.equal(res.length, 2); - //results - assert.equal(res[0][0].t, 4); - //execution result - assert.equal(res[1].affectedRows, 0); - assert.equal(res[1].insertId, 0); - assert.equal(res[1].warningStatus, 0); - shareConn - .query('SELECT 9 t') - .then((rows) => { - assert.equal(rows[0].t, 9); - done(); - }) - .catch(done); - } }); + +const testRes = async function (res) { + assert.equal(res.length, 2); + //results + assert.equal(res[0][0].t, 4); + //execution result + assert.equal(res[1].affectedRows, 0); + assert.equal(res[1].insertId, 0); + assert.equal(res[1].warningStatus, 0); + const rows = await shareConn.query('SELECT 9 t'); + assert.equal(rows[0].t, 9); +}; From 61598b3fe49da50427e2c3a9bd8b600577988677 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Mon, 24 Jan 2022 11:38:35 +0100 Subject: [PATCH 8/8] misc - update documentation with for-await-of use #189 --- documentation/promise-api.md | 15 ++++++++++++++- test/integration/test-resultset-streaming.js | 9 +++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/documentation/promise-api.md b/documentation/promise-api.md index 5dd5212d..208c9e8c 100644 --- a/documentation/promise-api.md +++ b/documentation/promise-api.md @@ -776,7 +776,20 @@ When using the `query()` method, documented above, the Connector returns the ent Query times and result handlers take the same amount of time, but you may want to consider updating the [`net_read_timeout`](https://mariadb.com/kb/en/library/server-system-variables/#net_read_timeout) server system variable. The query must be totally received before this timeout, which defaults to 30 seconds. -There is 2 differents methods to implement streaming: +There is different methods to implement streaming: + +* for-await-of + +simple use with for-await-of only available since Node.js 10 (note that this must be use within async function) : + +```javascript +async function streamingFunction() { + const queryStream = connection.queryStream('SELECT * FROM mysql.user'); + for await (const row of queryStream) { + console.log(row); + } +} +``` * Events diff --git a/test/integration/test-resultset-streaming.js b/test/integration/test-resultset-streaming.js index 294aa401..3bdbdd63 100644 --- a/test/integration/test-resultset-streaming.js +++ b/test/integration/test-resultset-streaming.js @@ -33,6 +33,15 @@ describe('results-set streaming', () => { .catch(done); }); + it('Streaming result-set for-await-of', async function () { + let currRow = 0; + const stream = shareConn.queryStream('SELECT * FROM testStreamResult'); + for await (const row of stream) { + assert.equal(currRow++, row.v); + } + assert.equal(10000, currRow); + }); + it('Streaming result-set with promise implementation', function (done) { let currRow = 0; let metaReceived = false;