diff --git a/src/connection.ts b/src/connection.ts index 20d35fd6c..6fb4a3e05 100644 --- a/src/connection.ts +++ b/src/connection.ts @@ -854,6 +854,7 @@ const CLEANUP_TYPE = { interface RoutingData { server: string; port: number; + instanceName: string | undefined; } /** @@ -1253,7 +1254,7 @@ class Connection extends EventEmitter { maxRetriesOnTransientErrors: 3, multiSubnetFailover: false, packetSize: DEFAULT_PACKET_SIZE, - port: DEFAULT_PORT, + port: undefined, readOnlyIntent: false, requestTimeout: DEFAULT_CLIENT_REQUEST_TIMEOUT, rowCollectionOnDone: false, @@ -1272,10 +1273,6 @@ class Connection extends EventEmitter { }; if (config.options) { - if (config.options.port && config.options.instanceName) { - throw new Error('Port and instanceName are mutually exclusive, but ' + config.options.port + ' and ' + config.options.instanceName + ' provided'); - } - if (config.options.abortTransactionOnError !== undefined) { if (typeof config.options.abortTransactionOnError !== 'boolean' && config.options.abortTransactionOnError !== null) { throw new TypeError('The "config.options.abortTransactionOnError" property must be of type string or null.'); @@ -1502,7 +1499,6 @@ class Connection extends EventEmitter { } this.config.options.instanceName = config.options.instanceName; - this.config.options.port = undefined; } if (config.options.isolationLevel !== undefined) { @@ -1553,7 +1549,8 @@ class Connection extends EventEmitter { } this.config.options.port = config.options.port; - this.config.options.instanceName = undefined; + } else if (!this.config.options.instanceName) { + this.config.options.port = DEFAULT_PORT; } if (config.options.readOnlyIntent !== undefined) { @@ -1883,15 +1880,23 @@ class Connection extends EventEmitter { initialiseConnection() { const signal = this.createConnectTimer(); + // If user provided both port and an instance name, + // the code should always use the port and skip the instance lookup if (this.config.options.port) { return this.connectOnPort(this.config.options.port, this.config.options.multiSubnetFailover, signal); } else { + // The instance lookup communicates with the server and gets all the + // available instance name and their corresponding ports. + // Then based on the provided instance name and the result ports, + // logic will try to find a match and connection to the instance by its port. return instanceLookup({ server: this.config.server, instanceName: this.config.options.instanceName!, timeout: this.config.options.connectTimeout, signal: signal }).then((port) => { + // If we get a port from the instance lookup process, the logic will try to connect + // using this port. process.nextTick(() => { this.connectOnPort(port, this.config.options.multiSubnetFailover, signal); }); @@ -2235,7 +2240,8 @@ class Connection extends EventEmitter { const payload = new PreloginPayload({ encrypt: this.config.options.encrypt, - version: { major: Number(major), minor: Number(minor), build: Number(build), subbuild: 0 } + version: { major: Number(major), minor: Number(minor), build: Number(build), subbuild: 0 }, + instanceName: this.routingData?.instanceName ?? this.config.options.instanceName }); this.messageIo.sendMessage(TYPE.PRELOGIN, payload.data); @@ -3148,6 +3154,11 @@ Connection.prototype.STATE = { } const preloginPayload = new PreloginPayload(messageBuffer); + if (preloginPayload.instanceName !== undefined && preloginPayload.instanceName !== '\x00') { + this.emit('connect', new ConnectionError('Server instanceName does not match')); + return this.close(); + } + this.debug.payload(function() { return preloginPayload.toString(' '); }); diff --git a/src/instance-lookup.ts b/src/instance-lookup.ts index 1d6645644..e1076343a 100644 --- a/src/instance-lookup.ts +++ b/src/instance-lookup.ts @@ -57,6 +57,8 @@ export async function instanceLookup(options: { server: string, instanceName: st try { response = await withTimeout(timeout, async (signal) => { const request = Buffer.from([0x02]); + // This will send message to request a response that containing + // all instances available on the host return await sendMessage(options.server, port, lookup, signal, request); }, signal); } catch (err) { @@ -74,6 +76,9 @@ export async function instanceLookup(options: { server: string, instanceName: st } const message = response.toString('ascii', MYSTERY_HEADER_LENGTH); + // This function will try to parse out all the port information from the response + // Then compare the instance name while parsing the response, and if there is an + // instance name match, then return that port. const foundPort = parseBrowserResponse(message, instanceName); if (!foundPort) { diff --git a/src/prelogin-payload.ts b/src/prelogin-payload.ts index 77f1e6581..87ce772fa 100644 --- a/src/prelogin-payload.ts +++ b/src/prelogin-payload.ts @@ -48,6 +48,7 @@ interface Options { build: number; subbuild: number; }; + instanceName: string | undefined; } /* @@ -67,7 +68,7 @@ class PreloginPayload { encryption!: number; encryptionString!: string; - instance!: number; + instanceName!: string; threadId!: number; @@ -75,10 +76,10 @@ class PreloginPayload { marsString!: string; fedAuthRequired!: number; - constructor(bufferOrOptions: Buffer | Options = { encrypt: false, version: { major: 0, minor: 0, build: 0, subbuild: 0 } }) { + constructor(bufferOrOptions: Buffer | Options = { encrypt: false, version: { major: 0, minor: 0, build: 0, subbuild: 0 }, instanceName: undefined }) { if (bufferOrOptions instanceof Buffer) { this.data = bufferOrOptions; - this.options = { encrypt: false, version: { major: 0, minor: 0, build: 0, subbuild: 0 } }; + this.options = { encrypt: false, version: { major: 0, minor: 0, build: 0, subbuild: 0 }, instanceName: undefined }; } else { this.options = bufferOrOptions; this.createOptions(); @@ -90,7 +91,7 @@ class PreloginPayload { const options = [ this.createVersionOption(), this.createEncryptionOption(), - this.createInstanceOption(), + this.createInstanceNameOption(), this.createThreadIdOption(), this.createMarsOption(), this.createFedAuthOption() @@ -144,8 +145,11 @@ class PreloginPayload { }; } - createInstanceOption() { + createInstanceNameOption() { const buffer = new WritableTrackingBuffer(optionBufferSize); + if (this.options.instanceName !== undefined) { + buffer.writeString(this.options.instanceName, 'utf8'); + } buffer.writeUInt8(0x00); return { token: TOKEN.INSTOPT, @@ -193,7 +197,7 @@ class PreloginPayload { this.extractEncryption(dataOffset); break; case TOKEN.INSTOPT: - this.extractInstance(dataOffset); + this.extractInstanceName(dataOffset, dataLength); break; case TOKEN.THREADID: if (dataLength > 0) { @@ -226,8 +230,11 @@ class PreloginPayload { this.encryptionString = encryptByValue[this.encryption]; } - extractInstance(offset: number) { - this.instance = this.data.readUInt8(offset); + extractInstanceName(offset: number, dataLength: number) { + if (dataLength === 0) { + return; + } + this.instanceName = this.data.toString('utf8', offset, offset + dataLength); } extractThreadId(offset: number) { @@ -245,11 +252,11 @@ class PreloginPayload { toString(indent = '') { return indent + 'PreLogin - ' + sprintf( - 'version:%d.%d.%d.%d, encryption:0x%02X(%s), instopt:0x%02X, threadId:0x%08X, mars:0x%02X(%s)', + 'version:%d.%d.%d.%d, encryption:0x%02X(%s), instanceName:%s, threadId:0x%08X, mars:0x%02X(%s)', this.version.major, this.version.minor, this.version.build, this.version.subbuild, this.encryption ? this.encryption : 0, this.encryptionString ? this.encryptionString : '', - this.instance ? this.instance : 0, + this.instanceName ? this.instanceName : '', this.threadId ? this.threadId : 0, this.mars ? this.mars : 0, this.marsString ? this.marsString : '' diff --git a/src/sender.ts b/src/sender.ts index a33e2d741..58ac3f190 100644 --- a/src/sender.ts +++ b/src/sender.ts @@ -90,6 +90,7 @@ export async function sendMessage(host: string, port: number, lookup: LookupFunc }); }); } - + // This sends one or multiple IP addresses in parallel (for clustered SQL server setups) + // and it returns the first response it gets back from the SQL server browser agent return await sendInParallel(addresses, port, request, signal); } diff --git a/src/token/handler.ts b/src/token/handler.ts index 8d0088c9d..ca1e2d2bc 100644 --- a/src/token/handler.ts +++ b/src/token/handler.ts @@ -248,7 +248,7 @@ export class Login7TokenHandler extends TokenHandler { connection: Connection; fedAuthInfoToken: FedAuthInfoToken | undefined; - routingData: { server: string, port: number } | undefined; + routingData: { server: string, port: number, instanceName: string | undefined } | undefined; loginAckReceived = false; @@ -338,11 +338,11 @@ export class Login7TokenHandler extends TokenHandler { } onRoutingChange(token: RoutingEnvChangeToken) { - // Removes instance name attached to the redirect url. E.g., redirect.db.net\instance1 --> redirect.db.net - const [ server ] = token.newValue.server.split('\\'); + // Split the target into servername and instance name, e.g. redirect.db.net\instance1 --> redirect.db.net and instance1 + const [ server, instanceName ] = token.newValue.server.split('\\'); this.routingData = { - server, port: token.newValue.port + server, port: token.newValue.port, instanceName }; } diff --git a/test/integration/connection-test.js b/test/integration/connection-test.js index 2022c4588..4f914dfe7 100644 --- a/test/integration/connection-test.js +++ b/test/integration/connection-test.js @@ -147,6 +147,31 @@ describe('Initiate Connect Test', function() { }); }); + it('should fail when connecting by port with wrong instance name', function(done) { + const config = getConfig(); + + config.options.instanceName = 'NonExistInstanceName'; + + if ((config.options != null ? config.options.port : undefined) == null) { + // Config says don't do this test (probably because ports are dynamic). + return this.skip(); + } + + const connection = new Connection(config); + + connection.connect((err) => { + assert.instanceOf(err, ConnectionError); + assert.strictEqual(err?.message, 'Server instanceName does not match'); + + done(); + }); + + connection.on('infoMessage', function(info) { + // console.log("#{info.number} : #{info.message}") + }); + }); + + it('should connect by instance name', function(done) { if (!getInstanceName()) { // Config says don't do this test (probably because SQL Server Browser is not available). diff --git a/test/unit/prelogin-payload-test.js b/test/unit/prelogin-payload-test.js index 56b8e5c78..482626f83 100644 --- a/test/unit/prelogin-payload-test.js +++ b/test/unit/prelogin-payload-test.js @@ -1,14 +1,14 @@ const PreloginPayload = require('../../src/prelogin-payload'); const assert = require('chai').assert; -function assertPayload(payload, encryptionString, { major, minor, build, subbuild }) { +function assertPayload(payload, encryptionString, { major, minor, build, subbuild }, instanceName) { assert.strictEqual(payload.version.major, major); assert.strictEqual(payload.version.minor, minor); assert.strictEqual(payload.version.build, build); assert.strictEqual(payload.version.subbuild, subbuild); assert.strictEqual(payload.encryptionString, encryptionString); - assert.strictEqual(payload.instance, 0); + assert.strictEqual(payload.instanceName, instanceName); assert.strictEqual(payload.threadId, 0); assert.strictEqual(payload.marsString, 'OFF'); assert.strictEqual(payload.fedAuthRequired, 1); @@ -17,17 +17,22 @@ function assertPayload(payload, encryptionString, { major, minor, build, subbuil describe('prelogin-payload-assert', function() { it('should not encrypt', function() { const payload = new PreloginPayload(); - assertPayload(payload, 'NOT_SUP', { major: 0, minor: 0, build: 0, subbuild: 0 }); + assertPayload(payload, 'NOT_SUP', { major: 0, minor: 0, build: 0, subbuild: 0 }, '\u0000'); }); it('should encrypt', function() { const payload = new PreloginPayload({ encrypt: true, version: { major: 11, minor: 3, build: 2, subbuild: 0 } }); - assertPayload(payload, 'ON', { major: 11, minor: 3, build: 2, subbuild: 0 }); + assertPayload(payload, 'ON', { major: 11, minor: 3, build: 2, subbuild: 0 }, '\u0000'); + }); + + it('should accept an instance name', function() { + const payload = new PreloginPayload({ encrypt: true, version: { major: 11, minor: 3, build: 2, subbuild: 0 }, instanceName: 'MSSQLServer' }); + assert.strictEqual(payload.instanceName, 'MSSQLServer\u0000'); }); it('should create from buffer', function() { const payload = new PreloginPayload(); new PreloginPayload(payload.data); - assertPayload(payload, 'NOT_SUP', { major: 0, minor: 0, build: 0, subbuild: 0 }); + assertPayload(payload, 'NOT_SUP', { major: 0, minor: 0, build: 0, subbuild: 0 }, '\u0000'); }); }); diff --git a/test/unit/rerouting-test.js b/test/unit/rerouting-test.js index a793aa4be..cb4865822 100644 --- a/test/unit/rerouting-test.js +++ b/test/unit/rerouting-test.js @@ -256,6 +256,9 @@ describe('Connecting to a server that sends a re-routing information', function( chunks.push(data); } + const preloginPayload = new PreloginPayload(Buffer.concat(chunks)); + assert.strictEqual(preloginPayload.instanceName, 'instanceNameA\u0000'); + const responsePayload = new PreloginPayload({ encrypt: false, version: { major: 0, minor: 0, build: 0, subbuild: 0 } }); const responseMessage = new Message({ type: 0x12 }); responseMessage.end(responsePayload.data); @@ -367,7 +370,8 @@ describe('Connecting to a server that sends a re-routing information', function( server: routingServer.address().address, options: { port: routingServer.address().port, - encrypt: false + encrypt: false, + instanceName: 'instanceNameA' } }); diff --git a/test/unit/token/env-change-token-parser-test.js b/test/unit/token/env-change-token-parser-test.js index 43919e727..e96a20b0c 100644 --- a/test/unit/token/env-change-token-parser-test.js +++ b/test/unit/token/env-change-token-parser-test.js @@ -1,5 +1,8 @@ const StreamParser = require('../../../src/token/stream-parser'); const WritableTrackingBuffer = require('../../../src/tracking-buffer/writable-tracking-buffer'); +const { Connection } = require('../../../src/tedious'); +const Message = require('../../../src/message'); +const Login7TokenHandler = require('../../../src/token/handler').Login7TokenHandler; const assert = require('chai').assert; describe('Env Change Token Parser', () => { @@ -66,4 +69,29 @@ describe('Env Change Token Parser', () => { const result = await parser.next(); assert.isTrue(result.done); }); + + it('Test if routing data capture the correct instance name value', async function() { + const valueBuffer = new WritableTrackingBuffer(0); + valueBuffer.writeUInt8(0); // Protocol + valueBuffer.writeUInt16LE(1433); // Port + valueBuffer.writeUsVarchar('127.0.0.1\\instanceNameA', 'ucs-2'); + + const envValueDataBuffer = new WritableTrackingBuffer(0); + envValueDataBuffer.writeUInt8(20); // Type + envValueDataBuffer.writeUsVarbyte(valueBuffer.data); + envValueDataBuffer.writeUsVarbyte(Buffer.alloc(0)); + + const envChangeBuffer = new WritableTrackingBuffer(0); + envChangeBuffer.writeUInt8(0xE3); // TokenType + envChangeBuffer.writeUsVarbyte(envValueDataBuffer.data); // Length + EnvValueData + + const responseMessage = new Message({ type: 0x04 }); + responseMessage.end(envChangeBuffer.data); + const parser = StreamParser.parseTokens(responseMessage, {}, {}); + const result = await parser.next(); + const handler = new Login7TokenHandler(new Connection({ server: 'servername' })); + handler[result.value.handlerName](result.value); + assert.strictEqual(handler.routingData.instanceName, 'instanceNameA'); + assert.isTrue((await parser.next()).done); + }); });