From 02cf66de691e18fed43c9c685525fb30222c8933 Mon Sep 17 00:00:00 2001 From: Loonride Date: Fri, 5 Jul 2019 00:12:28 -0500 Subject: [PATCH] feat(server): serverMode 'ws' option (#2082) * feat(server): server mode ws string option * test(server): rearrange bad host test * test(server): added mock server implementation tests * test(server): remove bad host test temporarily * test(server): re added bad host test --- lib/utils/getSocketServerImplementation.js | 5 +- .../serverMode-option.test.js.snap | 78 ++- test/server/serverMode-option.test.js | 647 +++++++++++------- .../getSocketServerImplementation.test.js | 43 +- 4 files changed, 528 insertions(+), 245 deletions(-) diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js index 6df634085f..c60376a9ba 100644 --- a/lib/utils/getSocketServerImplementation.js +++ b/lib/utils/getSocketServerImplementation.js @@ -9,6 +9,9 @@ function getSocketServerImplementation(options) { if (options.serverMode === 'sockjs') { // eslint-disable-next-line global-require ServerImplementation = require('../servers/SockJSServer'); + } else if (options.serverMode === 'ws') { + // eslint-disable-next-line global-require + ServerImplementation = require('../servers/WebsocketServer'); } else { try { // eslint-disable-next-line global-require, import/no-dynamic-require @@ -29,7 +32,7 @@ function getSocketServerImplementation(options) { if (!serverImplFound) { throw new Error( - "serverMode must be a string denoting a default implementation (e.g. 'sockjs'), a full path to " + + "serverMode must be a string denoting a default implementation (e.g. 'sockjs', 'ws'), a full path to " + 'a JS file which exports a class extending BaseServer (webpack-dev-server/lib/servers/BaseServer) ' + 'via require.resolve(...), or the class itself which extends BaseServer' ); diff --git a/test/server/__snapshots__/serverMode-option.test.js.snap b/test/server/__snapshots__/serverMode-option.test.js.snap index ef6ab793a5..01d175c0cc 100644 --- a/test/server/__snapshots__/serverMode-option.test.js.snap +++ b/test/server/__snapshots__/serverMode-option.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`serverMode option with a bad host header results in an error 1`] = ` +exports[`serverMode option passed to server with a bad host header results in an error 1`] = ` Array [ "open", "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", @@ -8,10 +8,84 @@ Array [ ] `; -exports[`serverMode option without a header results in an error 1`] = ` +exports[`serverMode option passed to server without a header results in an error 1`] = ` Array [ "open", "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", "close", ] `; + +exports[`serverMode option server should close client with bad headers 1`] = ` +Array [ + Array [ + [Function], + ], +] +`; + +exports[`serverMode option server should close client with bad headers 2`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", + ], +] +`; + +exports[`serverMode option server should close client with bad headers 3`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + ], +] +`; + +exports[`serverMode option server should use server implementation correctly 1`] = ` +Array [ + Object { + "foo": "bar", + }, +] +`; + +exports[`serverMode option server should use server implementation correctly 2`] = ` +Array [ + Array [ + [Function], + ], +] +`; + +exports[`serverMode option server should use server implementation correctly 3`] = ` +Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"liveReload\\"}", +] +`; + +exports[`serverMode option server should use server implementation correctly 4`] = ` +Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"ok\\"}", +] +`; + +exports[`serverMode option server should use server implementation correctly 5`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + [Function], + ], +] +`; diff --git a/test/server/serverMode-option.test.js b/test/server/serverMode-option.test.js index 1cd24b3685..ba4e924aee 100644 --- a/test/server/serverMode-option.test.js +++ b/test/server/serverMode-option.test.js @@ -8,310 +8,479 @@ const sockjs = require('sockjs'); const SockJS = require('sockjs-client/dist/sockjs'); const SockJSServer = require('../../lib/servers/SockJSServer'); const config = require('../fixtures/simple-config/webpack.config'); -const testServer = require('../helpers/test-server'); const BaseServer = require('../../lib/servers/BaseServer'); const port = require('../ports-map')['serverMode-option']; describe('serverMode option', () => { + let mockedTestServer; + let testServer; let server; let req; + let getSocketServerImplementation; + + const serverModes = [ + { + title: 'as a string ("sockjs")', + serverMode: 'sockjs', + }, + { + title: 'as a path ("sockjs")', + serverMode: require.resolve('../../lib/servers/SockJSServer'), + }, + { + title: 'as a string ("ws")', + serverMode: 'ws', + }, + { + title: 'as a path ("ws")', + serverMode: require.resolve('../../lib/servers/WebsocketServer'), + }, + { + title: 'as a class (custom implementation)', + serverMode: class CustomServer {}, + }, + { + title: 'as a nonexistent path', + serverMode: '/bad/path/to/implementation', + }, + ]; + + describe('is passed to getSocketServerImplementation correctly', () => { + beforeEach(() => { + jest.mock('../../lib/utils/getSocketServerImplementation'); + // eslint-disable-next-line global-require + getSocketServerImplementation = require('../../lib/utils/getSocketServerImplementation'); + getSocketServerImplementation.mockImplementation(() => { + return class MockServer { + // eslint-disable-next-line no-empty-function + onConnection() {} + }; + }); + }); - afterEach((done) => { - testServer.close(done); - req = null; - server = null; - }); + afterEach((done) => { + jest.resetAllMocks(); + jest.resetModules(); - describe('as a string ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: 'sockjs', - port, - }, - done - ); - req = request(`http://localhost:${port}`); + mockedTestServer.close(done); }); - it('sockjs path responds with a 200', async () => { - await req.get('/sockjs-node').expect(200); + serverModes.forEach((data) => { + it(data.title, (done) => { + // eslint-disable-next-line global-require + mockedTestServer = require('../helpers/test-server'); + mockedTestServer.start( + config, + { + serverMode: data.serverMode, + port, + }, + () => { + expect(getSocketServerImplementation.mock.calls.length).toEqual(1); + expect(getSocketServerImplementation.mock.calls[0].length).toEqual( + 1 + ); + expect( + getSocketServerImplementation.mock.calls[0][0].serverMode + ).toEqual(data.serverMode); + done(); + } + ); + }); }); }); - describe('as a path ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: require.resolve('../../lib/servers/SockJSServer'), - port, - }, - done - ); - req = request(`http://localhost:${port}`); + describe('passed to server', () => { + beforeAll(() => { + jest.unmock('../../lib/utils/getSocketServerImplementation'); + // eslint-disable-next-line global-require + testServer = require('../helpers/test-server'); }); - it('sockjs path responds with a 200', async () => { - await req.get('/sockjs-node').expect(200); + afterEach((done) => { + testServer.close(done); + req = null; + server = null; }); - }); - describe('as a class ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: SockJSServer, - port, - }, - done - ); - req = request(`http://localhost:${port}`); + describe('as a string ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + serverMode: 'sockjs', + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); }); - it('sockjs path responds with a 200', async () => { - await req.get('/sockjs-node').expect(200); + describe('as a path ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + serverMode: require.resolve('../../lib/servers/SockJSServer'), + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); }); - }); - describe('as a class (custom "sockjs" implementation)', () => { - it('uses supplied server implementation', (done) => { + describe('as a class ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + serverMode: SockJSServer, + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); + }); + + describe('as a class (custom "sockjs" implementation)', () => { let sockPath; - server = testServer.start( - config, - { - port, - sockPath: '/foo/test/bar/', - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - - sockPath = server.options.sockPath; - } + it('uses supplied server implementation', (done) => { + server = testServer.start( + config, + { + port, + sockPath: '/foo/test/bar/', + serverMode: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, + }); - send(connection, message) { - connection.write(message); - } + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); - close(connection) { - connection.close(); - } + sockPath = server.options.sockPath; + } - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection, connection.headers); - }); - } + send(connection, message) { + connection.write(message); + } - onConnectionClose(connection, f) { - connection.on('close', f); - } + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection, connection.headers); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, }, - }, - () => { - expect(sockPath).toEqual('/foo/test/bar/'); - done(); - } - ); + () => { + expect(sockPath).toEqual('/foo/test/bar/'); + done(); + } + ); + }); + }); + + describe('as a path with nonexistent path', () => { + it('should throw an error', () => { + expect(() => { + server = testServer.start( + config, + { + serverMode: '/bad/path/to/implementation', + port, + }, + () => {} + ); + }).toThrow(/serverMode must be a string/); + }); }); - }); - describe('as a path with nonexistent path', () => { - it('should throw an error', () => { - expect(() => { + describe('without a header', () => { + let mockWarn; + beforeAll((done) => { server = testServer.start( config, { - serverMode: '/bad/path/to/implementation', port, + serverMode: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, + }); + + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); + } + + send(connection, message) { + connection.write(message); + } + + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, }, - () => {} + done ); - }).toThrow(/serverMode must be a string/); - }); - }); - describe('with a bad host header', () => { - beforeAll((done) => { - server = testServer.start( - config, - { - port, - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - } + mockWarn = jest.spyOn(server.log, 'warn').mockImplementation(() => {}); + }); - send(connection, message) { - connection.write(message); - } + it('results in an error', (done) => { + const data = []; + const client = new SockJS(`http://localhost:${port}/sockjs-node`); + + client.onopen = () => { + data.push('open'); + }; + + client.onmessage = (e) => { + data.push(e.data); + }; + + client.onclose = () => { + data.push('close'); + }; - close(connection) { - connection.close(); + setTimeout(() => { + expect(data).toMatchSnapshot(); + const calls = mockWarn.mock.calls; + mockWarn.mockRestore(); + + let foundWarning = false; + const regExp = /serverMode implementation must pass headers to the callback of onConnection\(f\)/; + calls.every((call) => { + if (regExp.test(call)) { + foundWarning = true; + return false; } + return true; + }); + + expect(foundWarning).toBeTruthy(); + + done(); + }, 5000); + }); + }); - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection, { - host: null, + describe('with a bad host header', () => { + beforeAll((done) => { + server = testServer.start( + config, + { + port, + serverMode: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, }); - }); - } - onConnectionClose(connection, f) { - connection.on('close', f); - } + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); + } + + send(connection, message) { + connection.write(message); + } + + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection, { + host: null, + }); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, }, - }, - done - ); - }); + done + ); + }); - it('results in an error', (done) => { - const data = []; - const client = new SockJS(`http://localhost:${port}/sockjs-node`); + it('results in an error', (done) => { + const data = []; + const client = new SockJS(`http://localhost:${port}/sockjs-node`); - client.onopen = () => { - data.push('open'); - }; + client.onopen = () => { + data.push('open'); + }; - client.onmessage = (e) => { - data.push(e.data); - }; + client.onmessage = (e) => { + data.push(e.data); + }; - client.onclose = () => { - data.push('close'); - }; + client.onclose = () => { + data.push('close'); + }; - setTimeout(() => { - expect(data).toMatchSnapshot(); - done(); - }, 5000); + setTimeout(() => { + expect(data).toMatchSnapshot(); + done(); + }, 5000); + }); }); }); - describe('without a header', () => { - let mockWarn; - beforeAll((done) => { - server = testServer.start( + describe('server', () => { + let MockSockJSServer; + beforeEach((done) => { + jest.mock('../../lib/servers/SockJSServer'); + // eslint-disable-next-line global-require + mockedTestServer = require('../helpers/test-server'); + // eslint-disable-next-line global-require + MockSockJSServer = require('../../lib/servers/SockJSServer'); + + server = mockedTestServer.start( config, { port, - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - } - - send(connection, message) { - connection.write(message); - } - - close(connection) { - connection.close(); - } - - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection); - }); - } - - onConnectionClose(connection, f) { - connection.on('close', f); - } - }, }, done ); - - mockWarn = jest.spyOn(server.log, 'warn').mockImplementation(() => {}); }); - it('results in an error', (done) => { - const data = []; - const client = new SockJS(`http://localhost:${port}/sockjs-node`); + afterEach((done) => { + mockedTestServer.close(done); + jest.resetAllMocks(); + jest.resetModules(); - client.onopen = () => { - data.push('open'); - }; + server = null; + }); - client.onmessage = (e) => { - data.push(e.data); - }; + it('should use server implementation correctly', () => { + const mockServerInstance = MockSockJSServer.mock.instances[0]; - client.onclose = () => { - data.push('close'); + const connectionObj = { + foo: 'bar', }; + // this simulates a client connecting to the server + mockServerInstance.onConnection.mock.calls[0][0](connectionObj, { + host: `localhost:${port}`, + origin: `http://localhost:${port}`, + }); + + expect(server.sockets.length).toEqual(1); + expect(server.sockets).toMatchSnapshot(); + + // this simulates a client leaving the server + mockServerInstance.onConnectionClose.mock.calls[0][1](connectionObj); + + expect(server.sockets.length).toEqual(0); + + // check that the dev server was passed to the socket server implementation constructor + expect(MockSockJSServer.mock.calls[0].length).toEqual(1); + expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); + + expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); + expect(mockServerInstance.send.mock.calls.length).toEqual(3); + // call 0 to the send() method is liveReload + expect(mockServerInstance.send.mock.calls[0]).toMatchSnapshot(); + // call 1 to the send() method is hash data, so we skip it + // call 2 to the send() method is the "ok" message + expect(mockServerInstance.send.mock.calls[2]).toMatchSnapshot(); + // close should not be called because the server never forcefully closes + // a successful client connection + expect(mockServerInstance.close.mock.calls.length).toEqual(0); + expect(mockServerInstance.onConnectionClose.mock.calls).toMatchSnapshot(); + }); - setTimeout(() => { - expect(data).toMatchSnapshot(); - const calls = mockWarn.mock.calls; - mockWarn.mockRestore(); - - let foundWarning = false; - const regExp = /serverMode implementation must pass headers to the callback of onConnection\(f\)/; - calls.every((call) => { - if (regExp.test(call)) { - foundWarning = true; - return false; - } - return true; - }); - - expect(foundWarning).toBeTruthy(); + it('should close client with bad headers', () => { + const mockServerInstance = MockSockJSServer.mock.instances[0]; - done(); - }, 5000); + // this simulates a client connecting to the server + mockServerInstance.onConnection.mock.calls[0][0]( + { + foo: 'bar', + }, + { + host: null, + } + ); + expect(server.sockets.length).toEqual(0); + expect(MockSockJSServer.mock.calls[0].length).toEqual(1); + expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); + expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); + // the only call to send() here should be an invalid header message + expect(mockServerInstance.send.mock.calls).toMatchSnapshot(); + expect(mockServerInstance.close.mock.calls).toMatchSnapshot(); + // onConnectionClose should never get called since the client should be closed first + expect(mockServerInstance.onConnectionClose.mock.calls.length).toEqual(0); }); }); }); diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 0a157c39a9..1898b8addf 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -2,9 +2,10 @@ const getSocketServerImplementation = require('../../../lib/utils/getSocketServerImplementation'); const SockJSServer = require('../../../lib/servers/SockJSServer'); +const WebsocketServer = require('../../../lib/servers/WebsocketServer'); describe('getSocketServerImplementation util', () => { - it("should works with string serverMode ('sockjs')", () => { + it("should work with string serverMode ('sockjs')", () => { let result; expect(() => { @@ -16,7 +17,7 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); - it('should works with serverMode (SockJSServer class)', () => { + it('should work with serverMode (SockJSServer class)', () => { let result; expect(() => { @@ -40,7 +41,43 @@ describe('getSocketServerImplementation util', () => { expect(result).toEqual(SockJSServer); }); - it('should throws with serverMode (bad path)', () => { + it("should work with string serverMode ('ws')", () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: 'ws', + }); + }).not.toThrow(); + + expect(result).toEqual(WebsocketServer); + }); + + it('should work with serverMode (WebsocketServer class)', () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: WebsocketServer, + }); + }).not.toThrow(); + + expect(result).toEqual(WebsocketServer); + }); + + it('should work with serverMode (WebsocketServer full path)', () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: require.resolve('../../../lib/servers/WebsocketServer'), + }); + }).not.toThrow(); + + expect(result).toEqual(WebsocketServer); + }); + + it('should throw with serverMode (bad path)', () => { expect(() => { getSocketServerImplementation({ serverMode: '/bad/path/to/implementation',