fix(server): check for external urls in array (webpack#1980)

* fix: check for external urls in array * test: move tests to contentBase * fix: use is-absolute-url & add test case for number type
knagaitsev · Jul 31, 2019 · 90c17ff · 90c17ff
1 parent a45b61b
commit 90c17ff
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 9 deletions.
diff --git 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');
@@ -356,7 +357,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.'
       );
@@ -408,8 +409,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));
     }
@@ -418,13 +419,13 @@ 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 (isAbsoluteUrl(String(item))) {
+          throw new Error('Watching remote files is not supported.');
+        }
         this._watch(item);
       });
     } else {

diff --git 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
@@ -255,6 +255,56 @@ 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, {
+          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 (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, {
+          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);