From 0bd184fba3ef345baf2fb1f4125ce3f449a4e627 Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Sun, 4 Oct 2026 22:05:30 +0100 Subject: [PATCH 1/2] fix: pass max lengths to tedious as a number A length of 'max' (e.g. `sql.NVarChar('max')` or a Table column with `length: 'max'`) was handed to tedious as the string. tedious only compares a length numerically, and since 20.3.0 it decides whether to write the PLP header and the PLP terminator with opposite comparisons, so a word fails both and the value is sent without its terminator. The server rejects the request with "The incoming tabular data stream (TDS) protocol stream is incorrect". A TVP column without a length (e.g. `sql.NVarChar` with no length) fails the same way, because tedious writes TVP columns with the length they are given rather than resolving a missing one. Map 'max' to 65535 at the tedious boundary for parameters, bulk load columns and TVP columns, and send a missing TVP length as 65535 for NVarChar, VarChar and VarBinary, as tedious wrote it before 20.3.0; the SQL declarations still emit `max`. Co-Authored-By: Claude Opus 5.5 --- lib/tedious/request.js | 25 +++++++++++++++++++------ test/cleanup.sql | 6 ++++++ test/common/tests.js | 33 +++++++++++++++++++++++++++++++++ test/msnodesqlv8/msnodesqlv8.js | 2 ++ test/prepare.sql | 15 +++++++++++++++ test/tedious/tedious.js | 2 ++ 6 files changed, 77 insertions(+), 6 deletions(-) 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..227610cf 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -1268,6 +1268,21 @@ 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) + }, + 'prepared statement' (done) { const ps = new TestPreparedStatement() ps.input('num', sql.Int) @@ -2483,6 +2498,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..ee17f21e 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -165,6 +165,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 +265,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..ae3e3ad1 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -90,6 +90,7 @@ 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('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 +319,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)) From a05796ae3dd41a368f9da45bb2fede58719d7e78 Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Sun, 4 Oct 2026 23:35:45 +0100 Subject: [PATCH 2/2] fix: set output parameter initial values in msnodesqlv8 requests query() and batch() on msnodesqlv8 bound an output parameter's initial value but only wrote a `set @name=?` marker for inputs, so the value was never assigned and the output started as NULL. Values are bound by position, so an output with an initial value declared before an input also shifted the input onto the output's value. An initial value of 0, '' or false was skipped altogether, by query() and by execute(), so those outputs started as NULL too. Write the marker for every value that is bound, and treat only a null initial value as no value. Co-Authored-By: Claude Opus 5.5 --- lib/msnodesqlv8/request.js | 8 +++++--- test/common/tests.js | 20 ++++++++++++++++++++ test/msnodesqlv8/msnodesqlv8.js | 2 ++ test/tedious/tedious.js | 1 + 4 files changed, 28 insertions(+), 3 deletions(-) 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/test/common/tests.js b/test/common/tests.js index 227610cf..a48650b2 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -1283,6 +1283,26 @@ module.exports = (sql, driver) => { }).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) diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index ee17f21e..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)) diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index ae3e3ad1..1c278e25 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -91,6 +91,7 @@ describe('tedious', () => { 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))