From 668cd3fa76ee1b3a6596b8a4a2ab1109f2b40e38 Mon Sep 17 00:00:00 2001 From: Eslam El-Hakmey Date: Thu, 6 Jun 2019 13:44:32 +0200 Subject: [PATCH 1/3] fix: check for external urls in array --- lib/Server.js | 3 ++ test/ExternalPathsWatch.test.js | 91 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 test/ExternalPathsWatch.test.js diff --git a/lib/Server.js b/lib/Server.js index 1c58eaea43..9f5c49d611 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -416,6 +416,9 @@ class Server { throw new Error('Watching remote files is not supported.'); } else if (Array.isArray(contentBase)) { contentBase.forEach((item) => { + if (/^(https?:)?\/\//.test(item)) { + throw new Error('Watching remote files is not supported.'); + } this._watch(item); }); } else { diff --git a/test/ExternalPathsWatch.test.js b/test/ExternalPathsWatch.test.js new file mode 100644 index 0000000000..aebab79a35 --- /dev/null +++ b/test/ExternalPathsWatch.test.js @@ -0,0 +1,91 @@ +'use strict'; + +/* eslint-disable + no-unused-vars +*/ +const path = require('path'); +const webpack = require('webpack'); +const Server = require('../lib/Server'); +const config = require('./fixtures/simple-config/webpack.config'); + +const contentBasePublic = path.join( + __dirname, + 'fixtures/contentbase-config/public' +); +const contentBaseOther = path.join( + __dirname, + 'fixtures/contentbase-config/other' +); +const compiler = webpack(config); + +describe('Watching external files', () => { + let server; + + describe('testing single & multiple external paths', () => { + it('Should throw exception (single line)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = new Server(compiler, { + contentBase: 'https://example.com/', + watchContentBase: true, + }); + + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Watching remote files is not supported.'); + compiler.run(() => { + done(); + }); + } + }); + it('Should throw exception (array)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = new Server(compiler, { + contentBase: [contentBasePublic, 'https://example.com/'], + watchContentBase: true, + }); + + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Watching remote files is not supported.'); + compiler.run(() => { + done(); + }); + } + }); + }); + + describe('testing single & multiple internal paths', () => { + it('Should not throw exception (single line)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = new Server(compiler, { + contentBase: contentBasePublic, + watchContentBase: true, + }); + + compiler.run(() => { + done(); + }); + } catch (e) { + expect(true).toBe(false); + } + }); + it('Should not throw exception (array)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = new Server(compiler, { + contentBase: [contentBasePublic, contentBaseOther], + watchContentBase: true, + }); + + compiler.run(() => { + done(); + }); + } catch (e) { + expect(true).toBe(false); + } + }); + }); +}); From 6fccea4b07f5091529d6dacbce000b552c356a28 Mon Sep 17 00:00:00 2001 From: Eslam El-Hakmey Date: Thu, 6 Jun 2019 14:49:56 +0200 Subject: [PATCH 2/3] test: move tests to contentBase --- test/ExternalPathsWatch.test.js | 91 -------------------------- test/server/contentBase-option.test.js | 38 +++++++++++ 2 files changed, 38 insertions(+), 91 deletions(-) delete mode 100644 test/ExternalPathsWatch.test.js diff --git a/test/ExternalPathsWatch.test.js b/test/ExternalPathsWatch.test.js deleted file mode 100644 index aebab79a35..0000000000 --- a/test/ExternalPathsWatch.test.js +++ /dev/null @@ -1,91 +0,0 @@ -'use strict'; - -/* eslint-disable - no-unused-vars -*/ -const path = require('path'); -const webpack = require('webpack'); -const Server = require('../lib/Server'); -const config = require('./fixtures/simple-config/webpack.config'); - -const contentBasePublic = path.join( - __dirname, - 'fixtures/contentbase-config/public' -); -const contentBaseOther = path.join( - __dirname, - 'fixtures/contentbase-config/other' -); -const compiler = webpack(config); - -describe('Watching external files', () => { - let server; - - describe('testing single & multiple external paths', () => { - it('Should throw exception (single line)', (done) => { - try { - // eslint-disable-next-line no-unused-vars - server = new Server(compiler, { - contentBase: 'https://example.com/', - watchContentBase: true, - }); - - expect(true).toBe(false); - } catch (e) { - expect(e.message).toBe('Watching remote files is not supported.'); - compiler.run(() => { - done(); - }); - } - }); - it('Should throw exception (array)', (done) => { - try { - // eslint-disable-next-line no-unused-vars - server = new Server(compiler, { - contentBase: [contentBasePublic, 'https://example.com/'], - watchContentBase: true, - }); - - expect(true).toBe(false); - } catch (e) { - expect(e.message).toBe('Watching remote files is not supported.'); - compiler.run(() => { - done(); - }); - } - }); - }); - - describe('testing single & multiple internal paths', () => { - it('Should not throw exception (single line)', (done) => { - try { - // eslint-disable-next-line no-unused-vars - server = new Server(compiler, { - contentBase: contentBasePublic, - watchContentBase: true, - }); - - compiler.run(() => { - done(); - }); - } catch (e) { - expect(true).toBe(false); - } - }); - it('Should not throw exception (array)', (done) => { - try { - // eslint-disable-next-line no-unused-vars - server = new Server(compiler, { - contentBase: [contentBasePublic, contentBaseOther], - watchContentBase: true, - }); - - compiler.run(() => { - done(); - }); - } catch (e) { - expect(true).toBe(false); - } - }); - }); -}); diff --git a/test/server/contentBase-option.test.js b/test/server/contentBase-option.test.js index baa4111124..3582251987 100644 --- a/test/server/contentBase-option.test.js +++ b/test/server/contentBase-option.test.js @@ -267,6 +267,44 @@ describe('contentBase option', () => { }); }); + describe('testing single & multiple external paths', () => { + afterAll((done) => { + testServer.close(() => { + done(); + }); + }); + it('Should throw exception (string)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = testServer.start(config, { + port: 9004, + contentBase: 'https://example.com/', + watchContentBase: true, + }); + + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Watching remote files is not supported.'); + done(); + } + }); + it('Should throw exception (array)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = testServer.start(config, { + port: 9003, + contentBase: [contentBasePublic, 'https://example.com/'], + watchContentBase: true, + }); + + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Watching remote files is not supported.'); + done(); + } + }); + }); + describe('default to PWD', () => { beforeAll((done) => { jest.spyOn(process, 'cwd').mockImplementation(() => contentBasePublic); From 2ac39d38c70c006848a984c59b306f32e0a657b8 Mon Sep 17 00:00:00 2001 From: eslam Date: Wed, 26 Jun 2019 15:03:08 +0200 Subject: [PATCH 3/3] fix: use is-absolute-url & add test case for number type --- lib/Server.js | 14 ++++++-------- lib/utils/createConfig.js | 5 +++-- test/server/contentBase-option.test.js | 16 ++++++++++++++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 9f5c49d611..0935b4e9b9 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -23,6 +23,7 @@ const serveIndex = require('serve-index'); const webpack = require('webpack'); const webpackDevMiddleware = require('webpack-dev-middleware'); const validateOptions = require('schema-utils'); +const isAbsoluteUrl = require('is-absolute-url'); const updateCompiler = require('./utils/updateCompiler'); const createLogger = require('./utils/createLogger'); const getCertificate = require('./utils/getCertificate'); @@ -347,7 +348,7 @@ class Server { contentBase.forEach((item) => { this.app.get('*', express.static(item)); }); - } else if (/^(https?:)?\/\//.test(contentBase)) { + } else if (isAbsoluteUrl(String(contentBase))) { this.log.warn( 'Using a URL as contentBase is deprecated and will be removed in the next major version. Please use the proxy option instead.' ); @@ -399,8 +400,8 @@ class Server { this.app.get('*', serveIndex(item)); }); } else if ( - !/^(https?:)?\/\//.test(contentBase) && - typeof contentBase !== 'number' + typeof contentBase !== 'number' && + !isAbsoluteUrl(String(contentBase)) ) { this.app.get('*', serveIndex(contentBase)); } @@ -409,14 +410,11 @@ class Server { setupWatchStaticFeature() { const contentBase = this.options.contentBase; - if ( - /^(https?:)?\/\//.test(contentBase) || - typeof contentBase === 'number' - ) { + if (isAbsoluteUrl(String(contentBase)) || typeof contentBase === 'number') { throw new Error('Watching remote files is not supported.'); } else if (Array.isArray(contentBase)) { contentBase.forEach((item) => { - if (/^(https?:)?\/\//.test(item)) { + if (isAbsoluteUrl(String(item))) { throw new Error('Watching remote files is not supported.'); } this._watch(item); diff --git a/lib/utils/createConfig.js b/lib/utils/createConfig.js index c16a95138b..1902466620 100644 --- a/lib/utils/createConfig.js +++ b/lib/utils/createConfig.js @@ -1,6 +1,7 @@ 'use strict'; const path = require('path'); +const isAbsoluteUrl = require('is-absolute-url'); const defaultTo = require('./defaultTo'); function createConfig(config, argv, { port }) { @@ -60,7 +61,7 @@ function createConfig(config, argv, { port }) { (firstWpOpt.output && firstWpOpt.output.publicPath) || ''; if ( - !/^(https?:)?\/\//.test(options.publicPath) && + !isAbsoluteUrl(String(options.publicPath)) && options.publicPath[0] !== '/' ) { options.publicPath = `/${options.publicPath}`; @@ -109,7 +110,7 @@ function createConfig(config, argv, { port }) { options.contentBase = options.contentBase.map((p) => path.resolve(p)); } else if (/^[0-9]$/.test(options.contentBase)) { options.contentBase = +options.contentBase; - } else if (!/^(https?:)?\/\//.test(options.contentBase)) { + } else if (!isAbsoluteUrl(String(options.contentBase))) { options.contentBase = path.resolve(options.contentBase); } } diff --git a/test/server/contentBase-option.test.js b/test/server/contentBase-option.test.js index 3582251987..3d6ab7de58 100644 --- a/test/server/contentBase-option.test.js +++ b/test/server/contentBase-option.test.js @@ -277,7 +277,6 @@ describe('contentBase option', () => { try { // eslint-disable-next-line no-unused-vars server = testServer.start(config, { - port: 9004, contentBase: 'https://example.com/', watchContentBase: true, }); @@ -288,11 +287,24 @@ describe('contentBase option', () => { done(); } }); + it('Should throw exception (number)', (done) => { + try { + // eslint-disable-next-line no-unused-vars + server = testServer.start(config, { + contentBase: 2, + watchContentBase: true, + }); + + expect(true).toBe(false); + } catch (e) { + expect(e.message).toBe('Watching remote files is not supported.'); + done(); + } + }); it('Should throw exception (array)', (done) => { try { // eslint-disable-next-line no-unused-vars server = testServer.start(config, { - port: 9003, contentBase: [contentBasePublic, 'https://example.com/'], watchContentBase: true, });