From 5a1835dc3619cd801128d733b446ce84fa1dc127 Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 05:02:57 +0800 Subject: [PATCH 1/7] fix: move resource query parsing to overlay entry to ensure its success --- client/ErrorOverlayEntry.js | 13 +++++++------ client/{ => utils}/errorEventHandlers.js | 0 client/{ => utils}/formatWebpackErrors.js | 0 .../utils/parseResourceQuery.js | 14 ++++---------- sockets/WDSSocket.js | 11 +++++------ 5 files changed, 16 insertions(+), 22 deletions(-) rename client/{ => utils}/errorEventHandlers.js (100%) rename client/{ => utils}/formatWebpackErrors.js (100%) rename sockets/utils/getResourceQuery.js => client/utils/parseResourceQuery.js (71%) diff --git a/client/ErrorOverlayEntry.js b/client/ErrorOverlayEntry.js index 35f2e7c2..dcc07584 100644 --- a/client/ErrorOverlayEntry.js +++ b/client/ErrorOverlayEntry.js @@ -1,7 +1,8 @@ -/* global __react_refresh_error_overlay__, __react_refresh_init_socket__ */ +/* global __react_refresh_error_overlay__, __react_refresh_init_socket__, __resourceQuery */ -const registerErrorEventHandlers = require('./errorEventHandlers'); -const formatWebpackErrors = require('./formatWebpackErrors'); +const errorEventHandlers = require('./utils/errorEventHandlers'); +const formatWebpackErrors = require('./utils/formatWebpackErrors'); +const parseResourceQuery = require('./utils/parseResourceQuery'); // Setup error states let isHotReload = false; @@ -71,13 +72,13 @@ function compileMessageHandler(message) { } // Registers handlers for compile errors -__react_refresh_init_socket__(compileMessageHandler); +__react_refresh_init_socket__(compileMessageHandler, parseResourceQuery(__resourceQuery)); // Registers handlers for runtime errors -registerErrorEventHandlers.error(function handleError(error) { +errorEventHandlers.error(function handleError(error) { hasRuntimeErrors = true; __react_refresh_error_overlay__.handleRuntimeError(error); }); -registerErrorEventHandlers.unhandledRejection(function handleUnhandledPromiseRejection(error) { +errorEventHandlers.unhandledRejection(function handleUnhandledPromiseRejection(error) { hasRuntimeErrors = true; __react_refresh_error_overlay__.handleRuntimeError(error); }); diff --git a/client/errorEventHandlers.js b/client/utils/errorEventHandlers.js similarity index 100% rename from client/errorEventHandlers.js rename to client/utils/errorEventHandlers.js diff --git a/client/formatWebpackErrors.js b/client/utils/formatWebpackErrors.js similarity index 100% rename from client/formatWebpackErrors.js rename to client/utils/formatWebpackErrors.js diff --git a/sockets/utils/getResourceQuery.js b/client/utils/parseResourceQuery.js similarity index 71% rename from sockets/utils/getResourceQuery.js rename to client/utils/parseResourceQuery.js index fef3bae7..9cf0999f 100644 --- a/sockets/utils/getResourceQuery.js +++ b/client/utils/parseResourceQuery.js @@ -1,16 +1,10 @@ -/* global __resourceQuery */ - /** - * Parse webpack `__resourceQuery` string into an object. + * Parse Webpack's `__resourceQuery` string into an object. * @see https://webpack.js.org/api/module-variables/#__resourcequery-webpack-specific + * @param {*} [query] The Webpack `__resourceQuery` string. * @returns {*} The parsed query params. */ -function getResourceQuery() { - let query = ''; - if (typeof __resourceQuery === 'string') { - query = __resourceQuery; - } - +function parseResourceQuery(query = '') { /** * Reduce __resourceQuery string such as `?foo1=bar1&foo2=bar2`: * - remove `?` from the start @@ -32,4 +26,4 @@ function getResourceQuery() { }, {}); } -module.exports = getResourceQuery; +module.exports = parseResourceQuery; diff --git a/sockets/WDSSocket.js b/sockets/WDSSocket.js index 7a9711df..ca248c06 100644 --- a/sockets/WDSSocket.js +++ b/sockets/WDSSocket.js @@ -1,26 +1,25 @@ /* global __webpack_dev_server_client__ */ const url = require('native-url'); -const getResourceQuery = require('./utils/getResourceQuery'); /** * Initializes a socket server for HMR for webpack-dev-server. * @param {function(*): void} messageHandler A handler to consume Webpack compilation messages. + * @param {*} overrides Socket integration overrides to change the connection URL. * @returns {void} */ -function initWDSSocket(messageHandler) { +function initWDSSocket(messageHandler, overrides) { if (typeof __webpack_dev_server_client__ !== 'undefined') { // Get config overrides from webpack __resourceQuery global - const query = getResourceQuery(); const SocketClient = __webpack_dev_server_client__; // TODO: Support usage of custom sockets after WDS 4.0 is released // Ref: https://github.com/webpack/webpack-dev-server/pull/2055 const connection = new SocketClient( url.format({ protocol: window.location.protocol, - hostname: query.sockHost || window.location.hostname, - port: query.sockPort || window.location.port, - pathname: query.sockPath || '/sockjs-node', + hostname: overrides.sockHost || window.location.hostname, + port: overrides.sockPort || window.location.port, + pathname: overrides.sockPath || '/sockjs-node', }) ); From 0f58781e0a206ce540a195420c87292fd7ec5b3f Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 05:03:07 +0800 Subject: [PATCH 2/7] test: update resource query parsing tests --- test/unit/getResourceQuery.test.js | 30 ---------------------------- test/unit/parseResourceQuery.test.js | 21 +++++++++++++++++++ 2 files changed, 21 insertions(+), 30 deletions(-) delete mode 100644 test/unit/getResourceQuery.test.js create mode 100644 test/unit/parseResourceQuery.test.js diff --git a/test/unit/getResourceQuery.test.js b/test/unit/getResourceQuery.test.js deleted file mode 100644 index 184497c7..00000000 --- a/test/unit/getResourceQuery.test.js +++ /dev/null @@ -1,30 +0,0 @@ -const getResourceQuery = require('../../sockets/utils/getResourceQuery'); - -describe('getResourceQuery', () => { - beforeEach(() => { - global.__resourceQuery = undefined; - }); - - afterAll(() => { - delete global.__resourceQuery; - }); - - it('should parse __resourceQuery', () => { - global.__resourceQuery = '?sockHost=localhost&sockPath=/__socket&sockPort=8080'; - expect(getResourceQuery()).toStrictEqual({ - sockHost: 'localhost', - sockPath: '/__socket', - sockPort: '8080', - }); - }); - - it('should handle undefined __resourceQuery', () => { - delete global.__resourceQuery; - expect(getResourceQuery()).toStrictEqual({}); - }); - - it('should handle empty string __resourceQuery', () => { - global.__resourceQuery = ''; - expect(getResourceQuery()).toStrictEqual({}); - }); -}); diff --git a/test/unit/parseResourceQuery.test.js b/test/unit/parseResourceQuery.test.js new file mode 100644 index 00000000..164401a8 --- /dev/null +++ b/test/unit/parseResourceQuery.test.js @@ -0,0 +1,21 @@ +const parseResourceQuery = require('../../client/utils/parseResourceQuery'); + +describe('parseResourceQuery', () => { + it('should parse __resourceQuery', () => { + expect( + parseResourceQuery('?sockHost=localhost&sockPath=/__socket&sockPort=8080') + ).toStrictEqual({ + sockHost: 'localhost', + sockPath: '/__socket', + sockPort: '8080', + }); + }); + + it('should handle undefined __resourceQuery', () => { + expect(parseResourceQuery()).toStrictEqual({}); + }); + + it('should handle empty string __resourceQuery', () => { + expect(parseResourceQuery('')).toStrictEqual({}); + }); +}); From 139225798ac9eeeb831750fa017d0d7f2c75fc20 Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 16:33:48 +0800 Subject: [PATCH 3/7] fix: add current script handling and better url parts parsing --- client/ErrorOverlayEntry.js | 3 +- client/LegacyWDSSocketEntry.js | 2 +- client/ReactRefreshEntry.js | 2 +- client/{ => utils}/safeThis.js | 0 sockets/WDSSocket.js | 19 ++-- sockets/utils/getCurrentScriptSource.js | 22 +++++ sockets/utils/getSocketUrlParts.js | 86 +++++++++++++++++++ .../utils/parseQuery.js | 20 +++-- 8 files changed, 129 insertions(+), 25 deletions(-) rename client/{ => utils}/safeThis.js (100%) create mode 100644 sockets/utils/getCurrentScriptSource.js create mode 100644 sockets/utils/getSocketUrlParts.js rename client/utils/parseResourceQuery.js => sockets/utils/parseQuery.js (50%) diff --git a/client/ErrorOverlayEntry.js b/client/ErrorOverlayEntry.js index dcc07584..b1f85f6e 100644 --- a/client/ErrorOverlayEntry.js +++ b/client/ErrorOverlayEntry.js @@ -2,7 +2,6 @@ const errorEventHandlers = require('./utils/errorEventHandlers'); const formatWebpackErrors = require('./utils/formatWebpackErrors'); -const parseResourceQuery = require('./utils/parseResourceQuery'); // Setup error states let isHotReload = false; @@ -72,7 +71,7 @@ function compileMessageHandler(message) { } // Registers handlers for compile errors -__react_refresh_init_socket__(compileMessageHandler, parseResourceQuery(__resourceQuery)); +__react_refresh_init_socket__(compileMessageHandler, __resourceQuery); // Registers handlers for runtime errors errorEventHandlers.error(function handleError(error) { hasRuntimeErrors = true; diff --git a/client/LegacyWDSSocketEntry.js b/client/LegacyWDSSocketEntry.js index e199b60a..445292bd 100644 --- a/client/LegacyWDSSocketEntry.js +++ b/client/LegacyWDSSocketEntry.js @@ -1,5 +1,5 @@ const SockJS = require('sockjs-client/dist/sockjs'); -const safeThis = require('./safeThis'); +const safeThis = require('./utils/safeThis'); /** * A SockJS client adapted for use with webpack-dev-server. diff --git a/client/ReactRefreshEntry.js b/client/ReactRefreshEntry.js index 2d6354e4..ac9d505a 100644 --- a/client/ReactRefreshEntry.js +++ b/client/ReactRefreshEntry.js @@ -1,4 +1,4 @@ -const safeThis = require('./safeThis'); +const safeThis = require('./utils/safeThis'); if (process.env.NODE_ENV !== 'production' && typeof safeThis !== 'undefined') { // Only inject the runtime if it hasn't been injected diff --git a/client/safeThis.js b/client/utils/safeThis.js similarity index 100% rename from client/safeThis.js rename to client/utils/safeThis.js diff --git a/sockets/WDSSocket.js b/sockets/WDSSocket.js index ca248c06..0bf66cba 100644 --- a/sockets/WDSSocket.js +++ b/sockets/WDSSocket.js @@ -1,27 +1,20 @@ /* global __webpack_dev_server_client__ */ const url = require('native-url'); +const getSocketUrlParts = require('./utils/getSocketUrlParts'); /** * Initializes a socket server for HMR for webpack-dev-server. * @param {function(*): void} messageHandler A handler to consume Webpack compilation messages. - * @param {*} overrides Socket integration overrides to change the connection URL. + * @param {string} [resourceQuery] Webpack's `__resourceQuery` string. * @returns {void} */ -function initWDSSocket(messageHandler, overrides) { +function initWDSSocket(messageHandler, resourceQuery) { if (typeof __webpack_dev_server_client__ !== 'undefined') { - // Get config overrides from webpack __resourceQuery global const SocketClient = __webpack_dev_server_client__; - // TODO: Support usage of custom sockets after WDS 4.0 is released - // Ref: https://github.com/webpack/webpack-dev-server/pull/2055 - const connection = new SocketClient( - url.format({ - protocol: window.location.protocol, - hostname: overrides.sockHost || window.location.hostname, - port: overrides.sockPort || window.location.port, - pathname: overrides.sockPath || '/sockjs-node', - }) - ); + + const urlParts = getSocketUrlParts(resourceQuery); + const connection = new SocketClient(url.format(urlParts)); connection.onMessage(function onSocketMessage(data) { const message = JSON.parse(data); diff --git a/sockets/utils/getCurrentScriptSource.js b/sockets/utils/getCurrentScriptSource.js new file mode 100644 index 00000000..ef2fe49c --- /dev/null +++ b/sockets/utils/getCurrentScriptSource.js @@ -0,0 +1,22 @@ +/** + * Gets the source (i.e. host) of the script currently running. + * @returns {string} + */ +function getCurrentScriptSource() { + // `document.currentScript` is the most accurate way to get the current running script, + // but is not supported in all browsers (most notably, IE). + if (document.currentScript) { + return document.currentScript.getAttribute('src'); + } + + // Fallback to getting all scripts running in the document. + const scriptElements = document.scripts || []; + const currentScript = scriptElements[scriptElements.length - 1]; + if (currentScript) { + return currentScript.getAttribute('src'); + } + + throw new Error('Failed to get current script source!'); +} + +module.exports = getCurrentScriptSource; diff --git a/sockets/utils/getSocketUrlParts.js b/sockets/utils/getSocketUrlParts.js new file mode 100644 index 00000000..7223e700 --- /dev/null +++ b/sockets/utils/getSocketUrlParts.js @@ -0,0 +1,86 @@ +const getCurrentScriptSource = require('./getCurrentScriptSource'); +const parseQuery = require('./parseQuery'); + +/** + * @typedef {Object} SocketUrlParts + * @property {string} [auth] + * @property {string} [hostname] + * @property {string} [protocol] + * @property {string} [pathname] + * @property {string} [port] + */ + +/** + * Parse current location and Webpack's `__resourceQuery` into parts that can create a valid socket URL. + * @param {string} [resourceQuery] The Webpack `__resourceQuery` string. + * @returns {SocketUrlParts} The parsed URL parts. + * @see https://webpack.js.org/api/module-variables/#__resourcequery-webpack-specific + */ +function getSocketUrlParts(resourceQuery) { + const url = new URL(getCurrentScriptSource()); + + /** @type {string} */ + let auth; + let hostname = url.hostname; + let protocol = url.protocol; + let pathname = '/sockjs-node'; // This is hard-coded in WDS + let port = url.port; + + // Parse authentication credentials in case we need them + if (url.username) { + auth = url.username; + + // Since HTTP basic authentication does not allow empty username, + // we only include password if the username is not empty. + if (url.password) { + // Result: : + auth = auth.concat(':', url.password); + } + } + + // Check for IPv4 and IPv6 host addresses that corresponds to `any`/`empty`. + // This is important because `hostname` can be empty for some hosts, + // such as `about:blank` or `file://` URLs. + const isEmptyHostname = + url.hostname === '0.0.0.0' || + url.hostname === '0000:0000:0000:0000:0000:0000:0000:0000' || + url.hostname === '::'; + + // We only re-assign the hostname if we are using the HTTP protocol + if ( + isEmptyHostname && + window.location.hostname && + window.location.protocol.indexOf('http') === 0 + ) { + hostname = window.location.hostname; + } + + // We only re-assign `protocol` when `hostname` is available and is empty, + // since otherwise we risk creating an invalid URL. + // We also do this when `https` is used as it mandates the use of secure sockets. + if (hostname && (isEmptyHostname || window.location.protocol === 'https:')) { + protocol = window.location.protocol; + } + + // We only re-assign port when it is not available or `empty` + if (!port || port === '0') { + port = window.location.port; + } + + // If the resource query is available, + // parse it and overwrite everything we received from the script host. + const parsedQuery = parseQuery(resourceQuery || ''); + hostname = parsedQuery.sockHost || hostname; + pathname = parsedQuery.sockPath || pathname; + port = parsedQuery.sockPort || port; + + return { + auth: auth, + hostname: hostname, + pathname: pathname, + protocol: protocol, + port: port, + }; +} + +module.exports = getSocketUrlParts; diff --git a/client/utils/parseResourceQuery.js b/sockets/utils/parseQuery.js similarity index 50% rename from client/utils/parseResourceQuery.js rename to sockets/utils/parseQuery.js index 9cf0999f..4422b30b 100644 --- a/client/utils/parseResourceQuery.js +++ b/sockets/utils/parseQuery.js @@ -1,15 +1,19 @@ /** - * Parse Webpack's `__resourceQuery` string into an object. - * @see https://webpack.js.org/api/module-variables/#__resourcequery-webpack-specific - * @param {*} [query] The Webpack `__resourceQuery` string. - * @returns {*} The parsed query params. + * Parse a query string into an object. + * @param {string} [querystring] The query string. + * @returns {Record} The parsed query object. */ -function parseResourceQuery(query = '') { +function parseQuery(querystring) { + let query = ''; + if (typeof querystring === 'string') { + query = querystring; + } + /** - * Reduce __resourceQuery string such as `?foo1=bar1&foo2=bar2`: + * Transform query strings such as `?foo1=bar1&foo2=bar2`: * - remove `?` from the start * - split with `&` - * - split with `=` + * - split pairs with `=` * The resulting format will be { foo1: 'bar1', foo2: 'bar2' } */ return query @@ -26,4 +30,4 @@ function parseResourceQuery(query = '') { }, {}); } -module.exports = parseResourceQuery; +module.exports = parseQuery; From 694f6a602cc1bcef2f584a5d67cbae32f7adcdba Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 16:34:13 +0800 Subject: [PATCH 4/7] test: update tests in accordance to changes --- test/loader/loader.test.js | 2 +- test/unit/parseQuery.test.js | 23 +++++++++++++++++++++++ test/unit/parseResourceQuery.test.js | 21 --------------------- 3 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 test/unit/parseQuery.test.js delete mode 100644 test/unit/parseResourceQuery.test.js diff --git a/test/loader/loader.test.js b/test/loader/loader.test.js index c9891394..8193489b 100644 --- a/test/loader/loader.test.js +++ b/test/loader/loader.test.js @@ -603,7 +603,7 @@ describe('loader', () => { /*! namespace exports */ /*! export default [provided] [unused] [could be renamed] */ /*! other exports [not provided] [unused] */ - /*! runtime requirements: module, __webpack_require__, module.id */ + /*! runtime requirements: __webpack_require__, module.id, module */ /***/ ((module, __unused_webpack___webpack_exports__, __webpack_require__) => { \\"use strict\\"; diff --git a/test/unit/parseQuery.test.js b/test/unit/parseQuery.test.js new file mode 100644 index 00000000..412057fa --- /dev/null +++ b/test/unit/parseQuery.test.js @@ -0,0 +1,23 @@ +/** + * @jest-environment jsdom + */ + +const parseQuery = require('../../sockets/utils/parseQuery'); + +describe('parseQuery', () => { + it('should parse valid query string', () => { + expect(parseQuery('?sockHost=localhost&sockPath=/__socket&sockPort=8080')).toStrictEqual({ + sockHost: 'localhost', + sockPath: '/__socket', + sockPort: '8080', + }); + }); + + it('should handle undefined query string', () => { + expect(parseQuery()).toStrictEqual({}); + }); + + it('should handle empty query string', () => { + expect(parseQuery('')).toStrictEqual({}); + }); +}); diff --git a/test/unit/parseResourceQuery.test.js b/test/unit/parseResourceQuery.test.js deleted file mode 100644 index 164401a8..00000000 --- a/test/unit/parseResourceQuery.test.js +++ /dev/null @@ -1,21 +0,0 @@ -const parseResourceQuery = require('../../client/utils/parseResourceQuery'); - -describe('parseResourceQuery', () => { - it('should parse __resourceQuery', () => { - expect( - parseResourceQuery('?sockHost=localhost&sockPath=/__socket&sockPort=8080') - ).toStrictEqual({ - sockHost: 'localhost', - sockPath: '/__socket', - sockPort: '8080', - }); - }); - - it('should handle undefined __resourceQuery', () => { - expect(parseResourceQuery()).toStrictEqual({}); - }); - - it('should handle empty string __resourceQuery', () => { - expect(parseResourceQuery('')).toStrictEqual({}); - }); -}); From 3cd30e073351f78ffe0877a4a53a6e97ee49e4a0 Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 18:44:32 +0800 Subject: [PATCH 5/7] test: add getSocketUrlParts tests --- .eslintrc.json | 1 + sockets/utils/getSocketUrlParts.js | 9 +- test/mocks/location.js | 24 ++++ test/unit/getSocketUrlParts.test.js | 177 ++++++++++++++++++++++++++++ test/unit/parseQuery.test.js | 9 +- 5 files changed, 213 insertions(+), 7 deletions(-) create mode 100644 test/mocks/location.js create mode 100644 test/unit/getSocketUrlParts.test.js diff --git a/.eslintrc.json b/.eslintrc.json index dbfebfc4..4e47aacd 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -29,6 +29,7 @@ "files": [ "test/jest-test-setup.js", "test/helpers/{,!(fixtures)*/}*.js", + "test/mocks/**/*.js", "test/**/*.test.js" ], "env": { diff --git a/sockets/utils/getSocketUrlParts.js b/sockets/utils/getSocketUrlParts.js index 7223e700..2c6c1460 100644 --- a/sockets/utils/getSocketUrlParts.js +++ b/sockets/utils/getSocketUrlParts.js @@ -41,16 +41,13 @@ function getSocketUrlParts(resourceQuery) { // Check for IPv4 and IPv6 host addresses that corresponds to `any`/`empty`. // This is important because `hostname` can be empty for some hosts, // such as `about:blank` or `file://` URLs. - const isEmptyHostname = - url.hostname === '0.0.0.0' || - url.hostname === '0000:0000:0000:0000:0000:0000:0000:0000' || - url.hostname === '::'; + const isEmptyHostname = url.hostname === '0.0.0.0' || url.hostname === '[::]'; - // We only re-assign the hostname if we are using the HTTP protocol + // We only re-assign the hostname if we are using HTTP/HTTPS protocols if ( isEmptyHostname && window.location.hostname && - window.location.protocol.indexOf('http') === 0 + window.location.protocol.indexOf('http') !== -1 ) { hostname = window.location.hostname; } diff --git a/test/mocks/location.js b/test/mocks/location.js new file mode 100644 index 00000000..bff01b8a --- /dev/null +++ b/test/mocks/location.js @@ -0,0 +1,24 @@ +/** @type {Set} */ +const cleanupHandlers = new Set(); +afterEach(() => { + [...cleanupHandlers].map((callback) => callback()); +}); + +const location = (href) => { + const originalLocation = global.window.location; + + delete global.window.location; + global.window.location = new URL(href); + + function mockRestore() { + global.window.location = originalLocation; + } + + cleanupHandlers.add(mockRestore); + + return { + mockRestore, + }; +}; + +module.exports = location; diff --git a/test/unit/getSocketUrlParts.test.js b/test/unit/getSocketUrlParts.test.js new file mode 100644 index 00000000..03167ef4 --- /dev/null +++ b/test/unit/getSocketUrlParts.test.js @@ -0,0 +1,177 @@ +/** + * @jest-environment jsdom + */ + +jest.mock('../../sockets/utils/getCurrentScriptSource'); + +const getCurrentScriptSource = require('../../sockets/utils/getCurrentScriptSource'); +const getSocketUrlParts = require('../../sockets/utils/getSocketUrlParts'); +const mockLocation = require('../mocks/location'); + +describe('getSocketUrlParts', () => { + beforeEach(() => { + getCurrentScriptSource.mockReset(); + jest.resetModules(); + }); + + it('should work when script source is a valid HTTP URL', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when script source is a valid HTTPS URL', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'https://localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'https:', + }); + }); + + it('should work when script source is 0.0.0.0', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://0.0.0.0:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when script source is [::]', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://[::]:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when script source port is 0', () => { + mockLocation('http://localhost:8080'); + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:0'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when script source port is unavailable', () => { + mockLocation('http://localhost:8080'); + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when current location is about:blank', () => { + mockLocation('about:blank'); + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should work when current location uses the file protocol', () => { + mockLocation('file://test.html'); + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should use HTTPS when current location uses HTTPS', () => { + mockLocation('https://localhost:8080'); + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'https:', + }); + }); + + it('should include username when it is available from the script source', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://username@localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: 'username', + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should include username and password when both are available from the script source', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://username:password@localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: 'username:password', + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should not include password when username is unavailable from the script source', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://:password@localhost:8080'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + + it('should use info from resource query when available', () => { + getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); + + expect(getSocketUrlParts('?sockHost=foo.com&sockPath=socket&sockPort=9000')).toStrictEqual({ + auth: undefined, + hostname: 'foo.com', + pathname: '/socket', + port: '9000', + protocol: 'https:', + }); + }); +}); diff --git a/test/unit/parseQuery.test.js b/test/unit/parseQuery.test.js index 412057fa..d0642d1e 100644 --- a/test/unit/parseQuery.test.js +++ b/test/unit/parseQuery.test.js @@ -5,7 +5,7 @@ const parseQuery = require('../../sockets/utils/parseQuery'); describe('parseQuery', () => { - it('should parse valid query string', () => { + it('should handle valid query string', () => { expect(parseQuery('?sockHost=localhost&sockPath=/__socket&sockPort=8080')).toStrictEqual({ sockHost: 'localhost', sockPath: '/__socket', @@ -13,6 +13,13 @@ describe('parseQuery', () => { }); }); + it('should handle malformed query string', () => { + expect(parseQuery('?malformedKey&=malformedValue&valid=1')).toStrictEqual({ + malformedKey: undefined, + valid: '1', + }); + }); + it('should handle undefined query string', () => { expect(parseQuery()).toStrictEqual({}); }); From 24e15cac415d751a35e9ad3c953359fdd40a9b72 Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Tue, 7 Jul 2020 18:57:12 +0800 Subject: [PATCH 6/7] test: fix typos in test assertion --- test/unit/getSocketUrlParts.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/getSocketUrlParts.test.js b/test/unit/getSocketUrlParts.test.js index 03167ef4..04a27031 100644 --- a/test/unit/getSocketUrlParts.test.js +++ b/test/unit/getSocketUrlParts.test.js @@ -166,12 +166,12 @@ describe('getSocketUrlParts', () => { it('should use info from resource query when available', () => { getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080'); - expect(getSocketUrlParts('?sockHost=foo.com&sockPath=socket&sockPort=9000')).toStrictEqual({ + expect(getSocketUrlParts('?sockHost=foo.com&sockPath=/socket&sockPort=9000')).toStrictEqual({ auth: undefined, hostname: 'foo.com', pathname: '/socket', port: '9000', - protocol: 'https:', + protocol: 'http:', }); }); }); From 16c6f182a25708b0cda1c8ec68578531eb944de4 Mon Sep 17 00:00:00 2001 From: Michael Mok Date: Wed, 8 Jul 2020 16:01:11 +0800 Subject: [PATCH 7/7] fix: handle relative script srcs with url.parse --- sockets/utils/getSocketUrlParts.js | 31 +++++++++++++++++++---------- test/unit/getSocketUrlParts.test.js | 13 ++++++++++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/sockets/utils/getSocketUrlParts.js b/sockets/utils/getSocketUrlParts.js index 2c6c1460..b5c59105 100644 --- a/sockets/utils/getSocketUrlParts.js +++ b/sockets/utils/getSocketUrlParts.js @@ -1,3 +1,4 @@ +const url = require('native-url'); const getCurrentScriptSource = require('./getCurrentScriptSource'); const parseQuery = require('./parseQuery'); @@ -17,31 +18,41 @@ const parseQuery = require('./parseQuery'); * @see https://webpack.js.org/api/module-variables/#__resourcequery-webpack-specific */ function getSocketUrlParts(resourceQuery) { - const url = new URL(getCurrentScriptSource()); + const scriptSource = getCurrentScriptSource(); + const urlParts = url.parse(scriptSource); - /** @type {string} */ + /** @type {string | undefined} */ let auth; - let hostname = url.hostname; - let protocol = url.protocol; + let hostname = urlParts.hostname; + let protocol = urlParts.protocol; let pathname = '/sockjs-node'; // This is hard-coded in WDS - let port = url.port; + let port = urlParts.port; + // FIXME: + // This is a hack to work-around `native-url`'s parse method, + // which filters out falsy values when concatenating the `auth` string. + // In reality, we need to check for both values to correctly inject them. + // Ref: GoogleChromeLabs/native-url#32 + // The placeholder `baseURL` is to allow parsing of relative paths, + // and will have no effect if `scriptSource` is a proper URL. + const authUrlParts = new URL(scriptSource, 'http://foo.bar'); // Parse authentication credentials in case we need them - if (url.username) { - auth = url.username; + if (authUrlParts.username) { + auth = authUrlParts.username; // Since HTTP basic authentication does not allow empty username, // we only include password if the username is not empty. - if (url.password) { + if (authUrlParts.password) { // Result: : - auth = auth.concat(':', url.password); + auth = auth.concat(':', authUrlParts.password); } } // Check for IPv4 and IPv6 host addresses that corresponds to `any`/`empty`. // This is important because `hostname` can be empty for some hosts, // such as `about:blank` or `file://` URLs. - const isEmptyHostname = url.hostname === '0.0.0.0' || url.hostname === '[::]'; + const isEmptyHostname = + urlParts.hostname === '0.0.0.0' || urlParts.hostname === '::' || urlParts.hostname === null; // We only re-assign the hostname if we are using HTTP/HTTPS protocols if ( diff --git a/test/unit/getSocketUrlParts.test.js b/test/unit/getSocketUrlParts.test.js index 04a27031..62ffd350 100644 --- a/test/unit/getSocketUrlParts.test.js +++ b/test/unit/getSocketUrlParts.test.js @@ -62,6 +62,19 @@ describe('getSocketUrlParts', () => { }); }); + it('should work when script source is relative', () => { + mockLocation('http://localhost:8080'); + getCurrentScriptSource.mockImplementationOnce(() => 'main.js'); + + expect(getSocketUrlParts()).toStrictEqual({ + auth: undefined, + hostname: 'localhost', + pathname: '/sockjs-node', + port: '8080', + protocol: 'http:', + }); + }); + it('should work when script source port is 0', () => { mockLocation('http://localhost:8080'); getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:0');