From a2d217c4449d72c8b7560d26fd9668e892ee0896 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 25 Sep 2019 12:38:11 -0700 Subject: [PATCH] [TS] Add support for negotiateVersion and connectionToken (#14157) --- .../clients/ts/signalr/src/HttpConnection.ts | 41 ++- .../clients/ts/signalr/tests/Common.ts | 8 +- .../ts/signalr/tests/HttpConnection.test.ts | 271 +++++++++++++++++- .../tests/HubConnectionBuilder.test.ts | 18 +- 4 files changed, 304 insertions(+), 34 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index 5bd423124b..5b1a3744cd 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -23,6 +23,8 @@ const enum ConnectionState { /** @private */ export interface INegotiateResponse { connectionId?: string; + connectionToken?: string; + negotiateVersion?: number; availableTransports?: IAvailableTransport[]; url?: string; accessToken?: string; @@ -70,6 +72,8 @@ export class HttpConnection implements IConnection { public onreceive: ((data: string | ArrayBuffer) => void) | null; public onclose: ((e?: Error) => void) | null; + private readonly negotiateVersion: number = 1; + constructor(url: string, options: IHttpConnectionOptions = {}) { Arg.isRequired(url, "url"); @@ -272,8 +276,6 @@ export class HttpConnection implements IConnection { throw new Error("Negotiate redirection limit exceeded."); } - this.connectionId = negotiateResponse.connectionId; - await this.createTransport(url, this.options.transport, negotiateResponse, transferFormat); } @@ -322,32 +324,41 @@ export class HttpConnection implements IConnection { return Promise.reject(new Error(`Unexpected status code returned from negotiate ${response.statusCode}`)); } - return JSON.parse(response.content as string) as INegotiateResponse; + const negotiateResponse = JSON.parse(response.content as string) as INegotiateResponse; + if (!negotiateResponse.negotiateVersion || negotiateResponse.negotiateVersion < 1) { + // Negotiate version 0 doesn't use connectionToken + // So we set it equal to connectionId so all our logic can use connectionToken without being aware of the negotiate version + negotiateResponse.connectionToken = negotiateResponse.connectionId; + } + return negotiateResponse; } catch (e) { this.logger.log(LogLevel.Error, "Failed to complete negotiation with the server: " + e); return Promise.reject(e); } } - private createConnectUrl(url: string, connectionId: string | null | undefined) { - if (!connectionId) { + private createConnectUrl(url: string, connectionToken: string | null | undefined) { + if (!connectionToken) { return url; } - return url + (url.indexOf("?") === -1 ? "?" : "&") + `id=${connectionId}`; + + return url + (url.indexOf("?") === -1 ? "?" : "&") + `id=${connectionToken}`; } private async createTransport(url: string, requestedTransport: HttpTransportType | ITransport | undefined, negotiateResponse: INegotiateResponse, requestedTransferFormat: TransferFormat): Promise { - let connectUrl = this.createConnectUrl(url, negotiateResponse.connectionId); + let connectUrl = this.createConnectUrl(url, negotiateResponse.connectionToken); if (this.isITransport(requestedTransport)) { this.logger.log(LogLevel.Debug, "Connection was provided an instance of ITransport, using that directly."); this.transport = requestedTransport; await this.startTransport(connectUrl, requestedTransferFormat); + this.connectionId = negotiateResponse.connectionId; return; } const transportExceptions: any[] = []; const transports = negotiateResponse.availableTransports || []; + let negotiate: INegotiateResponse | undefined = negotiateResponse; for (const endpoint of transports) { const transportOrError = this.resolveTransportOrError(endpoint, requestedTransport, requestedTransferFormat); if (transportOrError instanceof Error) { @@ -355,20 +366,21 @@ export class HttpConnection implements IConnection { transportExceptions.push(`${endpoint.transport} failed: ${transportOrError}`); } else if (this.isITransport(transportOrError)) { this.transport = transportOrError; - if (!negotiateResponse.connectionId) { + if (!negotiate) { try { - negotiateResponse = await this.getNegotiationResponse(url); + negotiate = await this.getNegotiationResponse(url); } catch (ex) { return Promise.reject(ex); } - connectUrl = this.createConnectUrl(url, negotiateResponse.connectionId); + connectUrl = this.createConnectUrl(url, negotiate.connectionToken); } try { await this.startTransport(connectUrl, requestedTransferFormat); + this.connectionId = negotiate.connectionId; return; } catch (ex) { this.logger.log(LogLevel.Error, `Failed to start the transport '${endpoint.transport}': ${ex}`); - negotiateResponse.connectionId = undefined; + negotiate = undefined; transportExceptions.push(`${endpoint.transport} failed: ${ex}`); if (this.connectionState !== ConnectionState.Connecting) { @@ -504,7 +516,7 @@ export class HttpConnection implements IConnection { // Setting the url to the href propery of an anchor tag handles normalization // for us. There are 3 main cases. - // 1. Relative path normalization e.g "b" -> "http://localhost:5000/a/b" + // 1. Relative path normalization e.g "b" -> "http://localhost:5000/a/b" // 2. Absolute path normalization e.g "/a/b" -> "http://localhost:5000/a/b" // 3. Networkpath reference normalization e.g "//localhost:5000/a/b" -> "http://localhost:5000/a/b" const aTag = window.document.createElement("a"); @@ -522,6 +534,11 @@ export class HttpConnection implements IConnection { } negotiateUrl += "negotiate"; negotiateUrl += index === -1 ? "" : url.substring(index); + + if (negotiateUrl.indexOf("negotiateVersion") === -1) { + negotiateUrl += index === -1 ? "?" : "&"; + negotiateUrl += "negotiateVersion=" + this.negotiateVersion; + } return negotiateUrl; } } diff --git a/src/SignalR/clients/ts/signalr/tests/Common.ts b/src/SignalR/clients/ts/signalr/tests/Common.ts index 7a07a35d3d..6ce9e7779f 100644 --- a/src/SignalR/clients/ts/signalr/tests/Common.ts +++ b/src/SignalR/clients/ts/signalr/tests/Common.ts @@ -15,10 +15,10 @@ export function eachTransport(action: (transport: HttpTransportType) => void) { export function eachEndpointUrl(action: (givenUrl: string, expectedUrl: string) => void) { const urls = [ - [ "http://tempuri.org/endpoint/?q=my/Data", "http://tempuri.org/endpoint/negotiate?q=my/Data" ], - [ "http://tempuri.org/endpoint?q=my/Data", "http://tempuri.org/endpoint/negotiate?q=my/Data" ], - [ "http://tempuri.org/endpoint", "http://tempuri.org/endpoint/negotiate" ], - [ "http://tempuri.org/endpoint/", "http://tempuri.org/endpoint/negotiate" ], + [ "http://tempuri.org/endpoint/?q=my/Data", "http://tempuri.org/endpoint/negotiate?q=my/Data&negotiateVersion=1" ], + [ "http://tempuri.org/endpoint?q=my/Data", "http://tempuri.org/endpoint/negotiate?q=my/Data&negotiateVersion=1" ], + [ "http://tempuri.org/endpoint", "http://tempuri.org/endpoint/negotiate?negotiateVersion=1" ], + [ "http://tempuri.org/endpoint/", "http://tempuri.org/endpoint/negotiate?negotiateVersion=1" ], ]; urls.forEach((t) => action(t[0], t[1])); diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index d720011334..d010fbdbc6 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts @@ -13,6 +13,7 @@ import { EventSourceConstructor, WebSocketConstructor } from "../src/Polyfills"; import { eachEndpointUrl, eachTransport, VerifyLogger } from "./Common"; import { TestHttpClient } from "./TestHttpClient"; import { TestTransport } from "./TestTransport"; +import { TestEvent, TestWebSocket } from "./TestWebSocket"; import { PromiseSource, registerUnhandledRejectionHandler, SyncPoint } from "./Utils"; const commonOptions: IHttpConnectionOptions = { @@ -20,6 +21,7 @@ const commonOptions: IHttpConnectionOptions = { }; const defaultConnectionId = "abc123"; +const defaultConnectionToken = "123abc"; const defaultNegotiateResponse: INegotiateResponse = { availableTransports: [ { transport: "WebSockets", transferFormats: ["Text", "Binary"] }, @@ -27,6 +29,8 @@ const defaultNegotiateResponse: INegotiateResponse = { { transport: "LongPolling", transferFormats: ["Text", "Binary"] }, ], connectionId: defaultConnectionId, + connectionToken: defaultConnectionToken, + negotiateVersion: 1, }; registerUnhandledRejectionHandler(); @@ -571,7 +575,7 @@ describe("HttpConnection", () => { let firstNegotiate = true; let firstPoll = true; const httpClient = new TestHttpClient() - .on("POST", /negotiate$/, () => { + .on("POST", /\/negotiate/, () => { if (firstNegotiate) { firstNegotiate = false; return { url: "https://another.domain.url/chat" }; @@ -602,8 +606,8 @@ describe("HttpConnection", () => { await connection.start(TransferFormat.Text); expect(httpClient.sentRequests.length).toBe(4); - expect(httpClient.sentRequests[0].url).toBe("http://tempuri.org/negotiate"); - expect(httpClient.sentRequests[1].url).toBe("https://another.domain.url/chat/negotiate"); + expect(httpClient.sentRequests[0].url).toBe("http://tempuri.org/negotiate?negotiateVersion=1"); + expect(httpClient.sentRequests[1].url).toBe("https://another.domain.url/chat/negotiate?negotiateVersion=1"); expect(httpClient.sentRequests[2].url).toMatch(/^https:\/\/another\.domain\.url\/chat\?id=0rge0d00-0040-0030-0r00-000q00r00e00/i); expect(httpClient.sentRequests[3].url).toMatch(/^https:\/\/another\.domain\.url\/chat\?id=0rge0d00-0040-0030-0r00-000q00r00e00/i); } finally { @@ -615,7 +619,7 @@ describe("HttpConnection", () => { it("fails to start if negotiate redirects more than 100 times", async () => { await VerifyLogger.run(async (logger) => { const httpClient = new TestHttpClient() - .on("POST", /negotiate$/, () => ({ url: "https://another.domain.url/chat" })); + .on("POST", /\/negotiate/, () => ({ url: "https://another.domain.url/chat" })); const options: IHttpConnectionOptions = { ...commonOptions, @@ -637,7 +641,7 @@ describe("HttpConnection", () => { let firstNegotiate = true; let firstPoll = true; const httpClient = new TestHttpClient() - .on("POST", /negotiate$/, (r) => { + .on("POST", /\/negotiate/, (r) => { if (firstNegotiate) { firstNegotiate = false; @@ -683,8 +687,8 @@ describe("HttpConnection", () => { await connection.start(TransferFormat.Text); expect(httpClient.sentRequests.length).toBe(4); - expect(httpClient.sentRequests[0].url).toBe("http://tempuri.org/negotiate"); - expect(httpClient.sentRequests[1].url).toBe("https://another.domain.url/chat/negotiate"); + expect(httpClient.sentRequests[0].url).toBe("http://tempuri.org/negotiate?negotiateVersion=1"); + expect(httpClient.sentRequests[1].url).toBe("https://another.domain.url/chat/negotiate?negotiateVersion=1"); expect(httpClient.sentRequests[2].url).toMatch(/^https:\/\/another\.domain\.url\/chat\?id=0rge0d00-0040-0030-0r00-000q00r00e00/i); expect(httpClient.sentRequests[3].url).toMatch(/^https:\/\/another\.domain\.url\/chat\?id=0rge0d00-0040-0030-0r00-000q00r00e00/i); } finally { @@ -696,7 +700,7 @@ describe("HttpConnection", () => { it("throws error if negotiate response has error", async () => { await VerifyLogger.run(async (logger) => { const httpClient = new TestHttpClient() - .on("POST", /negotiate$/, () => ({ error: "Negotiate error." })); + .on("POST", /\/negotiate/, () => ({ error: "Negotiate error." })); const options: IHttpConnectionOptions = { ...commonOptions, @@ -873,6 +877,253 @@ describe("HttpConnection", () => { }); }); + it("missing negotiateVersion ignores connectionToken", async () => { + await VerifyLogger.run(async (logger) => { + const availableTransport = { transport: "Custom", transferFormats: ["Text"] }; + const transport = { + connect(url: string, transferFormat: TransferFormat) { + return Promise.resolve(); + }, + send(data: any) { + return Promise.resolve(); + }, + stop() { + if (transport.onclose) { + transport.onclose(); + } + return Promise.resolve(); + }, + onclose: null, + onreceive: null, + } as ITransport; + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", () => ({ connectionId: "42", connectionToken: "token", availableTransports: [availableTransport] })), + logger, + transport, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + connection.onreceive = () => null; + try { + await connection.start(TransferFormat.Text); + expect(connection.connectionId).toBe("42"); + } finally { + await connection.stop(); + } + }); + }); + + it("negotiate version 0 ignores connectionToken", async () => { + await VerifyLogger.run(async (logger) => { + const availableTransport = { transport: "Custom", transferFormats: ["Text"] }; + const transport = { + connect(url: string, transferFormat: TransferFormat) { + return Promise.resolve(); + }, + send(data: any) { + return Promise.resolve(); + }, + stop() { + if (transport.onclose) { + transport.onclose(); + } + return Promise.resolve(); + }, + onclose: null, + onreceive: null, + } as ITransport; + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", () => ({ connectionId: "42", connectionToken: "token", negotiateVersion: 0, availableTransports: [availableTransport] })), + logger, + transport, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + connection.onreceive = () => null; + try { + await connection.start(TransferFormat.Text); + expect(connection.connectionId).toBe("42"); + } finally { + await connection.stop(); + } + }); + }); + + it("negotiate version 1 uses connectionToken for url and connectionId for property", async () => { + await VerifyLogger.run(async (logger) => { + const availableTransport = { transport: "Custom", transferFormats: ["Text"] }; + let connectUrl = ""; + const transport = { + connect(url: string, transferFormat: TransferFormat) { + connectUrl = url; + return Promise.resolve(); + }, + send(data: any) { + return Promise.resolve(); + }, + stop() { + if (transport.onclose) { + transport.onclose(); + } + return Promise.resolve(); + }, + onclose: null, + onreceive: null, + } as ITransport; + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", () => ({ connectionId: "42", connectionToken: "token", negotiateVersion: 1, availableTransports: [availableTransport] })), + logger, + transport, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + connection.onreceive = () => null; + try { + await connection.start(TransferFormat.Text); + expect(connection.connectionId).toBe("42"); + expect(connectUrl).toBe("http://tempuri.org?id=token"); + } finally { + await connection.stop(); + } + }); + }); + + it("negotiateVersion query string not added if already present", async () => { + await VerifyLogger.run(async (logger) => { + const connectUrl = new PromiseSource(); + const fakeTransport: ITransport = { + connect(url: string): Promise { + connectUrl.resolve(url); + return Promise.resolve(); + }, + send(): Promise { + return Promise.resolve(); + }, + stop(): Promise { + return Promise.resolve(); + }, + onclose: null, + onreceive: null, + }; + + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", "http://tempuri.org/negotiate?negotiateVersion=42", () => "{ \"connectionId\": \"42\" }") + .on("GET", () => ""), + logger, + transport: fakeTransport, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org?negotiateVersion=42", options); + try { + const startPromise = connection.start(TransferFormat.Text); + + expect(await connectUrl).toBe("http://tempuri.org?negotiateVersion=42&id=42"); + + await startPromise; + } finally { + (options.transport as ITransport).onclose!(); + await connection.stop(); + } + }); + }); + + it("negotiateVersion query string not added if already present after redirect", async () => { + await VerifyLogger.run(async (logger) => { + const connectUrl = new PromiseSource(); + const fakeTransport: ITransport = { + connect(url: string): Promise { + connectUrl.resolve(url); + return Promise.resolve(); + }, + send(): Promise { + return Promise.resolve(); + }, + stop(): Promise { + return Promise.resolve(); + }, + onclose: null, + onreceive: null, + }; + + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", "http://tempuri.org/negotiate?negotiateVersion=1", () => "{ \"url\": \"http://redirect.org\" }") + .on("POST", "http://redirect.org/negotiate?negotiateVersion=1", () => "{ \"connectionId\": \"42\"}") + .on("GET", () => ""), + logger, + transport: fakeTransport, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + try { + const startPromise = connection.start(TransferFormat.Text); + + expect(await connectUrl).toBe("http://redirect.org?id=42"); + + await startPromise; + } finally { + (options.transport as ITransport).onclose!(); + await connection.stop(); + } + }); + }); + + it("fallback changes connectionId property", async () => { + await VerifyLogger.run(async (logger) => { + const availableTransports = [{ transport: "WebSockets", transferFormats: ["Text"] }, { transport: "LongPolling", transferFormats: ["Text"] }]; + let negotiateCount: number = 0; + let getCount: number = 0; + let connection: HttpConnection; + let connectionId: string | undefined; + const options: IHttpConnectionOptions = { + WebSocket: TestWebSocket, + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", () => { + negotiateCount++; + return ({ connectionId: negotiateCount.toString(), connectionToken: "token", negotiateVersion: 1, availableTransports }); + }) + .on("GET", () => { + getCount++; + if (getCount === 1) { + return new HttpResponse(200); + } + connectionId = connection.connectionId; + return new HttpResponse(204); + }) + .on("DELETE", () => new HttpResponse(202)), + + logger, + } as IHttpConnectionOptions; + + TestWebSocket.webSocketSet = new PromiseSource(); + + connection = new HttpConnection("http://tempuri.org", options); + const startPromise = connection.start(TransferFormat.Text); + + await TestWebSocket.webSocketSet; + await TestWebSocket.webSocket.closeSet; + TestWebSocket.webSocket.onerror(new TestEvent()); + + try { + await startPromise; + } catch { } + + expect(negotiateCount).toEqual(2); + expect(connectionId).toEqual("2"); + }, + "Failed to start the transport 'WebSockets': Error: There was an error with the transport."); + }); + describe(".constructor", () => { it("throws if no Url is provided", async () => { // Force TypeScript to let us call the constructor incorrectly :) @@ -921,7 +1172,7 @@ describe("HttpConnection", () => { it("uses WebSocket constructor from options if provided", async () => { await VerifyLogger.run(async (logger) => { - class TestWebSocket { + class BadConstructorWebSocket { // The "_" prefix tell TypeScript not to worry about unused parameter, but tslint doesn't like it. // tslint:disable-next-line:variable-name constructor(_url: string, _protocols?: string | string[]) { @@ -931,7 +1182,7 @@ describe("HttpConnection", () => { const options: IHttpConnectionOptions = { ...commonOptions, - WebSocket: TestWebSocket as WebSocketConstructor, + WebSocket: BadConstructorWebSocket as WebSocketConstructor, logger, skipNegotiation: true, transport: HttpTransportType.WebSockets, diff --git a/src/SignalR/clients/ts/signalr/tests/HubConnectionBuilder.test.ts b/src/SignalR/clients/ts/signalr/tests/HubConnectionBuilder.test.ts index 4aaa70a0d8..e43f3a336c 100644 --- a/src/SignalR/clients/ts/signalr/tests/HubConnectionBuilder.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HubConnectionBuilder.test.ts @@ -21,6 +21,8 @@ const longPollingNegotiateResponse = { { transport: "LongPolling", transferFormats: ["Text", "Binary"] }, ], connectionId: "abc123", + connectionToken: "123abc", + negotiateVersion: 1, }; const commonHttpOptions: IHttpConnectionOptions = { @@ -88,7 +90,7 @@ describe("HubConnectionBuilder", () => { const pollSent = new PromiseSource(); const pollCompleted = new PromiseSource(); const testClient = createTestClient(pollSent, pollCompleted.promise) - .on("POST", "http://example.com?id=abc123", (req) => { + .on("POST", "http://example.com?id=123abc", (req) => { // Respond from the poll with the handshake response pollCompleted.resolve(new HttpResponse(204, "No Content", "{}")); return new HttpResponse(202); @@ -104,7 +106,7 @@ describe("HubConnectionBuilder", () => { await expect(connection.start()).rejects.toThrow("The underlying connection was closed before the hub handshake could complete."); expect(connection.state).toBe(HubConnectionState.Disconnected); - expect((await pollSent.promise).url).toMatch(/http:\/\/example.com\?id=abc123.*/); + expect((await pollSent.promise).url).toMatch(/http:\/\/example.com\?id=123abc.*/); }); }); @@ -125,7 +127,7 @@ describe("HubConnectionBuilder", () => { const pollCompleted = new PromiseSource(); let negotiateRequest!: HttpRequest; const testClient = createTestClient(pollSent, pollCompleted.promise) - .on("POST", "http://example.com?id=abc123", (req) => { + .on("POST", "http://example.com?id=123abc", (req) => { // Respond from the poll with the handshake response negotiateRequest = req; pollCompleted.resolve(new HttpResponse(204, "No Content", "{}")); @@ -219,7 +221,7 @@ describe("HubConnectionBuilder", () => { const pollSent = new PromiseSource(); const pollCompleted = new PromiseSource(); const testClient = createTestClient(pollSent, pollCompleted.promise) - .on("POST", "http://example.com?id=abc123", (req) => { + .on("POST", "http://example.com?id=123abc", (req) => { // Respond from the poll with the handshake response pollCompleted.resolve(new HttpResponse(204, "No Content", "{}")); return new HttpResponse(202); @@ -244,7 +246,7 @@ describe("HubConnectionBuilder", () => { const pollSent = new PromiseSource(); const pollCompleted = new PromiseSource(); const testClient = createTestClient(pollSent, pollCompleted.promise) - .on("POST", "http://example.com?id=abc123", (req) => { + .on("POST", "http://example.com?id=123abc", (req) => { // Respond from the poll with the handshake response pollCompleted.resolve(new HttpResponse(204, "No Content", "{}")); return new HttpResponse(202); @@ -274,7 +276,7 @@ describe("HubConnectionBuilder", () => { const pollSent = new PromiseSource(); const pollCompleted = new PromiseSource(); const testClient = createTestClient(pollSent, pollCompleted.promise) - .on("POST", "http://example.com?id=abc123", (req) => { + .on("POST", "http://example.com?id=123abc", (req) => { // Respond from the poll with the handshake response pollCompleted.resolve(new HttpResponse(204, "No Content", "{}")); return new HttpResponse(202); @@ -413,8 +415,8 @@ function createConnectionBuilder(logger?: ILogger): HubConnectionBuilder { function createTestClient(pollSent: PromiseSource, pollCompleted: Promise, negotiateResponse?: any): TestHttpClient { let firstRequest = true; return new TestHttpClient() - .on("POST", "http://example.com/negotiate", () => negotiateResponse || longPollingNegotiateResponse) - .on("GET", /http:\/\/example.com\?id=abc123&_=.*/, (req) => { + .on("POST", "http://example.com/negotiate?negotiateVersion=1", () => negotiateResponse || longPollingNegotiateResponse) + .on("GET", /http:\/\/example.com\?id=123abc&_=.*/, (req) => { if (firstRequest) { firstRequest = false; return new HttpResponse(200);