From 02977f41a8e836529ca5d3b387569b65208f55b4 Mon Sep 17 00:00:00 2001 From: stevengum <14935595+stevengum@users.noreply.github.com> Date: Tue, 19 Nov 2019 22:01:45 -0800 Subject: [PATCH 1/5] rewrite useWebSocket for low-level usage, remove from processActivity --- .../botbuilder/src/botFrameworkAdapter.ts | 48 ++-- .../botFrameworkAdapterStreaming.test.js | 231 ++++++------------ .../tests/streaming/mockHttpRequest.js | 32 +++ .../tests/streaming/mockNetSocket.js | 18 ++ 4 files changed, 146 insertions(+), 183 deletions(-) create mode 100644 libraries/botbuilder/tests/streaming/mockHttpRequest.js create mode 100644 libraries/botbuilder/tests/streaming/mockNetSocket.js diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index 21c57450d9..4fe7132cf9 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/libraries/botbuilder/src/botFrameworkAdapter.ts @@ -6,7 +6,8 @@ * Licensed under the MIT License. */ -import { IncomingMessage } from 'http'; +import { IncomingMessage, STATUS_CODES } from 'http'; +import { Socket } from 'net'; import * as os from 'os'; import { Activity, ActivityTypes, BotAdapter, BotCallbackHandlerKey, ChannelAccount, ConversationAccount, ConversationParameters, ConversationReference, ConversationsResult, IUserTokenProvider, ResourceResponse, TokenResponse, TurnContext } from 'botbuilder-core'; @@ -755,9 +756,6 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide * ``` */ public async processActivity(req: WebRequest, res: WebResponse, logic: (context: TurnContext) => Promise): Promise { - if (this.settings.enableWebSockets && req.method === GET && (req.headers.Upgrade || req.headers.upgrade)) { - return this.useWebSocket(req, res, logic); - } let body: any; let status: number; @@ -1151,36 +1149,37 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide * @param res The response sent on error or connection termination. * @param logic The logic that will handle incoming requests. */ - public async useWebSocket(req: WebRequest, res: WebResponse, logic: (context: TurnContext) => Promise): Promise { - if (!logic) { - throw new Error('Streaming logic needs to be provided to `useWebSocket`'); - } - + public async useWebSocket(req: IncomingMessage, socket: Socket, head: Buffer, logic: (context: TurnContext) => Promise): Promise { if (!this.webSocketFactory || !this.webSocketFactory.createWebSocket) { throw new Error('BotFrameworkAdapter must have a WebSocketFactory in order to support streaming.'); } - this.logic = logic; - - // Restify-specific check. - if (typeof((res as any).claimUpgrade) !== 'function') { - throw new Error('ClaimUpgrade is required for creating WebSocket connection.'); + if (!logic) { + throw new Error('Streaming logic needs to be provided to `useWebSocket`'); } + this.logic = logic; + try { await this.authenticateConnection(req, this.settings.channelService); } catch (err) { - // Set the correct status code for the socket to send back to the channel. - res.status(StatusCodes.UNAUTHORIZED); - res.send(err.message); + // If the authenticateConnection call fails, send back the correct error code and close + // the connection. + if (typeof(err.message) === 'string' && err.message.toLowerCase().startsWith('unauthorized')) { + abortWebSocketUpgrade(socket, 401); + } else if (typeof(err.message) === 'string' && err.message.toLowerCase().startsWith(`'authheader'`)) { + abortWebSocketUpgrade(socket, 400); + } else { + abortWebSocketUpgrade(socket, 500); + } + // Re-throw the error so the developer will know what occurred. throw err; } - const upgrade = (res as any).claimUpgrade(); - const socket = await this.webSocketFactory.createWebSocket(req as IncomingMessage, upgrade.socket, upgrade.head); + const nodeWebSocket = await this.webSocketFactory.createWebSocket(req, socket, head); - await this.startWebSocket(socket); + await this.startWebSocket(nodeWebSocket); } private async authenticateConnection(req: WebRequest, channelService?: string): Promise { @@ -1303,4 +1302,13 @@ function delay(timeout: number): Promise { return new Promise((resolve) => { setTimeout(resolve, timeout); }); +} + +function abortWebSocketUpgrade(socket: Socket, code: number) { + if (socket.writable) { + const connectionHeader = `Connection: 'close'\r\n`; + socket.write(`HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\n${connectionHeader}\r\n`); + } + + socket.destroy(); } \ No newline at end of file diff --git a/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js b/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js index 2e53733940..0d119eac56 100644 --- a/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js +++ b/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js @@ -1,67 +1,19 @@ -const { MockContentStream, MockStreamingRequest } = require('./mockStreamingRequest'); -const { BotFrameworkAdapter, StatusCodes } = require('../../'); -const { ActivityHandler, ActivityTypes } = require('botbuilder-core'); -const chai = require('chai'); -const { randomBytes } = require('crypto'); const { Socket } = require('net'); -const sinon = require('sinon'); -const expect = chai.expect; - -const createNetSocket = (readable = true, writable = true) => { - return new Socket({ readable, writable }); -}; - -class TestRequest { - constructor(method = 'GET',) { - this.method = method; - let headers = []; - } - - status() { - return this.statusVal; - } - - status(value) { - this.statusVal = value; - } - - streams(value) { - this.streamsVal = value; - } - - streams() { - return this.streamsVal; - } - - setHeaders() { - return this.headersVal; - } - - setHeaders(value) { - this.headers = value; - } - -} -class TestResponse { - send(value) { - this.sendVal = value; - return this.sendVal; - } +const { expect } = require('chai'); +const { spy } = require('sinon'); +const { ActivityHandler, ActivityTypes } = require('botbuilder-core'); - status(value) { - this.statusVal = value; - return this.statusVal; - } +const { BotFrameworkAdapter, StatusCodes } = require('../../'); - setClaimUpgrade(value) { - this.claimUpgradeVal = value; - } +// Import Helper Classes +const { MockHttpRequest } = require('./mockHttpRequest'); +const { MockNetSocket } = require('./mockNetSocket'); +const { MockContentStream, MockStreamingRequest } = require('./mockStreamingRequest'); - claimUpgrade() { - return this.claimUpgradeVal; - } -} +const createNetSocket = (readable = true, writable = true) => { + return new Socket({ readable, writable }); +}; class TestAdapterSettings { constructor(appId = undefined, appPassword = undefined, channelAuthTenant, oAuthEndpoint, openIdMetadata, channelServce) { @@ -102,20 +54,10 @@ describe('BotFrameworkAdapter Streaming tests', () => { it('starts and stops a websocket server', async () => { const bot = new ActivityHandler(); const adapter = new BotFrameworkAdapter(new TestAdapterSettings()); - const request = new TestRequest(); - - request.headers = []; - request.headers['upgrade'] = 'websocket'; - request.headers['sec-websocket-key'] = randomBytes(16).toString('base64'); - request.headers['sec-websocket-version'] = '13'; - request.headers['sec-websocket-protocol'] = ''; - - const response = new TestResponse({ claimUpgrade: 'anything' }); - const mockedSocket = createNetSocket(); - - response.socket = mockedSocket; - response.setClaimUpgrade({ socket: mockedSocket, head: 'websocket' }); - await adapter.useWebSocket(request, response, async (context) => { + const request = new MockHttpRequest(); + const realSocket = createNetSocket(); + + await adapter.useWebSocket(request, realSocket, Buffer.from([]), async (context) => { await bot.run(context); }); }); @@ -123,20 +65,10 @@ describe('BotFrameworkAdapter Streaming tests', () => { it('returns a connector client', async () => { const bot = new ActivityHandler(); const adapter = new BotFrameworkAdapter(new TestAdapterSettings()); - let request = new TestRequest(); - - request.headers = []; - request.headers['upgrade'] = 'websocket'; - request.headers['sec-websocket-key'] = randomBytes(16).toString('base64'); - request.headers['sec-websocket-version'] = '13'; - request.headers['sec-websocket-protocol'] = ''; - let response = new TestResponse(); - const mockedSocket = createNetSocket(); - - response.socket = mockedSocket; - response.setClaimUpgrade({ socket: mockedSocket, head: 'websocket' }); - - await adapter.useWebSocket(request, response, async (context) => { + const request = new MockHttpRequest(); + const realSocket = createNetSocket(); + + await adapter.useWebSocket(request, realSocket, Buffer.from([]), async (context) => { await bot.run(context); }); const cc = adapter.createConnectorClient('urn:test'); @@ -147,38 +79,36 @@ describe('BotFrameworkAdapter Streaming tests', () => { it('connects', async () => { const bot = new ActivityHandler(); const adapter = new BotFrameworkAdapter(new TestAdapterSettings()); - let request = new TestRequest(); - - request.headers = []; - request.headers['upgrade'] = 'websocket'; - request.headers['sec-websocket-key'] = randomBytes(16).toString('base64'); - request.headers['sec-websocket-version'] = '13'; - request.headers['sec-websocket-protocol'] = ''; - let response = new TestResponse(); - const mockedSocket = createNetSocket(); - - response.socket = mockedSocket; - response.setClaimUpgrade({ socket: mockedSocket, head: 'websocket' }); - - await adapter.useWebSocket(request, response, async (context) => { + const request = new MockHttpRequest(); + const realSocket = createNetSocket(); + + const writeSpy = spy(realSocket, 'write'); + await adapter.useWebSocket(request, realSocket, Buffer.from([]), async (context) => { await bot.run(context); }); + expect(writeSpy.called).to.be.true; }); it('returns status code 401 when request is not authorized', async () => { const bot = new ActivityHandler(); const settings = new TestAdapterSettings('appId', 'password'); const adapter = new BotFrameworkAdapter(settings); - let request = new TestRequest(); - - request.setHeaders({ channelid: 'fakechannel', authorization: 'donttrustme' }); - let response = new TestResponse(); + const request = new MockHttpRequest(); + request.setHeader('authorization', 'donttustme'); - await adapter.useWebSocket(request, response, async (context) => { + const socket = new MockNetSocket(); + const writeSpy = spy(socket, 'write'); + const destroySpy = spy(socket, 'destroy'); + + await adapter.useWebSocket(request, socket, Buffer.from([]), async (context) => { await bot.run(context); throw new Error('useWebSocket should have thrown an error'); }).catch(err => { expect(err.message).to.equal('Unauthorized. Is not authenticated'); + const socketResponse = MockNetSocket.createNonSuccessResponse(401); + expect(writeSpy.called).to.be.true; + expect(writeSpy.calledWithExactly(socketResponse)).to.be.true; + expect(destroySpy.calledOnceWithExactly()).to.be.true; }); }); }); @@ -282,13 +212,7 @@ describe('BotFrameworkAdapter Streaming tests', () => { it('returns a 501 when activity type is invoke, but the activity is invalid', async () => { const bot = new ActivityHandler(); const adapter = new BotFrameworkAdapter(); - let request = new TestRequest(); - request.verb = 'POST'; - request.path = '/api/messages'; - let fakeStream = { - readAsJson: function () { return { type: 'invoke', serviceUrl: 'somewhere/', channelId: 'test' }; }, - }; - request.streams[0] = fakeStream; + const request = new MockStreamingRequest(); adapter.logic = async (context) => { await bot.run(context); @@ -298,53 +222,48 @@ describe('BotFrameworkAdapter Streaming tests', () => { expect(response.statusCode).to.equal(501); }); - it('returns a 500 when bot can not run', async () => { - const MiddleWare = require('botbuilder-core'); - const bot = {}; - let mw = { + it('returns a 500 when BotFrameworkAdapter.logic is not callable', async () => { + const adapter = new BotFrameworkAdapter(); + const request = new MockStreamingRequest(); + + const response = await adapter.processRequest(request); + expect(response.statusCode).to.equal(500); + }); + + it('returns a 500 and calls middleware when BotFrameworkAdapter.logic is not callable', async () => { + let middlewareCalled = false; + const middleware = { async onTurn(context, next) { - console.log('Middleware executed!'); + middlewareCalled = true; await next(); } }; - let mwset = []; - mwset.push(mw); - const adapter = new BotFrameworkAdapter({ bot: bot, middleWare: mwset }); - let request = new TestRequest(); - request.verb = 'POST'; - request.path = '/api/messages'; - let fakeStream = { - readAsJson: function () { return { type: 'invoke', serviceUrl: 'somewhere/', channelId: 'test' }; }, - }; - request.streams[0] = fakeStream; + + const adapter = new BotFrameworkAdapter(); + adapter.use(middleware); + const request = new MockStreamingRequest(); const response = await adapter.processRequest(request); expect(response.statusCode).to.equal(500); + expect(middlewareCalled).to.be.true; }); it('executes middleware', async () => { const bot = new ActivityHandler(); - bot.run = function (turnContext) { return Promise.resolve(); }; - const adapter = new BotFrameworkAdapter(); + let middlewareCalled = false; const middleware = { async onTurn(context, next) { middlewareCalled = true; return next(); } - } + }; adapter.use(middleware); - const runSpy = sinon.spy(bot, 'run'); - let request = new TestRequest(); - request.verb = 'POST'; - request.path = '/api/messages'; - let fakeStream = { - readAsJson: function () { return { type: 'invoke', serviceUrl: 'somewhere/', channelId: 'test' }; }, - }; - request.streams[0] = fakeStream; + const runSpy = spy(bot, 'run'); + const request = new MockStreamingRequest(); adapter.logic = async (context) => { await bot.run(context); @@ -360,32 +279,18 @@ describe('BotFrameworkAdapter Streaming tests', () => { it('sends a request', async () => { const bot = new ActivityHandler(); const adapter = new BotFrameworkAdapter(new TestAdapterSettings()); - let request = new TestRequest(); - - request.headers = []; - request.headers['upgrade'] = 'websocket'; - request.headers['sec-websocket-key'] = randomBytes(16).toString('base64'); - request.headers['sec-websocket-version'] = '13'; - request.headers['sec-websocket-protocol'] = ''; - let response = new TestResponse(); - - const mockedSocket = createNetSocket(); - - response.socket = mockedSocket; - const spy = sinon.spy(mockedSocket, "write"); - response.setClaimUpgrade({ socket: mockedSocket, head: 'websocket' }); - - try { - await adapter.useWebSocket(request, response, async (context) => { - await bot.run(context); - }) - } catch (err) { - throw err; - } + const request = new MockHttpRequest(); + const realSocket = createNetSocket(); + + const writeSpy = spy(realSocket, 'write'); + + await adapter.useWebSocket(request, realSocket, Buffer.from([]), async (context) => { + await bot.run(context); + }); - let connection = adapter.createConnectorClient('fakeUrl'); + const connection = adapter.createConnectorClient('fakeUrl'); connection.sendRequest({ method: 'POST', url: 'testResultDotCom', body: 'Test body!' }); - expect(spy.called).to.be.true; + expect(writeSpy.called).to.be.true; }).timeout(2000); describe('private methods', () => { diff --git a/libraries/botbuilder/tests/streaming/mockHttpRequest.js b/libraries/botbuilder/tests/streaming/mockHttpRequest.js new file mode 100644 index 0000000000..c118b24591 --- /dev/null +++ b/libraries/botbuilder/tests/streaming/mockHttpRequest.js @@ -0,0 +1,32 @@ +const { randomBytes } = require('crypto'); + +class MockHttpRequest { + constructor(options = {}) { + const config = Object.assign({ + method: 'GET', + headers: { + 'upgrade': 'websocket', + 'sec-websocket-key': randomBytes(16).toString('base64'), + 'sec-websocket-version': '13', + 'sec-websocket-protocol': '' + } + }, options); + + this.method = config.method; + this.headers = config.headers; + } + + setHeader(key, value) { + this.headers[key] = value; + } + + streams(value) { + this.streamsVal = value; + } + + streams() { + return this.streamsVal; + } +} + +module.exports.MockHttpRequest = MockHttpRequest; diff --git a/libraries/botbuilder/tests/streaming/mockNetSocket.js b/libraries/botbuilder/tests/streaming/mockNetSocket.js new file mode 100644 index 0000000000..04379ff575 --- /dev/null +++ b/libraries/botbuilder/tests/streaming/mockNetSocket.js @@ -0,0 +1,18 @@ +const { STATUS_CODES } = require('http'); + +class MockNetSocket { + constructor(readable = true, writable = true) { + this.readable = readable; + this.writable = writable; + } + + write(response) { } + + destroy(err) { } +} + +MockNetSocket.createNonSuccessResponse = (code) => { + return `HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\nConnection: 'close'\r\n\r\n`; +}; + +module.exports.MockNetSocket = MockNetSocket; From 5d01690ac855446d568a1af5c2b2fb028b92883d Mon Sep 17 00:00:00 2001 From: Steven Gum <14935595+stevengum@users.noreply.github.com> Date: Thu, 21 Nov 2019 14:43:06 -0800 Subject: [PATCH 2/5] [4.6.x] Fix set-dependency-versions script (#1438) * change set-dependency-versions script to use ~ not ^, update changed libs * use pinned versions for intra-dependencies per @cleemullins feedback --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c6064652b0..6141469dda 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "build-docs": "lerna run build-docs", "eslint": "eslint ./libraries/*/src/*.ts ./libraries/*/src/**/*.ts", "eslint-fix": "eslint ./libraries/*/src/*.ts ./libraries/*/src/**/*.ts --fix", - "set-dependency-versions": "node tools/util/updateDependenciesInPackageJsons.js ./libraries ^${Version} botframework-streaming botbuilder botbuilder-choices botbuilder-dialogs botbuilder-core botbuilder-prompts botbuilder-testing botframework-connector botframework-config botframework-schema testbot && node tools/util/updateDependenciesInPackageJsons.js ./transcripts ^${Version} botframework-streaming botbuilder botbuilder-ai botbuilder-dialogs botbuilder-testing", + "set-dependency-versions": "node tools/util/updateDependenciesInPackageJsons.js ./libraries ${Version} botframework-streaming botbuilder botbuilder-ai botbuilder-dialogs botbuilder-core botbuilder-applicationinsights botbuilder-testing botframework-connector botframework-config botframework-schema testbot && node tools/util/updateDependenciesInPackageJsons.js ./transcripts ${Version} botbuilder botbuilder-ai botbuilder-dialogs", "update-versions": "lerna run set-version && npm run set-dependency-versions" }, "dependencies": { From 06cc3e4a45da1d6931d1a06c1cbd2a4b59016175 Mon Sep 17 00:00:00 2001 From: Steven Gum <14935595+stevengum@users.noreply.github.com> Date: Thu, 21 Nov 2019 12:53:04 -0800 Subject: [PATCH 3/5] [4.6.x] Pin TypeScript devDependency in each library. (Fixes #1436) (#1437) * move pinned typescript@3.5.2 devDependency into each package * pin transcripts/ botbuilder dependencies, fix import in Skype Middleware * bump to typescript@3.5.3 --- libraries/botbuilder-ai/package.json | 3 ++- libraries/botbuilder-applicationinsights/package.json | 3 ++- libraries/botbuilder-azure/package.json | 5 +++-- libraries/botbuilder-core/package.json | 1 + .../src/skypeMentionNormalizeMiddleware.ts | 4 +++- libraries/botbuilder-dialogs/package.json | 3 ++- libraries/botbuilder-testing/package.json | 1 + libraries/botbuilder/package.json | 1 + libraries/botframework-config/package.json | 3 ++- libraries/botframework-connector/package.json | 3 ++- libraries/botframework-schema/package.json | 7 ++++--- libraries/botframework-streaming/package.json | 3 ++- package.json | 9 ++++----- transcripts/package.json | 6 +++--- 14 files changed, 32 insertions(+), 20 deletions(-) diff --git a/libraries/botbuilder-ai/package.json b/libraries/botbuilder-ai/package.json index 83d3cf5bce..dabecca65f 100644 --- a/libraries/botbuilder-ai/package.json +++ b/libraries/botbuilder-ai/package.json @@ -38,7 +38,8 @@ "nock": "^10.0.3", "nyc": "^11.4.1", "source-map-support": "^0.5.3", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "test": "tsc && nyc mocha tests/", diff --git a/libraries/botbuilder-applicationinsights/package.json b/libraries/botbuilder-applicationinsights/package.json index 9b73683a6d..39f24c5c5b 100644 --- a/libraries/botbuilder-applicationinsights/package.json +++ b/libraries/botbuilder-applicationinsights/package.json @@ -33,7 +33,8 @@ "mocha": "^5.2.0", "nyc": "^11.4.1", "source-map-support": "^0.5.3", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "test": "tsc && nyc mocha tests/", diff --git a/libraries/botbuilder-azure/package.json b/libraries/botbuilder-azure/package.json index 5eb875ccbc..8995d9b98b 100644 --- a/libraries/botbuilder-azure/package.json +++ b/libraries/botbuilder-azure/package.json @@ -35,10 +35,11 @@ "@types/semaphore": "^1.1.0", "codelyzer": "^4.1.0", "mocha": "^5.2.0", - "nyc": "^11.4.1", "nock": "^10.0.3", + "nyc": "^11.4.1", "source-map-support": "^0.5.3", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "test": "tsc && nyc mocha tests/", diff --git a/libraries/botbuilder-core/package.json b/libraries/botbuilder-core/package.json index 1c3d309349..200e646878 100644 --- a/libraries/botbuilder-core/package.json +++ b/libraries/botbuilder-core/package.json @@ -30,6 +30,7 @@ "nyc": "^11.4.1", "source-map-support": "^0.5.3", "ts-node": "^4.1.0", + "typescript": "3.5.3", "unzip": "^0.1.11" }, "scripts": { diff --git a/libraries/botbuilder-core/src/skypeMentionNormalizeMiddleware.ts b/libraries/botbuilder-core/src/skypeMentionNormalizeMiddleware.ts index 770bd97526..48fd4c2e28 100644 --- a/libraries/botbuilder-core/src/skypeMentionNormalizeMiddleware.ts +++ b/libraries/botbuilder-core/src/skypeMentionNormalizeMiddleware.ts @@ -5,7 +5,9 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { Activity, Middleware, TurnContext } from 'botbuilder-core'; +import { Activity } from 'botframework-schema'; +import { Middleware } from './middlewareSet'; +import { TurnContext } from './turnContext'; /** diff --git a/libraries/botbuilder-dialogs/package.json b/libraries/botbuilder-dialogs/package.json index 5d575a6f23..29faa7e76d 100644 --- a/libraries/botbuilder-dialogs/package.json +++ b/libraries/botbuilder-dialogs/package.json @@ -35,7 +35,8 @@ "mocha": "^5.2.0", "nyc": "^11.4.1", "source-map-support": "^0.5.3", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "test": "tsc && nyc mocha tests/", diff --git a/libraries/botbuilder-testing/package.json b/libraries/botbuilder-testing/package.json index 651c5a6a1a..cefa88c438 100644 --- a/libraries/botbuilder-testing/package.json +++ b/libraries/botbuilder-testing/package.json @@ -34,6 +34,7 @@ "nyc": "^11.4.1", "source-map-support": "^0.5.3", "ts-node": "^4.1.0", + "typescript": "3.5.3", "unzip": "^0.1.11", "uuid": "^3.3.2" }, diff --git a/libraries/botbuilder/package.json b/libraries/botbuilder/package.json index 48005fed96..17f5fbd9dd 100644 --- a/libraries/botbuilder/package.json +++ b/libraries/botbuilder/package.json @@ -36,6 +36,7 @@ "nyc": "^11.4.1", "source-map-support": "^0.5.3", "ts-node": "^4.1.0", + "typescript": "3.5.3", "uuid": "^3.3.2" }, "scripts": { diff --git a/libraries/botframework-config/package.json b/libraries/botframework-config/package.json index 3e3ec65b67..983b13ee4f 100644 --- a/libraries/botframework-config/package.json +++ b/libraries/botframework-config/package.json @@ -26,7 +26,8 @@ }, "devDependencies": { "@types/uuid": "^3.4.3", - "mocha": "^5.2.0" + "mocha": "^5.2.0", + "typescript": "3.5.3" }, "dependencies": { "fs-extra": "^7.0.0", diff --git a/libraries/botframework-connector/package.json b/libraries/botframework-connector/package.json index 48868f5355..86e65e69b1 100644 --- a/libraries/botframework-connector/package.json +++ b/libraries/botframework-connector/package.json @@ -38,7 +38,8 @@ "nyc": "^11.4.1", "should": "^13.2.3", "source-map-support": "^0.5.3", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "build": "tsc", diff --git a/libraries/botframework-schema/package.json b/libraries/botframework-schema/package.json index cde527ed03..054ec2f55d 100644 --- a/libraries/botframework-schema/package.json +++ b/libraries/botframework-schema/package.json @@ -9,8 +9,9 @@ ], "main": "./lib/index.js", "typings": "./lib/index.d.ts", - "dependencies": {}, - "devDependencies": {}, + "devDependencies": { + "typescript": "3.5.3" + }, "scripts": { "build": "tsc", "clean": "erase /q /s .\\lib", @@ -26,6 +27,6 @@ }, "files": [ "/lib", - "/src" + "/src" ] } diff --git a/libraries/botframework-streaming/package.json b/libraries/botframework-streaming/package.json index a837998579..78d3a8e4d0 100644 --- a/libraries/botframework-streaming/package.json +++ b/libraries/botframework-streaming/package.json @@ -37,7 +37,8 @@ "mocha": "^6.2.0", "nyc": "^14.1.1", "sinon": "^7.4.1", - "ts-node": "^4.1.0" + "ts-node": "^4.1.0", + "typescript": "3.5.3" }, "scripts": { "build": "tsc", diff --git a/package.json b/package.json index 6141469dda..e640f8d923 100644 --- a/package.json +++ b/package.json @@ -6,9 +6,9 @@ "build": "lerna run build", "clean": "lerna run clean", "functional-test": "lerna run build && nyc mocha \"libraries/functional-tests/tests/*.test.js\"", - "test": "lerna run build && nyc mocha \"libraries/bot*/tests/*.test.js\"", - "test:coveralls": "lerna run build && nyc mocha \"libraries/bot*/tests/*.test.js\" && nyc report --reporter=text-lcov | coveralls", - "test-coverage": "nyc mocha \"libraries/bot*/tests/*.test.js\" ", + "test": "lerna run build && nyc mocha \"libraries/bot*/tests/**/*.test.js\"", + "test:coveralls": "lerna run build && nyc mocha \"libraries/bot*/tests/**/*.test.js\" && nyc report --reporter=text-lcov | coveralls", + "test-coverage": "nyc mocha \"libraries/bot*/tests/**/*.test.js\" ", "upload-coverage": "nyc report --reporter=text-lcov | coveralls", "build-docs": "lerna run build-docs", "eslint": "eslint ./libraries/*/src/*.ts ./libraries/*/src/**/*.ts", @@ -33,8 +33,7 @@ "sinon": "^7.3.2", "typedoc": "^0.15.0", "typedoc-plugin-external-module-name": "^2.1.0", - "typedoc-plugin-markdown": "^2.2.10", - "typescript": "^3.5.2" + "typedoc-plugin-markdown": "^2.2.10" }, "nyc": { "exclude": [ diff --git a/transcripts/package.json b/transcripts/package.json index 540633c258..8da5a16101 100644 --- a/transcripts/package.json +++ b/transcripts/package.json @@ -10,9 +10,9 @@ "dependencies": { "@types/node": "^10.12.18", "@types/restify": "^7.2.1", - "botbuilder": "^4.2.1", - "botbuilder-ai": "^4.2.1", - "botbuilder-dialogs": "^4.2.1" + "botbuilder": "4.1.6", + "botbuilder-ai": "4.1.6", + "botbuilder-dialogs": "4.1.6" }, "devDependencies": { "mocha": "^5.2.0", From b08c9facfb14361725998707bc731ee674cffada Mon Sep 17 00:00:00 2001 From: stgum <14935595+stevengum@users.noreply.github.com> Date: Fri, 22 Nov 2019 16:08:24 -0800 Subject: [PATCH 4/5] remove unnecessary enableWebSockets flag, cleanup useWebSocket() --- .../botbuilder/src/botFrameworkAdapter.ts | 29 +++++++------------ .../botFrameworkAdapterStreaming.test.js | 3 +- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index 4fe7132cf9..c9ed62ed79 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/libraries/botbuilder/src/botFrameworkAdapter.ts @@ -136,12 +136,7 @@ export interface BotFrameworkAdapterSettings { channelService?: string; /** - * Optional. The option to determine if this adapter accepts WebSocket connections - */ - enableWebSockets?: boolean; - - /** - * Optional. Used to pass in a NodeWebSocketFactoryBase instance. Allows bot to accept WebSocket connections. + * Optional. Used to pass in a NodeWebSocketFactoryBase instance. */ webSocketFactory?: NodeWebSocketFactoryBase; } @@ -269,12 +264,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide this.credentials.oAuthScope = GovernmentConstants.ToChannelFromBotOAuthScope; } - // If the developer wants to use WebSockets, but didn't provide a WebSocketFactory, - // create a NodeWebSocketFactory. - if (this.settings.enableWebSockets && !this.settings.webSocketFactory) { - this.webSocketFactory = new NodeWebSocketFactory(); - } - + // If a NodeWebSocketFactoryBase was passed in, set it on the BotFrameworkAdapter. if (this.settings.webSocketFactory) { this.webSocketFactory = this.settings.webSocketFactory; } @@ -1146,13 +1136,14 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide /** * Process the initial request to establish a long lived connection via a streaming server. * @param req The connection request. - * @param res The response sent on error or connection termination. - * @param logic The logic that will handle incoming requests. + * @param socket The raw socket connection between the bot (server) and channel/caller (client). + * @param head The first packet of the upgraded stream. + * @param logic The logic that handles incoming streaming requests for the lifetime of the WebSocket connection. */ - public async useWebSocket(req: IncomingMessage, socket: Socket, head: Buffer, logic: (context: TurnContext) => Promise): Promise { - if (!this.webSocketFactory || !this.webSocketFactory.createWebSocket) { - throw new Error('BotFrameworkAdapter must have a WebSocketFactory in order to support streaming.'); - } + public async useWebSocket(req: IncomingMessage, socket: Socket, head: Buffer, logic: (context: TurnContext) => Promise): Promise { + // Use the provided NodeWebSocketFactoryBase on BotFrameworkAdapter construction, + // otherwise create a new NodeWebSocketFactory. + const webSocketFactory = this.webSocketFactory || new NodeWebSocketFactory(); if (!logic) { throw new Error('Streaming logic needs to be provided to `useWebSocket`'); @@ -1177,7 +1168,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide throw err; } - const nodeWebSocket = await this.webSocketFactory.createWebSocket(req, socket, head); + const nodeWebSocket = await webSocketFactory.createWebSocket(req, socket, head); await this.startWebSocket(nodeWebSocket); } diff --git a/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js b/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js index 0d119eac56..c4a1a92e35 100644 --- a/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js +++ b/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js @@ -16,10 +16,9 @@ const createNetSocket = (readable = true, writable = true) => { }; class TestAdapterSettings { - constructor(appId = undefined, appPassword = undefined, channelAuthTenant, oAuthEndpoint, openIdMetadata, channelServce) { + constructor(appId, appPassword) { this.appId = appId; this.appPassword = appPassword; - this.enableWebSockets = true; } } From 50a59e6a314d20800ec28a43c3766bad9773e999 Mon Sep 17 00:00:00 2001 From: stevengum <14935595+stevengum@users.noreply.github.com> Date: Sun, 1 Dec 2019 14:07:29 -0800 Subject: [PATCH 5/5] create Node Interfaces for PR feedback --- .../botbuilder/src/botFrameworkAdapter.ts | 9 ++++--- libraries/botframework-streaming/src/index.ts | 3 +++ .../src/interfaces/INodeBuffer.ts | 14 +++++++++++ .../src/interfaces/INodeIncomingMessage.ts | 24 +++++++++++++++++++ .../src/interfaces/INodeSocket.ts | 18 ++++++++++++++ .../src/interfaces/index.ts | 4 ++++ .../factories/nodeWebSocketFactory.ts | 6 ++--- .../factories/nodeWebSocketFactoryBase.ts | 6 ++--- .../src/webSocket/nodeWebSocket.ts | 14 +++++------ 9 files changed, 78 insertions(+), 20 deletions(-) create mode 100644 libraries/botframework-streaming/src/interfaces/INodeBuffer.ts create mode 100644 libraries/botframework-streaming/src/interfaces/INodeIncomingMessage.ts create mode 100644 libraries/botframework-streaming/src/interfaces/INodeSocket.ts diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index 4fe7132cf9..0d6969f5b3 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/libraries/botbuilder/src/botFrameworkAdapter.ts @@ -6,13 +6,12 @@ * Licensed under the MIT License. */ -import { IncomingMessage, STATUS_CODES } from 'http'; -import { Socket } from 'net'; +import { STATUS_CODES } from 'http'; import * as os from 'os'; import { Activity, ActivityTypes, BotAdapter, BotCallbackHandlerKey, ChannelAccount, ConversationAccount, ConversationParameters, ConversationReference, ConversationsResult, IUserTokenProvider, ResourceResponse, TokenResponse, TurnContext } from 'botbuilder-core'; import { AuthenticationConstants, ChannelValidation, ConnectorClient, EmulatorApiClient, GovernmentConstants, GovernmentChannelValidation, JwtTokenValidation, MicrosoftAppCredentials, SimpleCredentialProvider, TokenApiClient, TokenStatus, TokenApiModels } from 'botframework-connector'; -import { IReceiveRequest, ISocket, IStreamingTransportServer, NamedPipeServer, NodeWebSocketFactory, NodeWebSocketFactoryBase, RequestHandler, StreamingResponse, WebSocketServer } from 'botframework-streaming'; +import { INodeBuffer, INodeSocket, IReceiveRequest, ISocket, IStreamingTransportServer, NamedPipeServer, NodeWebSocketFactory, NodeWebSocketFactoryBase, RequestHandler, StreamingResponse, WebSocketServer } from 'botframework-streaming'; import { StreamingHttpClient, TokenResolver } from './streaming'; @@ -1149,7 +1148,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide * @param res The response sent on error or connection termination. * @param logic The logic that will handle incoming requests. */ - public async useWebSocket(req: IncomingMessage, socket: Socket, head: Buffer, logic: (context: TurnContext) => Promise): Promise { + public async useWebSocket(req: WebRequest, socket: INodeSocket, head: INodeBuffer, logic: (context: TurnContext) => Promise): Promise { if (!this.webSocketFactory || !this.webSocketFactory.createWebSocket) { throw new Error('BotFrameworkAdapter must have a WebSocketFactory in order to support streaming.'); } @@ -1304,7 +1303,7 @@ function delay(timeout: number): Promise { }); } -function abortWebSocketUpgrade(socket: Socket, code: number) { +function abortWebSocketUpgrade(socket: INodeSocket, code: number) { if (socket.writable) { const connectionHeader = `Connection: 'close'\r\n`; socket.write(`HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\n${connectionHeader}\r\n`); diff --git a/libraries/botframework-streaming/src/index.ts b/libraries/botframework-streaming/src/index.ts index 7100601cd4..b81e41f5b6 100644 --- a/libraries/botframework-streaming/src/index.ts +++ b/libraries/botframework-streaming/src/index.ts @@ -9,6 +9,9 @@ export { ContentStream } from './contentStream'; export { HttpContent } from './httpContentStream'; export { + INodeBuffer, + INodeIncomingMessage, + INodeSocket, IReceiveRequest, IReceiveResponse, ISocket, diff --git a/libraries/botframework-streaming/src/interfaces/INodeBuffer.ts b/libraries/botframework-streaming/src/interfaces/INodeBuffer.ts new file mode 100644 index 0000000000..b600e0eff1 --- /dev/null +++ b/libraries/botframework-streaming/src/interfaces/INodeBuffer.ts @@ -0,0 +1,14 @@ +/** + * @module botframework-streaming + */ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Represents a Buffer from the `buffer` module in Node.js. + * + * This interface supports the framework and is not intended to be called directly for your code. + */ +export interface INodeBuffer { } diff --git a/libraries/botframework-streaming/src/interfaces/INodeIncomingMessage.ts b/libraries/botframework-streaming/src/interfaces/INodeIncomingMessage.ts new file mode 100644 index 0000000000..af82eeaf6d --- /dev/null +++ b/libraries/botframework-streaming/src/interfaces/INodeIncomingMessage.ts @@ -0,0 +1,24 @@ +/** + * @module botframework-streaming + */ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Represents a IncomingMessage from the `http` module in Node.js. + * + * This interface supports the framework and is not intended to be called directly for your code. + */ +export interface INodeIncomingMessage { + /*** + * Optional. The request headers. + */ + headers?: any; + + /*** + * Optional. The request method. + */ + method?: any; +} diff --git a/libraries/botframework-streaming/src/interfaces/INodeSocket.ts b/libraries/botframework-streaming/src/interfaces/INodeSocket.ts new file mode 100644 index 0000000000..21ae2b37ae --- /dev/null +++ b/libraries/botframework-streaming/src/interfaces/INodeSocket.ts @@ -0,0 +1,18 @@ +/** + * @module botframework-streaming + */ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Represents a Socket from the `net` module in Node.js. + * + * This interface supports the framework and is not intended to be called directly for your code. + */ +export interface INodeSocket { + writable: boolean; + write(str: string, cb?: Function): boolean; + destroy(error?: Error): void; +} diff --git a/libraries/botframework-streaming/src/interfaces/index.ts b/libraries/botframework-streaming/src/interfaces/index.ts index df6b74c679..6da74891e6 100644 --- a/libraries/botframework-streaming/src/interfaces/index.ts +++ b/libraries/botframework-streaming/src/interfaces/index.ts @@ -5,6 +5,10 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ + +export * from './INodeBuffer'; +export * from './INodeIncomingMessage'; +export * from './INodeSocket'; export * from './IReceiveRequest'; export * from './IReceiveResponse'; export * from './ISocket'; diff --git a/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactory.ts b/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactory.ts index a793943109..7ed3747503 100644 --- a/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactory.ts +++ b/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactory.ts @@ -6,9 +6,7 @@ * Licensed under the MIT License. */ -import { IncomingMessage } from 'http'; -import { Socket } from 'net'; - +import { INodeIncomingMessage, INodeBuffer, INodeSocket } from '../../interfaces'; import { NodeWebSocket } from '../nodeWebSocket'; import { NodeWebSocketFactoryBase } from './nodeWebSocketFactoryBase'; @@ -25,7 +23,7 @@ export class NodeWebSocketFactory extends NodeWebSocketFactoryBase { * @param socket The Socket connecting the bot and the server, from the 'net' module in Node.js. * @param head The first packet of the upgraded stream which may be empty per https://nodejs.org/api/http.html#http_event_upgrade_1. */ - public async createWebSocket(req: IncomingMessage, socket: Socket, head: Buffer): Promise { + public async createWebSocket(req: INodeIncomingMessage, socket: INodeSocket, head: INodeBuffer): Promise { const s = new NodeWebSocket(); await s.create(req, socket, head); diff --git a/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactoryBase.ts b/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactoryBase.ts index e10ea59698..a959e1fca1 100644 --- a/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactoryBase.ts +++ b/libraries/botframework-streaming/src/webSocket/factories/nodeWebSocketFactoryBase.ts @@ -6,10 +6,8 @@ * Licensed under the MIT License. */ -import { IncomingMessage } from 'http'; -import { Socket } from 'net'; -import { ISocket } from '../../interfaces'; +import { INodeIncomingMessage, INodeBuffer, INodeSocket, ISocket } from '../../interfaces'; export abstract class NodeWebSocketFactoryBase { - public abstract createWebSocket(req: IncomingMessage, socket: Socket, head: Buffer): Promise; + public abstract createWebSocket(req: INodeIncomingMessage, socket: INodeSocket, head: INodeBuffer): Promise; } diff --git a/libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts b/libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts index 9ff796b86f..ac1ab5c9a0 100644 --- a/libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts +++ b/libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts @@ -11,7 +11,7 @@ import { IncomingMessage, request } from 'http'; import { Socket } from 'net'; import * as WebSocket from 'ws'; -import { ISocket } from '../interfaces'; +import { INodeIncomingMessage, INodeBuffer, INodeSocket, ISocket } from '../interfaces'; const NONCE_LENGTH = 16; export class NodeWebSocket implements ISocket { @@ -29,15 +29,15 @@ export class NodeWebSocket implements ISocket { /** * Create and set a `ws` WebSocket with an HTTP Request, Socket and Buffer. - * @param req IncomingMessage - * @param socket Socket - * @param head Buffer + * @param req INodeIncomingMessage + * @param socket INodeSocket + * @param head INodeBuffer */ - public async create(req: IncomingMessage, socket: Socket, head: Buffer): Promise { + public async create(req: INodeIncomingMessage, socket: INodeSocket, head: INodeBuffer): Promise { this.wsServer = new WebSocket.Server({ noServer: true }); return new Promise((resolve, reject) => { try { - this.wsServer.handleUpgrade(req, socket, head, (websocket) => { + this.wsServer.handleUpgrade(req as IncomingMessage, socket as Socket, head as Buffer, (websocket) => { this.wsSocket = websocket; resolve(); }); @@ -59,7 +59,7 @@ export class NodeWebSocket implements ISocket { * * @param buffer The buffer of data to send across the connection. */ - public write(buffer: Buffer): void { + public write(buffer: INodeBuffer): void { this.wsSocket.send(buffer); }