diff --git a/lib/msnodesqlv8/request.js b/lib/msnodesqlv8/request.js index 6ae3f60e..65e9dc31 100644 --- a/lib/msnodesqlv8/request.js +++ b/lib/msnodesqlv8/request.js @@ -392,7 +392,9 @@ class Request extends BaseRequest { continue } const param = this.parameters[name] - if (param.io === 1) { + // an output's initial value is bound with the inputs below, so it needs a marker too; + // an output given no value is null, and 0, '' and false are values like any other + if (param.io === 1 || (param.io === 2 && param.value != null)) { sets.push(`set @${param.name}=?`) } } @@ -432,7 +434,7 @@ class Request extends BaseRequest { continue } const param = this.parameters[name] - if (param.io === 1 || (param.io === 2 && param.value)) { + if (param.io === 1 || (param.io === 2 && param.value != null)) { params.push(castParameter(param.value, param.type)) } } @@ -733,7 +735,7 @@ class Request extends BaseRequest { continue } const param = this.parameters[name] - if (param.io === 2 && param.value) { + if (param.io === 2 && param.value != null) { sets.push(`set @${param.name}=?`) } } diff --git a/lib/tedious/request.js b/lib/tedious/request.js index 93d75f93..b9ab96ca 100644 --- a/lib/tedious/request.js +++ b/lib/tedious/request.js @@ -12,6 +12,7 @@ const { valueHandler } = require('../shared') const JSON_COLUMN_ID = 'JSON_F52E2B61-18A1-11d1-B105-00805F49916B' const XML_COLUMN_ID = 'XML_F52E2B61-18A1-11d1-B105-00805F49916B' +const MAX = 65535 // (1 << 16) - 1 const N_TYPES = { BitN: 0x68, @@ -174,6 +175,18 @@ const valueCorrection = function (value, metadata) { } } +// tedious only compares a length as a number, so `max` is passed as the length it stands +// for. As a word it compares false against the type's maximum both ways, and tedious then +// writes a `max` value without its terminator. +const tediousLength = function (length) { + return typeof length === 'string' && length.trim().toLowerCase() === 'max' ? MAX : length +} + +// tedious writes a TVP column with the length it is given, without resolving a missing one, +// and a missing length fails the same comparisons as `max`. These types are given `max`, +// which is how tedious wrote them before it compared the length both ways. +const MAX_LENGTH_TYPES = new Set([tds.TYPES.NVarChar, tds.TYPES.VarChar, tds.TYPES.VarBinary]) + const parameterCorrection = function (value) { if (value instanceof Table) { const tvp = { @@ -191,7 +204,7 @@ const parameterCorrection = function (value) { tvp.columns.push({ name: col.name, type: tediousType, - length: col.length, + length: col.length == null && MAX_LENGTH_TYPES.has(tediousType) ? MAX : tediousLength(col.length), scale: col.scale, precision: col.precision }) @@ -368,7 +381,7 @@ class Request extends BaseRequest { // tedious emits this name as a quoted identifier of its own when it builds // the `insert bulk` statement, so it is checked here as well as in // Table#declare, in case the name changed since it was checked above. - bulk.addColumn(assertSafeColumnName(col.name), getTediousType(col.type), { nullable: col.nullable, length: assertSafeTypeSize(col.length), scale: assertSafeTypeSize(col.scale), precision: assertSafeTypeSize(col.precision) }) + bulk.addColumn(assertSafeColumnName(col.name), getTediousType(col.type), { nullable: col.nullable, length: tediousLength(assertSafeTypeSize(col.length)), scale: assertSafeTypeSize(col.scale), precision: assertSafeTypeSize(col.precision) }) } if (table.create) { @@ -773,9 +786,9 @@ class Request extends BaseRequest { // tedious builds the sp_executesql declaration from these, so a rejected size // has to be reported through the path that releases the connection if (param.io === 1) { - req.addParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: assertSafeTypeSize(param.length), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) + req.addParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: tediousLength(assertSafeTypeSize(param.length)), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) } else { - req.addOutputParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: assertSafeTypeSize(param.length), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) + req.addOutputParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: tediousLength(assertSafeTypeSize(param.length)), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) } } } @@ -1093,9 +1106,9 @@ class Request extends BaseRequest { } const param = this.parameters[name] if (param.io === 1) { - req.addParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: assertSafeTypeSize(param.length), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) + req.addParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: tediousLength(assertSafeTypeSize(param.length)), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) } else { - req.addOutputParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: assertSafeTypeSize(param.length), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) + req.addOutputParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: tediousLength(assertSafeTypeSize(param.length)), scale: assertSafeTypeSize(param.scale), precision: assertSafeTypeSize(param.precision) }) } } diff --git a/test/cleanup.sql b/test/cleanup.sql index ed8085c7..e61dba6e 100644 --- a/test/cleanup.sql +++ b/test/cleanup.sql @@ -13,6 +13,9 @@ if exists (select * from sys.procedures where name = '__test5') if exists (select * from sys.procedures where name = '__test7') exec('drop procedure [dbo].[__test7]') +if exists (select * from sys.procedures where name = '__testMaxType') + exec('drop procedure [dbo].[__testMaxType]') + if exists (select * from sys.procedures where name = '__testDuplicateNames') exec('drop procedure [dbo].[__testDuplicateNames]') @@ -28,6 +31,9 @@ if exists (select * from sys.procedures where name = '__testDelay') if exists (select * from sys.types where is_user_defined = 1 and name = 'MSSQLTestType') exec('drop type [dbo].[MSSQLTestType]') +if exists (select * from sys.types where is_user_defined = 1 and name = 'MSSQLTestMaxType') + exec('drop type [dbo].[MSSQLTestMaxType]') + if exists (select * from sys.tables where name = 'prepstm_test') exec('drop table [dbo].[tvp_test]') diff --git a/test/common/tests.js b/test/common/tests.js index 85590fa9..a48650b2 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -1268,6 +1268,41 @@ module.exports = (sql, driver) => { }).catch(done) }, + 'parameters with length as max' (done) { + const req = new TestRequest() + req.input('nvarchar', sql.NVarChar('max'), 'JP1016') + req.input('varchar', sql.VarChar('max'), 'JP1016') + req.input('varbinary', sql.VarBinary('max'), Buffer.from('JP1016')) + req.output('out', sql.NVarChar('max'), 'JP1016') + req.query('select @nvarchar as nvarchar, @varchar as varchar, @varbinary as varbinary; set @out = @out + @out').then(result => { + assert.strictEqual(result.recordset[0].nvarchar, 'JP1016', 'nvarchar(max) input should round-trip') + assert.strictEqual(result.recordset[0].varchar, 'JP1016', 'varchar(max) input should round-trip') + assert.deepStrictEqual(result.recordset[0].varbinary, Buffer.from('JP1016'), 'varbinary(max) input should round-trip') + assert.strictEqual(result.output.out, 'JP1016JP1016', 'nvarchar(max) output should round-trip') + done() + }).catch(done) + }, + + 'output parameters with a falsy initial value' (done) { + const req = new TestRequest() + req.output('i', sql.Int, 0) + req.output('s', sql.NVarChar(10), '') + req.output('b', sql.Bit, false) + req.query("set @i = @i + 1; set @s = @s + N'x'; set @b = ~@b").then(result => { + assert.strictEqual(result.output.i, 1, 'an int output starting at 0 should be incremented') + assert.strictEqual(result.output.s, 'x', 'an nvarchar output starting empty should be appended to') + assert.strictEqual(result.output.b, true, 'a bit output starting false should be flipped') + + const proc = new TestRequest() + proc.input('in', sql.Int, 1) + proc.output('out', sql.Int, 0) + return proc.execute('__testInputOutputValue') + }).then(result => { + assert.strictEqual(result.output.out, 1, 'a procedure output starting at 0 should be added to') + done() + }).catch(done) + }, + 'prepared statement' (done) { const ps = new TestPreparedStatement() ps.input('num', sql.Int) @@ -2483,6 +2518,24 @@ module.exports = (sql, driver) => { }).catch(done) }, + 'new Table with string columns without a length' (done) { + const tvp = new sql.Table('dbo.MSSQLTestMaxType') + tvp.columns.add('n', sql.NVarChar) + tvp.columns.add('v', sql.VarChar) + tvp.columns.add('vb', sql.VarBinary) + tvp.rows.add('asdf', 'asdf', Buffer.from('asdf')) + + const req = new TestRequest() + req.input('tvp', tvp) + req.execute('__testMaxType').then(result => { + assert.strictEqual(result.recordsets[0].length, 1, 'the table should come back with its one row') + assert.strictEqual(result.recordsets[0][0].n, 'asdf', 'an nvarchar column should round-trip') + assert.strictEqual(result.recordsets[0][0].v, 'asdf', 'a varchar column should round-trip') + assert.deepStrictEqual(result.recordsets[0][0].vb, Buffer.from('asdf'), 'a varbinary column should round-trip') + done() + }).catch(done) + }, + 'Recordset.toTable()' (done) { const req = new TestRequest() req.query('select \'asdf\' as a, 15 as b').then(result => { diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index 4c8a5c58..1f8fc70f 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -81,6 +81,8 @@ describe('msnodesqlv8', function () { it('query with output parameters (batch)', done => TESTS['query with output parameters']('batch', done)) it('query with duplicate parameters throws', done => TESTS['query with duplicate parameters throws'](done)) it('query parameters can be replaced', done => TESTS['query parameters can be replaced'](done)) + it('parameters with length as max', done => TESTS['parameters with length as max'](done)) + it('output parameters with a falsy initial value', done => TESTS['output parameters with a falsy initial value'](done)) it('query with error', done => TESTS['query with error'](done)) it('query with multiple errors (not supported by msnodesqlv8)', done => TESTS['query with multiple errors'](done)) it.skip('query with raiseerror (not supported by msnodesqlv8)', done => TESTS['query with raiseerror'](done)) @@ -165,6 +167,7 @@ describe('msnodesqlv8', function () { it('bulk load rejects unsafe column names without leaking a connection', done => TESTS['bulk load rejects unsafe column names without leaking a connection']('bulk_table_unsafe', done)) it('bulk load releases the connection when a column name changes after the check', done => TESTS['bulk load releases the connection when a column name changes after the check']('bulk_table_mutated', done)) it('bulk load into an existing table rejects an unsafe column name', done => TESTS['bulk load into an existing table rejects an unsafe column name']('bulk_table', done)) + it('bulk insert with length as max (table)', done => TESTS['bulk insert with length as max']('bulk_table5', done)) after(done => sql.close(done)) }) @@ -264,6 +267,7 @@ describe('msnodesqlv8', function () { }) it('new Table', done => TESTS['new Table'](done)) + it('new Table with string columns without a length', done => TESTS['new Table with string columns without a length'](done)) it('Recordset.toTable()', done => TESTS['Recordset.toTable()'](done)) it('Recordset.toTable() from existing', done => TESTS['Recordset.toTable() from existing'](done)) diff --git a/test/prepare.sql b/test/prepare.sql index ca49adff..8e407dfb 100644 --- a/test/prepare.sql +++ b/test/prepare.sql @@ -112,6 +112,21 @@ exec('create type [dbo].[MSSQLTestType] as table( [b] [integer] null )') +exec('create type [dbo].[MSSQLTestMaxType] as table( + [n] [nvarchar](50) null, + [v] [varchar](50) null, + [vb] [varbinary](50) null +)') + +exec('create procedure [dbo].[__testMaxType] + @tvp MSSQLTestMaxType readonly +as +begin + + select * from @tvp + +end') + exec('create procedure [dbo].[__test7] @tvp MSSQLTestType readonly as diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index 6b24cad4..1c278e25 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -90,6 +90,8 @@ describe('tedious', () => { it('query with output parameters (batch)', done => TESTS['query with output parameters']('batch', done)) it('query with duplicate parameters throws', done => TESTS['query with duplicate parameters throws'](done)) it('query parameters can be replaced', done => TESTS['query parameters can be replaced'](done)) + it('parameters with length as max', done => TESTS['parameters with length as max'](done)) + it('output parameters with a falsy initial value', done => TESTS['output parameters with a falsy initial value'](done)) it('query with error', done => TESTS['query with error'](done)) it('query with multiple errors', done => TESTS['query with multiple errors'](done)) it('query with raiseerror', done => TESTS['query with raiseerror'](done)) @@ -318,6 +320,7 @@ describe('tedious', () => { }) it('new Table', done => TESTS['new Table'](done)) + it('new Table with string columns without a length', done => TESTS['new Table with string columns without a length'](done)) it('Recordset.toTable()', done => TESTS['Recordset.toTable()'](done)) it('Recordset.toTable() from existing', done => TESTS['Recordset.toTable() from existing'](done))