Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for instance name #1491

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/connection.ts
Expand Up @@ -854,6 +854,7 @@ const CLEANUP_TYPE = {
interface RoutingData {
server: string;
port: number;
instanceName: string | undefined;
}

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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.');
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
MichaelSun90 marked this conversation as resolved.
Show resolved Hide resolved
this.config.options.port = DEFAULT_PORT;
}

if (config.options.readOnlyIntent !== undefined) {
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(' ');
});
Expand Down
5 changes: 5 additions & 0 deletions src/instance-lookup.ts
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
27 changes: 17 additions & 10 deletions src/prelogin-payload.ts
Expand Up @@ -48,6 +48,7 @@ interface Options {
build: number;
subbuild: number;
};
instanceName: string | undefined;
}

/*
Expand All @@ -67,18 +68,18 @@ class PreloginPayload {
encryption!: number;
encryptionString!: string;

instance!: number;
instanceName!: string;

threadId!: number;

mars!: number;
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();
Expand All @@ -90,7 +91,7 @@ class PreloginPayload {
const options = [
this.createVersionOption(),
this.createEncryptionOption(),
this.createInstanceOption(),
this.createInstanceNameOption(),
this.createThreadIdOption(),
this.createMarsOption(),
this.createFedAuthOption()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 : ''
Expand Down
3 changes: 2 additions & 1 deletion src/sender.ts
Expand Up @@ -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);
}
8 changes: 4 additions & 4 deletions src/token/handler.ts
Expand Up @@ -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;

Expand Down Expand Up @@ -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
};
}

Expand Down
25 changes: 25 additions & 0 deletions test/integration/connection-test.js
Expand Up @@ -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).
Expand Down
15 changes: 10 additions & 5 deletions 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);
Expand All @@ -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');
});
});
6 changes: 5 additions & 1 deletion test/unit/rerouting-test.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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'
}
});

Expand Down
28 changes: 28 additions & 0 deletions 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', () => {
Expand Down Expand Up @@ -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);
});
});