Skip to content

Commit 8b0ef4a

Browse files
nougadnougad
committed
feat(client): refactor createSocketUrl to make it better unit tested
1 parent d1a86d7 commit 8b0ef4a

File tree

2 files changed

+154
-34
lines changed

2 files changed

+154
-34
lines changed

‎client-src/default/utils/createSocketUrl.js

+43-34
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,60 @@
33
/* global self */
44

55
const url = require('url');
6-
const querystring = require('querystring');
76
const getCurrentScriptSource = require('./getCurrentScriptSource');
87

9-
function createSocketUrl(resourceQuery) {
8+
function createSocketUrl(resourceQuery, currentLocation) {
109
let urlParts;
1110

1211
if (typeof resourceQuery === 'string' && resourceQuery !== '') {
1312
// If this bundle is inlined, use the resource query to get the correct url.
14-
// strip leading `?` from query string
15-
urlParts = url.parse(resourceQuery.substr(1));
13+
// format is like `?http://0.0.0.0:8096&sockPort=8097&sockHost=localhost`
14+
urlParts = url.parse(
15+
resourceQuery
16+
// strip leading `?` from query string to get a valid URL
17+
.substr(1)
18+
// replace first `&` with `?` to have a valid query string
19+
.replace('&', '?'),
20+
true
21+
);
1622
} else {
1723
// Else, get the url from the <script> this file was called with.
1824
const scriptHost = getCurrentScriptSource();
19-
urlParts = url.parse(scriptHost || '/', false, true);
25+
urlParts = url.parse(scriptHost || '/', true, true);
2026
}
2127

22-
if (!urlParts.port || urlParts.port === '0') {
23-
urlParts.port = self.location.port;
28+
// Use parameter to allow passing location in unit tests
29+
if (typeof currentLocation === 'string' && currentLocation !== '') {
30+
currentLocation = url.parse(currentLocation);
31+
} else {
32+
currentLocation = self.location;
2433
}
2534

26-
const { auth, path } = urlParts;
27-
let { hostname, protocol } = urlParts;
35+
return getSocketUrl(urlParts, currentLocation);
36+
}
37+
38+
/*
39+
* Gets socket URL based on Script Source/Location
40+
* (scriptSrc: URL, location: URL) -> URL
41+
*/
42+
function getSocketUrl(urlParts, loc) {
43+
const { auth, query } = urlParts;
44+
let { hostname, protocol, port } = urlParts;
45+
46+
if (!port || port === '0') {
47+
port = loc.port;
48+
}
2849

2950
// check ipv4 and ipv6 `all hostname`
3051
// why do we need this check?
3152
// hostname n/a for file protocol (example, when using electron, ionic)
3253
// see: https://github.com/webpack/webpack-dev-server/pull/384
33-
const isAnyHostname =
54+
if (
3455
(hostname === '0.0.0.0' || hostname === '::') &&
35-
self.location.hostname &&
36-
// eslint-disable-next-line no-bitwise
37-
!!~self.location.protocol.indexOf('http');
38-
39-
if (isAnyHostname) {
40-
hostname = self.location.hostname;
56+
loc.hostname &&
57+
loc.protocol.startsWith('http')
58+
) {
59+
hostname = loc.hostname;
4160
}
4261

4362
// `hostname` can be empty when the script path is relative. In that case, specifying
@@ -47,27 +66,17 @@ function createSocketUrl(resourceQuery) {
4766
if (
4867
hostname &&
4968
hostname !== '127.0.0.1' &&
50-
(self.location.protocol === 'https:' || urlParts.hostname === '0.0.0.0')
69+
(loc.protocol === 'https:' || urlParts.hostname === '0.0.0.0')
5170
) {
52-
protocol = self.location.protocol;
71+
protocol = loc.protocol;
5372
}
5473

55-
// default values of the sock url if they are not provided
56-
let sockHost = hostname;
57-
let sockPath = '/sockjs-node';
58-
let sockPort = urlParts.port;
59-
60-
// eslint-disable-next-line no-undefined
61-
const shouldParsePath = path !== null && path !== undefined && path !== '/';
62-
if (shouldParsePath) {
63-
const parsedQuery = querystring.parse(path);
64-
// all of these sock url params are optionally passed in through
65-
// resourceQuery, so we need to fall back to the default if
66-
// they are not provided
67-
sockHost = parsedQuery.sockHost || sockHost;
68-
sockPath = parsedQuery.sockPath || sockPath;
69-
sockPort = parsedQuery.sockPort || sockPort;
70-
}
74+
// all of these sock url params are optionally passed in through
75+
// resourceQuery, so we need to fall back to the default if
76+
// they are not provided
77+
const sockHost = query.sockHost || hostname;
78+
const sockPath = query.sockPath || '/sockjs-node';
79+
const sockPort = query.sockPort || port;
7180

7281
return url.format({
7382
protocol,

‎test/client/utils/createSocketUrl.test.js

+111
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,115 @@ describe('createSocketUrl', () => {
3838
// put here because resetModules mustn't be reset when L20 is finished
3939
jest.resetModules();
4040
});
41+
42+
const samples2 = [
43+
// script source, location, output socket URL
44+
[
45+
'http://example.com',
46+
'https://something.com',
47+
'https://example.com/sockjs-node',
48+
],
49+
[
50+
'http://127.0.0.1',
51+
'https://something.com',
52+
'http://127.0.0.1/sockjs-node',
53+
],
54+
[
55+
'http://0.0.0.0',
56+
'https://something.com',
57+
'https://something.com/sockjs-node',
58+
],
59+
[
60+
'http://0.0.0.0',
61+
'http://something.com',
62+
'http://something.com/sockjs-node',
63+
],
64+
[
65+
'http://example.com',
66+
'http://something.com',
67+
'http://example.com/sockjs-node',
68+
],
69+
[
70+
'https://example.com',
71+
'http://something.com',
72+
'https://example.com/sockjs-node',
73+
],
74+
];
75+
76+
samples2.forEach(([scriptSrc, loc, expected]) => {
77+
jest.doMock(
78+
'../../../client-src/default/utils/getCurrentScriptSource.js',
79+
() => () => scriptSrc
80+
);
81+
82+
const createSocketUrl = require('../../../client-src/default/utils/createSocketUrl');
83+
84+
test(`should return socket ${expected} for script source ${scriptSrc} and location ${loc}`, () => {
85+
// eslint-disable-next-line no-undefined
86+
expect(createSocketUrl(undefined, loc).toString()).toEqual(expected);
87+
});
88+
89+
jest.resetModules();
90+
});
91+
92+
const samples3 = [
93+
// script source, location, output socket URL
94+
[
95+
'?http://example.com',
96+
'https://something.com',
97+
'https://example.com/sockjs-node',
98+
],
99+
[
100+
'?http://127.0.0.1',
101+
'https://something.com',
102+
'http://127.0.0.1/sockjs-node',
103+
],
104+
[
105+
'?http://0.0.0.0',
106+
'https://something.com',
107+
'https://something.com/sockjs-node',
108+
],
109+
[
110+
'?http://0.0.0.0',
111+
'http://something.com',
112+
'http://something.com/sockjs-node',
113+
],
114+
[
115+
'?http://example.com',
116+
'http://something.com',
117+
'http://example.com/sockjs-node',
118+
],
119+
[
120+
'?https://example.com',
121+
'http://something.com',
122+
'https://example.com/sockjs-node',
123+
],
124+
[
125+
'?https://example.com?sockHost=asdf',
126+
'http://something.com',
127+
'https://asdf/sockjs-node',
128+
],
129+
[
130+
'?https://example.com?sockPort=34',
131+
'http://something.com',
132+
'https://example.com:34/sockjs-node',
133+
],
134+
[
135+
'?https://example.com?sockPath=xxx',
136+
'http://something.com',
137+
'https://example.com/xxx',
138+
],
139+
[
140+
'?http://0.0.0.0:8096&sockPort=8097',
141+
'http://localhost',
142+
'http://localhost:8097/sockjs-node',
143+
],
144+
];
145+
samples3.forEach(([scriptSrc, loc, expected]) => {
146+
test(`should return socket ${expected} for query ${scriptSrc} and location ${loc}`, () => {
147+
const createSocketUrl = require('../../../client-src/default/utils/createSocketUrl');
148+
149+
expect(createSocketUrl(scriptSrc, loc).toString()).toEqual(expected);
150+
});
151+
});
41152
});

0 commit comments

Comments
 (0)