diff --git a/bin/webpack-dev-server.js b/bin/webpack-dev-server.js index 286cad93b9..224a69d978 100755 --- a/bin/webpack-dev-server.js +++ b/bin/webpack-dev-server.js @@ -4,8 +4,6 @@ /* eslint-disable no-shadow, no-console */ -const fs = require('fs'); -const net = require('net'); const debug = require('debug')('webpack-dev-server'); const importLocal = require('import-local'); const yargs = require('yargs'); @@ -116,42 +114,10 @@ function startDevServer(config, options) { } if (options.socket) { - server.listeningApp.on('error', (e) => { - if (e.code === 'EADDRINUSE') { - const clientSocket = new net.Socket(); - - clientSocket.on('error', (err) => { - if (err.code === 'ECONNREFUSED') { - // No other server listening on this socket so it can be safely removed - fs.unlinkSync(options.socket); - - server.listen(options.socket, options.host, (error) => { - if (error) { - throw error; - } - }); - } - }); - - clientSocket.connect({ path: options.socket }, () => { - throw new Error('This socket is already used'); - }); - } - }); - server.listen(options.socket, options.host, (err) => { if (err) { throw err; } - - // chmod 666 (rw rw rw) - const READ_WRITE = 438; - - fs.chmod(options.socket, READ_WRITE, (err) => { - if (err) { - throw err; - } - }); }); } else { server.listen(options.port, options.host, (err) => { diff --git a/lib/Server.js b/lib/Server.js index 59b55f4153..7864b6d256 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -34,6 +34,8 @@ const runBonjour = require('./utils/runBonjour'); const routes = require('./utils/routes'); const getSocketServerImplementation = require('./utils/getSocketServerImplementation'); const handleStdin = require('./utils/handleStdin'); +const tryParseInt = require('./utils/tryParseInt'); +const startUnixSocket = require('./utils/startUnixSocket'); const schema = require('./options.json'); // Workaround for node ^8.6.0, ^9.0.0 @@ -742,7 +744,7 @@ class Server { listen(port, hostname, fn) { this.hostname = hostname; - return this.listeningApp.listen(port, hostname, (err) => { + const setupCallback = () => { this.createSocketServer(); if (this.options.bonjour) { @@ -750,15 +752,46 @@ class Server { } this.showStatus(); + }; - if (fn) { + // between setupCallback and userCallback should be any other needed handling, + // specifically so that things are done in the right order to prevent + // backwards compatability issues + let userCallbackCalled = false; + const userCallback = (err) => { + if (fn && !userCallbackCalled) { + userCallbackCalled = true; fn.call(this.listeningApp, err); } + }; + const onListeningCallback = () => { if (typeof this.options.onListening === 'function') { this.options.onListening(this); } - }); + }; + + const fullCallback = (err) => { + setupCallback(); + userCallback(err); + onListeningCallback(); + }; + + // try to follow the Node standard in terms of deciding + // whether this is a socket or a port that we will listen on: + // https://github.com/nodejs/node/blob/64219741218aa87e259cf8257596073b8e747f0a/lib/net.js#L196 + if (typeof port === 'string' && tryParseInt(port) === null) { + // in this case the "port" argument is actually a socket path + const socket = port; + // set this so that status helper can identify how the project is being run correctly + this.options.socket = socket; + + startUnixSocket(this.listeningApp, socket, fullCallback); + } else { + this.listeningApp.listen(port, hostname, fullCallback); + } + + return this.listeningApp; } close(cb) { diff --git a/lib/utils/startUnixSocket.js b/lib/utils/startUnixSocket.js new file mode 100644 index 0000000000..9bf850c2ec --- /dev/null +++ b/lib/utils/startUnixSocket.js @@ -0,0 +1,62 @@ +'use strict'; + +const fs = require('fs'); +const net = require('net'); +const { promisify } = require('util'); + +const accessAsync = promisify(fs.access); + +async function startUnixSocket(listeningApp, socket, cb) { + const chmodSocket = (done) => { + // chmod 666 (rw rw rw) - octal + const READ_WRITE = 438; + fs.chmod(socket, READ_WRITE, done); + }; + + const startSocket = () => { + listeningApp.on('error', (err) => { + cb(err); + }); + + // 511 is the default value for the server.listen backlog parameter + // https://nodejs.org/api/net.html#net_server_listen + listeningApp.listen(socket, 511, (err) => { + if (err) { + cb(err); + } else { + chmodSocket(cb); + } + }); + }; + + try { + await accessAsync(socket, fs.constants.F_OK); + } catch (e) { + // file does not exist + startSocket(); + return; + } + + // file exists + + const clientSocket = new net.Socket(); + + clientSocket.on('error', (err) => { + if (err.code === 'ECONNREFUSED' || err.code === 'ENOTSOCK') { + // No other server listening on this socket so it can be safely removed + fs.unlinkSync(socket); + + startSocket(); + } + }); + + clientSocket.connect({ path: socket }, () => { + // if a client socket successfully connects to the given socket path, + // it means that the socket is in use + const err = new Error('This socket is already used'); + clientSocket.destroy(); + cb(err); + }); +} + +module.exports = startUnixSocket; diff --git a/test/cli/cli.test.js b/test/cli/cli.test.js index 27de1fe416..27c511d3bd 100644 --- a/test/cli/cli.test.js +++ b/test/cli/cli.test.js @@ -10,6 +10,7 @@ const { unlinkAsync } = require('../helpers/fs'); const testBin = require('../helpers/test-bin'); const port1 = require('../ports-map').cli[0]; const timer = require('../helpers/timer'); +const { skipTestOnWindows } = require('../helpers/conditional-test'); const httpsCertificateDirectory = resolve( __dirname, @@ -94,14 +95,14 @@ describe('CLI', () => { .catch(done); }); - // The Unix socket to listen to (instead of a host). - it('--socket', async () => { - const socketPath = join('.', 'webpack.sock'); - const { exitCode, stdout } = await testBin(`--socket ${socketPath}`); - expect(exitCode).toEqual(0); + describe('Unix socket', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } - if (process.platform !== 'win32') { - expect(stdout.includes(socketPath)).toBe(true); + // The Unix socket to listen to (instead of a host). + it('--socket', async () => { + const socketPath = join('.', 'webpack.sock'); testBin(`--socket ${socketPath}`) .then((output) => { diff --git a/test/helpers/test-server.js b/test/helpers/test-server.js index f9284c4fca..85de3777f7 100644 --- a/test/helpers/test-server.js +++ b/test/helpers/test-server.js @@ -40,9 +40,19 @@ function startFullSetup(config, options, done) { server = new Server(compiler, options); - const port = Object.prototype.hasOwnProperty.call(options, 'port') - ? options.port - : 8080; + // originally the fallback default was 8080, but it should be + // undefined so that the server.listen method can choose it for us, + // and thus prevent port mapping collision between tests + let port; + if (Object.prototype.hasOwnProperty.call(options, 'port')) { + port = options.port; + } else if (Object.prototype.hasOwnProperty.call(options, 'socket')) { + port = options.socket; + } else { + // TODO: remove this when findPort is implemented in the server.listen method + port = 8080; + } + const host = Object.prototype.hasOwnProperty.call(options, 'host') ? options.host : 'localhost'; @@ -65,10 +75,12 @@ function startFullSetup(config, options, done) { function startAwaitingCompilationFullSetup(config, options, done) { let readyCount = 0; - const ready = () => { + let err; + const ready = (e) => { + err = e instanceof Error || (typeof e === 'object' && e.code) ? e : err; readyCount += 1; if (readyCount === 2) { - done(); + done(err); } }; diff --git a/test/helpers/test-unix-socket.js b/test/helpers/test-unix-socket.js new file mode 100644 index 0000000000..d2c504b62f --- /dev/null +++ b/test/helpers/test-unix-socket.js @@ -0,0 +1,30 @@ +'use strict'; + +const http = require('http'); + +const TestUnixSocket = class TestUnixSocket { + constructor() { + this.server = http.createServer(); + this.sockets = new Set(); + this.server.on('connection', (socket) => { + this.sockets.add(socket); + socket.on('close', () => { + this.sockets.delete(socket); + }); + }); + } + + close(done) { + if (this.server.listening) { + // get rid of connected sockets + for (const socket of this.sockets.values()) { + socket.destroy(); + } + this.server.close(done); + } else { + done(); + } + } +}; + +module.exports = TestUnixSocket; diff --git a/test/server/onListening-option.test.js b/test/server/onListening-option.test.js index d6d66b1565..2d8b86dc3e 100644 --- a/test/server/onListening-option.test.js +++ b/test/server/onListening-option.test.js @@ -1,32 +1,71 @@ 'use strict'; +const { unlink } = require('fs'); +const { join } = require('path'); const testServer = require('../helpers/test-server'); const config = require('../fixtures/simple-config/webpack.config'); const port = require('../ports-map')['onListening-option']; +const { skipTestOnWindows } = require('../helpers/conditional-test'); describe('onListening option', () => { - let onListeningIsRunning = false; - - beforeAll((done) => { - testServer.start( - config, - { - onListening: (devServer) => { - if (!devServer) { - throw new Error('webpack-dev-server is not defined'); - } - - onListeningIsRunning = true; + describe('with host and port', () => { + let onListeningIsRunning = false; + + beforeAll((done) => { + testServer.start( + config, + { + onListening: (devServer) => { + if (!devServer) { + throw new Error('webpack-dev-server is not defined'); + } + + onListeningIsRunning = true; + }, + port, }, - port, - }, - done - ); + done + ); + }); + + afterAll(testServer.close); + + it('should run onListening callback', () => { + expect(onListeningIsRunning).toBe(true); + }); }); - afterAll(testServer.close); + describe('with Unix socket', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } + + const socketPath = join('.', 'onListening.webpack.sock'); + let onListeningIsRunning = false; + + beforeAll((done) => { + testServer.start( + config, + { + onListening: (devServer) => { + if (!devServer) { + throw new Error('webpack-dev-server is not defined'); + } + + onListeningIsRunning = true; + }, + socket: socketPath, + }, + done + ); + }); + + afterAll(testServer.close); + + it('should run onListening callback', (done) => { + expect(onListeningIsRunning).toBe(true); - it('should runs onListening callback', () => { - expect(onListeningIsRunning).toBe(true); + unlink(socketPath, done); + }); }); }); diff --git a/test/server/socket-option.test.js b/test/server/socket-option.test.js new file mode 100644 index 0000000000..a933097bb1 --- /dev/null +++ b/test/server/socket-option.test.js @@ -0,0 +1,149 @@ +'use strict'; + +const net = require('net'); +const fs = require('fs'); +const path = require('path'); +const webpack = require('webpack'); +const testServer = require('../helpers/test-server'); +const TestUnixSocket = require('../helpers/test-unix-socket'); +const { skipTestOnWindows } = require('../helpers/conditional-test'); +const config = require('../fixtures/simple-config/webpack.config'); +const Server = require('../../lib/Server'); + +describe('socket', () => { + const socketPath = path.join('.', 'socket-option.webpack.sock'); + let server = null; + + describe('path to a non-existent file', () => { + let err; + beforeAll((done) => { + server = testServer.start( + config, + { + socket: socketPath, + }, + (e) => { + err = e; + done(); + } + ); + }); + + it('should work as Unix socket or error on windows', (done) => { + if (process.platform === 'win32') { + expect(err.code).toEqual('EACCES'); + done(); + } else { + const clientSocket = new net.Socket(); + clientSocket.connect({ path: socketPath }, () => { + // this means the connection was made successfully + expect(true).toBeTruthy(); + done(); + }); + } + }); + + afterAll((done) => { + testServer.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent, unused file', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } + + beforeAll((done) => { + fs.writeFileSync(socketPath, ''); + server = testServer.start( + config, + { + socket: socketPath, + }, + done + ); + }); + + it('should work as Unix socket', (done) => { + const clientSocket = new net.Socket(); + clientSocket.connect({ path: socketPath }, () => { + // this means the connection was made successfully + expect(true).toBeTruthy(); + done(); + }); + }); + + afterAll((done) => { + testServer.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent file with listening server', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } + + let testUnixSocket; + beforeAll((done) => { + testUnixSocket = new TestUnixSocket(); + testUnixSocket.server.listen(socketPath, 511, () => { + done(); + }); + }); + + it('should throw already used error', (done) => { + server = testServer.start( + config, + { + socket: socketPath, + }, + (err) => { + expect(err).not.toBeNull(); + expect(err).not.toBeUndefined(); + expect(err.message).toEqual('This socket is already used'); + server.close(done); + } + ); + }); + + afterAll((done) => { + testUnixSocket.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent, unused file', () => { + let devServer; + const options = { + socket: socketPath, + }; + beforeAll(() => { + fs.writeFileSync(socketPath, ''); + }); + + // this test is significant because previously the callback was called + // twice if a file at the given socket path already existed, but + // could be removed + it('should only call server.listen callback once', (done) => { + const compiler = webpack(config); + + devServer = new Server(compiler, options); + const onListen = jest.fn(); + // eslint-disable-next-line no-undefined + devServer.listen(options.socket, undefined, onListen); + setTimeout(() => { + expect(onListen).toBeCalledTimes(1); + done(); + }, 10000); + }); + + afterAll((done) => { + devServer.close(done); + }); + }); +}); diff --git a/test/server/utils/startUnixSocket.test.js b/test/server/utils/startUnixSocket.test.js new file mode 100644 index 0000000000..e0ab7b899a --- /dev/null +++ b/test/server/utils/startUnixSocket.test.js @@ -0,0 +1,124 @@ +'use strict'; + +const net = require('net'); +const fs = require('fs'); +const path = require('path'); +const TestUnixSocket = require('../../helpers/test-unix-socket'); +const { skipTestOnWindows } = require('../../helpers/conditional-test'); +const startUnixSocket = require('../../../lib/utils/startUnixSocket'); + +describe('startUnixSocket', () => { + const socketPath = path.join('.', 'startUnixSocket.webpack.sock'); + let testUnixSocket = null; + + describe('path to a non-existent file', () => { + let err; + beforeAll((done) => { + testUnixSocket = new TestUnixSocket(); + startUnixSocket(testUnixSocket.server, socketPath, (e) => { + err = e; + done(); + }); + }); + + it('should work as Unix socket or error on windows', (done) => { + if (process.platform === 'win32') { + expect(err.code).toEqual('EACCES'); + done(); + } else { + const clientSocket = new net.Socket(); + clientSocket.connect({ path: socketPath }, () => { + // this means the connection was made successfully + expect(true).toBeTruthy(); + done(); + }); + } + }); + + afterAll((done) => { + testUnixSocket.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent, unused file', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } + + beforeAll((done) => { + fs.writeFileSync(socketPath, ''); + testUnixSocket = new TestUnixSocket(); + startUnixSocket(testUnixSocket.server, socketPath, done); + }); + + it('should work as Unix socket', (done) => { + const clientSocket = new net.Socket(); + clientSocket.connect({ path: socketPath }, () => { + // this means the connection was made successfully + expect(true).toBeTruthy(); + done(); + }); + }); + + afterAll((done) => { + testUnixSocket.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent file with listening server', () => { + if (skipTestOnWindows('Unix sockets are not supported on Windows')) { + return; + } + + let dummyUnixSocket; + beforeAll((done) => { + dummyUnixSocket = new TestUnixSocket(); + dummyUnixSocket.server.listen(socketPath, 511, () => { + done(); + }); + }); + + it('should throw already used error', (done) => { + testUnixSocket = new TestUnixSocket(); + startUnixSocket(testUnixSocket.server, socketPath, (err) => { + expect(err).not.toBeNull(); + expect(err).not.toBeUndefined(); + expect(err.message).toEqual('This socket is already used'); + testUnixSocket.close(done); + }); + }); + + afterAll((done) => { + dummyUnixSocket.close(() => { + fs.unlink(socketPath, done); + }); + }); + }); + + describe('path to existent, unused file', () => { + beforeAll(() => { + fs.writeFileSync(socketPath, ''); + testUnixSocket = new TestUnixSocket(); + }); + + // this test is significant because previously the callback was called + // twice if a file at the given socket path already existed, but + // could be removed + it('should only call server.listen callback once', (done) => { + const userCallback = jest.fn(); + startUnixSocket(testUnixSocket.server, socketPath, userCallback); + setTimeout(() => { + expect(userCallback).toBeCalledTimes(1); + done(); + }, 3000); + }); + + afterAll((done) => { + testUnixSocket.close(done); + }); + }); +});