fix: pass max lengths to tedious as a number

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}=?`)
         }
       }

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) })
             }
           }
 

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]')
 

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 => {

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))
 

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

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))