From 9d9a8b343d875bcb4c28d65a8355be1bb8484f3e Mon Sep 17 00:00:00 2001 From: fyodorio Date: Wed, 14 Feb 2024 13:29:36 +0500 Subject: [PATCH 1/7] replace ip package with internal-ip The 'ip' package has been switched out for 'internal-ip' (with identical functionality coverage) because of the high-level security vulnerability in the former. The change is done at the consuming utility function and the corresponding test cases. --- code/lib/core-server/package.json | 3 +- .../utils/__tests__/server-address.test.ts | 8 +- .../src/utils/server-address.test.ts | 2 +- .../core-server/src/utils/server-address.ts | 4 +- code/yarn.lock | 153 ++++++++++++++++-- 5 files changed, 149 insertions(+), 21 deletions(-) diff --git a/code/lib/core-server/package.json b/code/lib/core-server/package.json index 1d00fe7feb56..907ddbae4c71 100644 --- a/code/lib/core-server/package.json +++ b/code/lib/core-server/package.json @@ -83,7 +83,7 @@ "express": "^4.17.3", "fs-extra": "^11.1.0", "globby": "^11.0.2", - "ip": "^2.0.0", + "internal-ip": "^8.0.0", "lodash": "^4.17.21", "open": "^8.4.0", "pretty-hrtime": "^1.0.3", @@ -101,7 +101,6 @@ "devDependencies": { "@storybook/addon-docs": "workspace:*", "@types/compression": "^1.7.0", - "@types/ip": "^1.1.0", "@types/node-fetch": "^2.5.7", "@types/ws": "^8", "boxen": "^7.1.1", diff --git a/code/lib/core-server/src/utils/__tests__/server-address.test.ts b/code/lib/core-server/src/utils/__tests__/server-address.test.ts index 3c2fb3defe91..ad7ef8d5c265 100644 --- a/code/lib/core-server/src/utils/__tests__/server-address.test.ts +++ b/code/lib/core-server/src/utils/__tests__/server-address.test.ts @@ -1,13 +1,13 @@ import { describe, beforeEach, it, expect, vi } from 'vitest'; -import ip from 'ip'; +import internalIp from 'internal-ip'; import { getServerAddresses } from '../server-address'; -vi.mock('ip'); -const mockedIp = vi.mocked(ip); +vi.mock('internal-ip'); +const mockedInternalIp = vi.mocked(internalIp); describe('getServerAddresses', () => { beforeEach(() => { - mockedIp.address.mockReturnValue('192.168.0.5'); + mockedInternalIp.internalIpV4Sync.mockReturnValue('192.168.0.5'); }); it('builds addresses with a specified host', () => { diff --git a/code/lib/core-server/src/utils/server-address.test.ts b/code/lib/core-server/src/utils/server-address.test.ts index e1a0b757c2ee..d376373cbf78 100644 --- a/code/lib/core-server/src/utils/server-address.test.ts +++ b/code/lib/core-server/src/utils/server-address.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi } from 'vitest'; import detectPort from 'detect-port'; import { getServerAddresses, getServerPort, getServerChannelUrl } from './server-address'; -vi.mock('ip'); +vi.mock('internal-ip'); vi.mock('detect-port'); vi.mock('@storybook/node-logger'); diff --git a/code/lib/core-server/src/utils/server-address.ts b/code/lib/core-server/src/utils/server-address.ts index bfcb2ba969d9..c93ace9e017d 100644 --- a/code/lib/core-server/src/utils/server-address.ts +++ b/code/lib/core-server/src/utils/server-address.ts @@ -1,4 +1,4 @@ -import ip from 'ip'; +import { internalIpV4Sync } from 'internal-ip'; import { logger } from '@storybook/node-logger'; import detectFreePort from 'detect-port'; @@ -10,7 +10,7 @@ export function getServerAddresses( initialPath?: string ) { const address = new URL(`${proto}://localhost:${port}/`); - const networkAddress = new URL(`${proto}://${host || ip.address()}:${port}/`); + const networkAddress = new URL(`${proto}://${host || internalIpV4Sync()}:${port}/`); if (initialPath) { const searchParams = `?path=${decodeURIComponent( diff --git a/code/yarn.lock b/code/yarn.lock index 755a51216c17..e5aa9165590b 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5556,7 +5556,6 @@ __metadata: "@storybook/types": "workspace:*" "@types/compression": "npm:^1.7.0" "@types/detect-port": "npm:^1.3.0" - "@types/ip": "npm:^1.1.0" "@types/node": "npm:^18.0.0" "@types/node-fetch": "npm:^2.5.7" "@types/pretty-hrtime": "npm:^1.0.0" @@ -5571,7 +5570,7 @@ __metadata: express: "npm:^4.17.3" fs-extra: "npm:^11.1.0" globby: "npm:^11.0.2" - ip: "npm:^2.0.0" + internal-ip: "npm:^8.0.0" lodash: "npm:^4.17.21" node-fetch: "npm:^3.3.1" open: "npm:^8.4.0" @@ -7491,15 +7490,6 @@ __metadata: languageName: node linkType: hard -"@types/ip@npm:^1.1.0": - version: 1.1.3 - resolution: "@types/ip@npm:1.1.3" - dependencies: - "@types/node": "npm:*" - checksum: af576e33830196be01b71c48ad5f83380a1c51d62f394a5601e8c2a5b8b31cf6dc8fe71ac39c38d806bcf1d6f1c5c8205c129eca6b6d168c0df7ab3722df23b9 - languageName: node - linkType: hard - "@types/is-empty@npm:^1.0.0": version: 1.2.3 resolution: "@types/is-empty@npm:1.2.3" @@ -11354,6 +11344,27 @@ __metadata: languageName: node linkType: hard +"cidr-regex@npm:4.0.3": + version: 4.0.3 + resolution: "cidr-regex@npm:4.0.3" + dependencies: + ip-regex: "npm:^5.0.0" + checksum: df12a5aecbae4fbafc38ca679d7654de97f0dc9ee0759ae1da3c001fadf063cf735ef7bec49e1421319e1adcc142a6603671ee562898cf16ff4ae8e29eddeec2 + languageName: node + linkType: hard + +"cidr-tools@npm:^6.4.1": + version: 6.4.2 + resolution: "cidr-tools@npm:6.4.2" + dependencies: + cidr-regex: "npm:4.0.3" + ip-bigint: "npm:7.3.0" + ip-regex: "npm:5.0.0" + string-natural-compare: "npm:3.0.1" + checksum: 0b856af13162f4a92b306897369b987cacc1e98c7fd7c26c64202312945eb283c6728063b7dfa727547cd566052c7fe375581bad8142d251f699e45367df8bb3 + languageName: node + linkType: hard + "cipher-base@npm:^1.0.0, cipher-base@npm:^1.0.1, cipher-base@npm:^1.0.3": version: 1.0.4 resolution: "cipher-base@npm:1.0.4" @@ -11537,6 +11548,15 @@ __metadata: languageName: node linkType: hard +"clone-regexp@npm:^3.0.0": + version: 3.0.0 + resolution: "clone-regexp@npm:3.0.0" + dependencies: + is-regexp: "npm:^3.0.0" + checksum: 892b6103ae9319d0283f9bb125e07cba7c043cbcc516ecc135be817769257c659eae42749d450ed3041af63fb79505e4c5b90105cf9c97cadbff712e2f72726b + languageName: node + linkType: hard + "clone-response@npm:^1.0.2": version: 1.0.3 resolution: "clone-response@npm:1.0.3" @@ -11947,6 +11967,13 @@ __metadata: languageName: node linkType: hard +"convert-hrtime@npm:^5.0.0": + version: 5.0.0 + resolution: "convert-hrtime@npm:5.0.0" + checksum: 2092e51aab205e1141440e84e2a89f8881e68e47c1f8bc168dfd7c67047d8f1db43bac28044bc05749205651fead4e7910f52c7bb6066213480df99e333e9f47 + languageName: node + linkType: hard + "convert-source-map@npm:^1.5.0, convert-source-map@npm:^1.5.1, convert-source-map@npm:^1.7.0": version: 1.9.0 resolution: "convert-source-map@npm:1.9.0" @@ -12633,6 +12660,15 @@ __metadata: languageName: node linkType: hard +"default-gateway@npm:^7.2.2": + version: 7.2.2 + resolution: "default-gateway@npm:7.2.2" + dependencies: + execa: "npm:^7.1.1" + checksum: 71338e8bd3eea4a2b5fd9371f3a4d7afe4218849da8035fa0d6a65df07ef3a8e7a81bb245983d85cf842872d79f4675d7382edc342507374b3732d151a9b5fc4 + languageName: node + linkType: hard + "defaults@npm:^1.0.3": version: 1.0.4 resolution: "defaults@npm:1.0.4" @@ -14577,7 +14613,7 @@ __metadata: languageName: node linkType: hard -"execa@npm:7.2.0": +"execa@npm:7.2.0, execa@npm:^7.1.1": version: 7.2.0 resolution: "execa@npm:7.2.0" dependencies: @@ -15654,6 +15690,13 @@ __metadata: languageName: node linkType: hard +"function-timeout@npm:^0.1.0": + version: 0.1.1 + resolution: "function-timeout@npm:0.1.1" + checksum: 45f0517907e541b7ea8238500429ac9ace50e596295c01931cbe61e176150aa2ad50d05dcfeeee9377ae48eb99a6fd0759e4ebc97cde8f4c38492d1c99db8f14 + languageName: node + linkType: hard + "function.prototype.name@npm:^1.1.5, function.prototype.name@npm:^1.1.6": version: 1.1.6 resolution: "function.prototype.name@npm:1.1.6" @@ -17243,6 +17286,18 @@ __metadata: languageName: node linkType: hard +"internal-ip@npm:^8.0.0": + version: 8.0.0 + resolution: "internal-ip@npm:8.0.0" + dependencies: + cidr-tools: "npm:^6.4.1" + default-gateway: "npm:^7.2.2" + is-ip: "npm:^5.0.0" + p-event: "npm:^5.0.1" + checksum: 68812fa59e92df7236c104c4bba1cbcea62e35e025ff8bf7799c22095c5bd1d05f7996d40bc82e723643ce007e40d03299584242cdd494ecf8bb8148a9b9728d + languageName: node + linkType: hard + "internal-slot@npm:^1.0.4, internal-slot@npm:^1.0.5": version: 1.0.5 resolution: "internal-slot@npm:1.0.5" @@ -17263,6 +17318,20 @@ __metadata: languageName: node linkType: hard +"ip-bigint@npm:7.3.0": + version: 7.3.0 + resolution: "ip-bigint@npm:7.3.0" + checksum: f9f77176ef3b2a506d3167287ebcd3eaf5db1ba3dac9c3758c04eade30855918f93e850e79036d14d0521c414af0e186f764d7f0c0d194c41ebc89b780d8fff2 + languageName: node + linkType: hard + +"ip-regex@npm:5.0.0, ip-regex@npm:^5.0.0": + version: 5.0.0 + resolution: "ip-regex@npm:5.0.0" + checksum: 23f07cf393436627b3a91f7121eee5bc831522d07c95ddd13f5a6f7757698b08551480f12e5dbb3bf248724da135d54405c9687733dba7314f74efae593bdf06 + languageName: node + linkType: hard + "ip@npm:^2.0.0": version: 2.0.0 resolution: "ip@npm:2.0.0" @@ -17640,6 +17709,16 @@ __metadata: languageName: node linkType: hard +"is-ip@npm:^5.0.0": + version: 5.0.1 + resolution: "is-ip@npm:5.0.1" + dependencies: + ip-regex: "npm:^5.0.0" + super-regex: "npm:^0.2.0" + checksum: ec7d833acaca68b5a11b7ee1d605ff26e6864f83cae6c1413aaf5be19975c4c8a9d0c2d4b339f3b9dd94f26851fc80c6942e4b02c2b42f5e42b40ecd900b7a5c + languageName: node + linkType: hard + "is-lambda@npm:^1.0.1": version: 1.0.1 resolution: "is-lambda@npm:1.0.1" @@ -17789,6 +17868,13 @@ __metadata: languageName: node linkType: hard +"is-regexp@npm:^3.0.0": + version: 3.1.0 + resolution: "is-regexp@npm:3.1.0" + checksum: 99dbaea41bddee2205db468c0946f5fee25cc4ae486333cb4d2b8095ab4b0a500e74ba61afd9e6e4f63ececcd55b4df5ae2a555b1c3e26308e516ff53c9533cd + languageName: node + linkType: hard + "is-set@npm:^2.0.1, is-set@npm:^2.0.2": version: 2.0.2 resolution: "is-set@npm:2.0.2" @@ -22225,6 +22311,15 @@ __metadata: languageName: node linkType: hard +"p-event@npm:^5.0.1": + version: 5.0.1 + resolution: "p-event@npm:5.0.1" + dependencies: + p-timeout: "npm:^5.0.2" + checksum: 2317171489537f316661fa863f3bb711b2ceb89182937238422cec10223cbb958c432d6c26a238446a622d788187bdd295b1d8ecedbe2e467e045930d60202b0 + languageName: node + linkType: hard + "p-finally@npm:^1.0.0": version: 1.0.0 resolution: "p-finally@npm:1.0.0" @@ -22357,6 +22452,13 @@ __metadata: languageName: node linkType: hard +"p-timeout@npm:^5.0.2": + version: 5.1.0 + resolution: "p-timeout@npm:5.1.0" + checksum: 1b026cf9d5878c64bec4341ca9cda8ec6b8b3aea8a57885ca0fe2b35753a20d767fb6f9d3aa41e1252f42bc95432c05ea33b6b18f271fb10bfb0789591850a41 + languageName: node + linkType: hard + "p-try@npm:^1.0.0": version: 1.0.0 resolution: "p-try@npm:1.0.0" @@ -26633,6 +26735,13 @@ __metadata: languageName: node linkType: hard +"string-natural-compare@npm:3.0.1": + version: 3.0.1 + resolution: "string-natural-compare@npm:3.0.1" + checksum: 85a6a9195736be500af5d817c7ea36b7e1ac278af079a807f70f79a56602359ee6743ca409af6291b94557de550ff60d1ec31b3c4fc8e7a08d0e12cdab57c149 + languageName: node + linkType: hard + "string-width-cjs@npm:string-width@^4.2.0, string-width@npm:^1.0.2 || 2 || 3 || 4, string-width@npm:^4.1.0, string-width@npm:^4.2.0, string-width@npm:^4.2.3": version: 4.2.3 resolution: "string-width@npm:4.2.3" @@ -26928,6 +27037,17 @@ __metadata: languageName: node linkType: hard +"super-regex@npm:^0.2.0": + version: 0.2.0 + resolution: "super-regex@npm:0.2.0" + dependencies: + clone-regexp: "npm:^3.0.0" + function-timeout: "npm:^0.1.0" + time-span: "npm:^5.1.0" + checksum: a8ff56aa521530df098433692c0a4c92353dd2fa3f7187785d654268031cc2876c8acc91e90bbd7b6b1ebabd68fb306f4fa61fc46f9fe0db04ed5f960fc2afc8 + languageName: node + linkType: hard + "supports-color@npm:^5.0.0, supports-color@npm:^5.3.0": version: 5.5.0 resolution: "supports-color@npm:5.5.0" @@ -27406,6 +27526,15 @@ __metadata: languageName: node linkType: hard +"time-span@npm:^5.1.0": + version: 5.1.0 + resolution: "time-span@npm:5.1.0" + dependencies: + convert-hrtime: "npm:^5.0.0" + checksum: 37b8284c53f4ee320377512ac19e3a034f2b025f5abd6959b8c1d0f69e0f06ab03681df209f2e452d30129e7b1f25bf573fb0f29d57e71f9b4a6b5b99f4c4b9e + languageName: node + linkType: hard + "timers-browserify@npm:^2.0.12": version: 2.0.12 resolution: "timers-browserify@npm:2.0.12" From 25a0d16be578e22e3534d05be319755abeaea781 Mon Sep 17 00:00:00 2001 From: fyodorio Date: Thu, 15 Feb 2024 10:46:53 +0500 Subject: [PATCH 2/7] change method for importing 'internal-ip' Replacing the static import with a dynamic one inside the 'beforeEach' hook should ensure that the new IP address utility is fully loaded before running the tests. --- .../core-server/src/utils/__tests__/server-address.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/code/lib/core-server/src/utils/__tests__/server-address.test.ts b/code/lib/core-server/src/utils/__tests__/server-address.test.ts index ad7ef8d5c265..ad57cb506b09 100644 --- a/code/lib/core-server/src/utils/__tests__/server-address.test.ts +++ b/code/lib/core-server/src/utils/__tests__/server-address.test.ts @@ -1,12 +1,13 @@ import { describe, beforeEach, it, expect, vi } from 'vitest'; -import internalIp from 'internal-ip'; import { getServerAddresses } from '../server-address'; vi.mock('internal-ip'); -const mockedInternalIp = vi.mocked(internalIp); +let mockedInternalIp; describe('getServerAddresses', () => { - beforeEach(() => { + beforeEach(async () => { + const importIp = await import('internal-ip'); + mockedInternalIp = vi.mocked(importIp); mockedInternalIp.internalIpV4Sync.mockReturnValue('192.168.0.5'); }); From 9c0d7a549fc63e72ddafb29ae21120dc61310692 Mon Sep 17 00:00:00 2001 From: fyodorio Date: Thu, 15 Feb 2024 14:03:10 +0500 Subject: [PATCH 3/7] refactor 'internal-ip' import method in 'getServerAddresses' The 'internal-ip' ESM module is now imported asynchronously within the 'getServerAddresses' function, rather than statically at the beginning of the script. That's necessary to comply with CommonJS output support flow. --- code/lib/core-server/src/utils/server-address.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/code/lib/core-server/src/utils/server-address.ts b/code/lib/core-server/src/utils/server-address.ts index c93ace9e017d..c8bc9a842d81 100644 --- a/code/lib/core-server/src/utils/server-address.ts +++ b/code/lib/core-server/src/utils/server-address.ts @@ -1,16 +1,20 @@ -import { internalIpV4Sync } from 'internal-ip'; +const logger = require('@storybook/node-logger'); +const detectFreePort = require('detect-port'); -import { logger } from '@storybook/node-logger'; -import detectFreePort from 'detect-port'; - -export function getServerAddresses( +export async function getServerAddresses( port: number, host: string | undefined, proto: string, initialPath?: string ) { const address = new URL(`${proto}://localhost:${port}/`); - const networkAddress = new URL(`${proto}://${host || internalIpV4Sync()}:${port}/`); + + // importing purely ESM 'internal-ip' package asynchronously to support CommonJS outputs + const internalIp = await import('internal-ip'); + + const networkAddress = new URL( + `${proto}://${host || internalIp.internalIpV4Sync()}:${port}/` + ); if (initialPath) { const searchParams = `?path=${decodeURIComponent( From ee79b17d450f3814e5c0e5cbbe0831b163e781fd Mon Sep 17 00:00:00 2001 From: fyodorio Date: Thu, 15 Feb 2024 14:34:00 +0500 Subject: [PATCH 4/7] update 'getServerAddresses' function consumers The 'getServerAddresses' function was updated to perform an asynchronous operation. That requires some tweaks in consuming code, including the tests. --- code/lib/core-server/src/dev-server.ts | 2 +- .../src/utils/__tests__/server-address.test.ts | 8 ++++---- .../lib/core-server/src/utils/server-address.test.ts | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/code/lib/core-server/src/dev-server.ts b/code/lib/core-server/src/dev-server.ts index f8805a75a690..86591caab1a6 100644 --- a/code/lib/core-server/src/dev-server.ts +++ b/code/lib/core-server/src/dev-server.ts @@ -61,7 +61,7 @@ export async function storybookDevServer(options: Options) { const { port, host, initialPath } = options; invariant(port, 'expected options to have a port'); const proto = options.https ? 'https' : 'http'; - const { address, networkAddress } = getServerAddresses(port, host, proto, initialPath); + const { address, networkAddress } = await getServerAddresses(port, host, proto, initialPath); const listening = new Promise((resolve, reject) => { // @ts-expect-error (Following line doesn't match TypeScript signature at all 🤔) diff --git a/code/lib/core-server/src/utils/__tests__/server-address.test.ts b/code/lib/core-server/src/utils/__tests__/server-address.test.ts index ad57cb506b09..62cd31f3c608 100644 --- a/code/lib/core-server/src/utils/__tests__/server-address.test.ts +++ b/code/lib/core-server/src/utils/__tests__/server-address.test.ts @@ -11,14 +11,14 @@ describe('getServerAddresses', () => { mockedInternalIp.internalIpV4Sync.mockReturnValue('192.168.0.5'); }); - it('builds addresses with a specified host', () => { - const { address, networkAddress } = getServerAddresses(9009, '192.168.89.89', 'http'); + it('builds addresses with a specified host', async () => { + const { address, networkAddress } = await getServerAddresses(9009, '192.168.89.89', 'http'); expect(address).toEqual('http://localhost:9009/'); expect(networkAddress).toEqual('http://192.168.89.89:9009/'); }); - it('builds addresses with local IP when host is not specified', () => { - const { address, networkAddress } = getServerAddresses(9009, '', 'http'); + it('builds addresses with local IP when host is not specified', async () => { + const { address, networkAddress } = await getServerAddresses(9009, '', 'http'); expect(address).toEqual('http://localhost:9009/'); expect(networkAddress).toEqual('http://192.168.0.5:9009/'); }); diff --git a/code/lib/core-server/src/utils/server-address.test.ts b/code/lib/core-server/src/utils/server-address.test.ts index d376373cbf78..eaf41ca1908c 100644 --- a/code/lib/core-server/src/utils/server-address.test.ts +++ b/code/lib/core-server/src/utils/server-address.test.ts @@ -11,35 +11,35 @@ describe('getServerAddresses', () => { const host = 'localhost'; const proto = 'http'; - it('should return server addresses without initial path by default', () => { + it('should return server addresses without initial path by default', async () => { const expectedAddress = `${proto}://localhost:${port}/`; const expectedNetworkAddress = `${proto}://${host}:${port}/`; - const result = getServerAddresses(port, host, proto); + const result = await getServerAddresses(port, host, proto); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); }); - it('should return server addresses with initial path', () => { + it('should return server addresses with initial path', async () => { const initialPath = '/foo/bar'; const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; const expectedNetworkAddress = `${proto}://${host}:${port}/?path=/foo/bar`; - const result = getServerAddresses(port, host, proto, initialPath); + const result = await getServerAddresses(port, host, proto, initialPath); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); }); - it('should return server addresses with initial path and add slash if missing', () => { + it('should return server addresses with initial path and add slash if missing', async () => { const initialPath = 'foo/bar'; const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; const expectedNetworkAddress = `${proto}://${host}:${port}/?path=/foo/bar`; - const result = getServerAddresses(port, host, proto, initialPath); + const result = await getServerAddresses(port, host, proto, initialPath); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); From d220ec5dd7f6ac9497abb6e8b6d1ce4f483ad488 Mon Sep 17 00:00:00 2001 From: fyodorio Date: Thu, 15 Feb 2024 15:10:54 +0500 Subject: [PATCH 5/7] add error handling to 'getServerAddresses' function call --- code/lib/core-server/src/dev-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/core-server/src/dev-server.ts b/code/lib/core-server/src/dev-server.ts index 86591caab1a6..a0c84c4c1186 100644 --- a/code/lib/core-server/src/dev-server.ts +++ b/code/lib/core-server/src/dev-server.ts @@ -61,7 +61,7 @@ export async function storybookDevServer(options: Options) { const { port, host, initialPath } = options; invariant(port, 'expected options to have a port'); const proto = options.https ? 'https' : 'http'; - const { address, networkAddress } = await getServerAddresses(port, host, proto, initialPath); + const { address, networkAddress } = await getServerAddresses(port, host, proto, initialPath).catch(); const listening = new Promise((resolve, reject) => { // @ts-expect-error (Following line doesn't match TypeScript signature at all 🤔) From b2490485b6614aba195636fb51c39405174b4b42 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 15 Feb 2024 11:11:28 +0100 Subject: [PATCH 6/7] Fix getServerAddress usage --- code/lib/core-server/src/dev-server.ts | 2 +- .../utils/__tests__/server-address.test.ts | 20 ++++++----- .../src/utils/server-address.test.ts | 36 ++++++++++++++----- .../core-server/src/utils/server-address.ts | 10 +++--- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/code/lib/core-server/src/dev-server.ts b/code/lib/core-server/src/dev-server.ts index f8805a75a690..86591caab1a6 100644 --- a/code/lib/core-server/src/dev-server.ts +++ b/code/lib/core-server/src/dev-server.ts @@ -61,7 +61,7 @@ export async function storybookDevServer(options: Options) { const { port, host, initialPath } = options; invariant(port, 'expected options to have a port'); const proto = options.https ? 'https' : 'http'; - const { address, networkAddress } = getServerAddresses(port, host, proto, initialPath); + const { address, networkAddress } = await getServerAddresses(port, host, proto, initialPath); const listening = new Promise((resolve, reject) => { // @ts-expect-error (Following line doesn't match TypeScript signature at all 🤔) diff --git a/code/lib/core-server/src/utils/__tests__/server-address.test.ts b/code/lib/core-server/src/utils/__tests__/server-address.test.ts index ad57cb506b09..17c508b0fd15 100644 --- a/code/lib/core-server/src/utils/__tests__/server-address.test.ts +++ b/code/lib/core-server/src/utils/__tests__/server-address.test.ts @@ -1,24 +1,26 @@ import { describe, beforeEach, it, expect, vi } from 'vitest'; import { getServerAddresses } from '../server-address'; +import internalIP from 'internal-ip'; -vi.mock('internal-ip'); -let mockedInternalIp; +vi.mock('internal-ip', () => ({ + default: { + internalIpV4Sync: vi.fn(), + }, +})); describe('getServerAddresses', () => { beforeEach(async () => { - const importIp = await import('internal-ip'); - mockedInternalIp = vi.mocked(importIp); - mockedInternalIp.internalIpV4Sync.mockReturnValue('192.168.0.5'); + vi.mocked(internalIP).internalIpV4Sync.mockReturnValue('192.168.0.5'); }); - it('builds addresses with a specified host', () => { - const { address, networkAddress } = getServerAddresses(9009, '192.168.89.89', 'http'); + it('builds addresses with a specified host', async () => { + const { address, networkAddress } = await getServerAddresses(9009, '192.168.89.89', 'http'); expect(address).toEqual('http://localhost:9009/'); expect(networkAddress).toEqual('http://192.168.89.89:9009/'); }); - it('builds addresses with local IP when host is not specified', () => { - const { address, networkAddress } = getServerAddresses(9009, '', 'http'); + it('builds addresses with local IP when host is not specified', async () => { + const { address, networkAddress } = await getServerAddresses(9009, '', 'http'); expect(address).toEqual('http://localhost:9009/'); expect(networkAddress).toEqual('http://192.168.0.5:9009/'); }); diff --git a/code/lib/core-server/src/utils/server-address.test.ts b/code/lib/core-server/src/utils/server-address.test.ts index d376373cbf78..2139e0741ef7 100644 --- a/code/lib/core-server/src/utils/server-address.test.ts +++ b/code/lib/core-server/src/utils/server-address.test.ts @@ -1,8 +1,13 @@ import { describe, it, expect, vi } from 'vitest'; -import detectPort from 'detect-port'; import { getServerAddresses, getServerPort, getServerChannelUrl } from './server-address'; +import detectPort from 'detect-port'; +import internalIP from 'internal-ip'; -vi.mock('internal-ip'); +vi.mock('internal-ip', () => ({ + default: { + internalIpV4Sync: vi.fn(), + }, +})); vi.mock('detect-port'); vi.mock('@storybook/node-logger'); @@ -11,35 +16,50 @@ describe('getServerAddresses', () => { const host = 'localhost'; const proto = 'http'; - it('should return server addresses without initial path by default', () => { + it('should return server addresses without initial path by default', async () => { const expectedAddress = `${proto}://localhost:${port}/`; const expectedNetworkAddress = `${proto}://${host}:${port}/`; - const result = getServerAddresses(port, host, proto); + const result = await getServerAddresses(port, host, proto); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); }); - it('should return server addresses with initial path', () => { + it('should return server addresses with initial path', async () => { const initialPath = '/foo/bar'; const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; const expectedNetworkAddress = `${proto}://${host}:${port}/?path=/foo/bar`; - const result = getServerAddresses(port, host, proto, initialPath); + const result = await getServerAddresses(port, host, proto, initialPath); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); }); - it('should return server addresses with initial path and add slash if missing', () => { + it('should return server addresses with initial path and add slash if missing', async () => { const initialPath = 'foo/bar'; const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; const expectedNetworkAddress = `${proto}://${host}:${port}/?path=/foo/bar`; - const result = getServerAddresses(port, host, proto, initialPath); + const result = await getServerAddresses(port, host, proto, initialPath); + + expect(result.address).toBe(expectedAddress); + expect(result.networkAddress).toBe(expectedNetworkAddress); + }); + + it('should return the internal ip if host is not specified', async () => { + const initialPath = 'foo/bar'; + const mockedNetworkIP = '192.168.0.5'; + + const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; + const expectedNetworkAddress = `${proto}://${mockedNetworkIP}:${port}/?path=/foo/bar`; + + vi.mocked(internalIP).internalIpV4Sync.mockReturnValue('192.168.0.5'); + + const result = await getServerAddresses(port, undefined, proto, initialPath); expect(result.address).toBe(expectedAddress); expect(result.networkAddress).toBe(expectedNetworkAddress); diff --git a/code/lib/core-server/src/utils/server-address.ts b/code/lib/core-server/src/utils/server-address.ts index c8bc9a842d81..5e7873f8977a 100644 --- a/code/lib/core-server/src/utils/server-address.ts +++ b/code/lib/core-server/src/utils/server-address.ts @@ -1,5 +1,5 @@ -const logger = require('@storybook/node-logger'); -const detectFreePort = require('detect-port'); +import { logger } from '@storybook/node-logger'; +import detectFreePort from 'detect-port'; export async function getServerAddresses( port: number, @@ -9,11 +9,9 @@ export async function getServerAddresses( ) { const address = new URL(`${proto}://localhost:${port}/`); - // importing purely ESM 'internal-ip' package asynchronously to support CommonJS outputs - const internalIp = await import('internal-ip'); - const networkAddress = new URL( - `${proto}://${host || internalIp.internalIpV4Sync()}:${port}/` + // importing purely ESM 'internal-ip' package asynchronously to support CommonJS outputs + `${proto}://${host || (await import('internal-ip')).default.internalIpV4Sync()}:${port}/` ); if (initialPath) { From 9ddb0c816e343ea34ddcf3a03e9501b6c45f4d48 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 15 Feb 2024 12:01:23 +0100 Subject: [PATCH 7/7] Do not use default export from internal-ip while dynamically requiring --- .../core-server/src/utils/__tests__/server-address.test.ts | 7 ++----- code/lib/core-server/src/utils/server-address.test.ts | 7 ++----- code/lib/core-server/src/utils/server-address.ts | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/code/lib/core-server/src/utils/__tests__/server-address.test.ts b/code/lib/core-server/src/utils/__tests__/server-address.test.ts index 17c508b0fd15..aa911b6858c8 100644 --- a/code/lib/core-server/src/utils/__tests__/server-address.test.ts +++ b/code/lib/core-server/src/utils/__tests__/server-address.test.ts @@ -1,16 +1,13 @@ import { describe, beforeEach, it, expect, vi } from 'vitest'; import { getServerAddresses } from '../server-address'; -import internalIP from 'internal-ip'; vi.mock('internal-ip', () => ({ - default: { - internalIpV4Sync: vi.fn(), - }, + internalIpV4Sync: vi.fn(), })); describe('getServerAddresses', () => { beforeEach(async () => { - vi.mocked(internalIP).internalIpV4Sync.mockReturnValue('192.168.0.5'); + vi.mocked(await import('internal-ip')).internalIpV4Sync.mockReturnValue('192.168.0.5'); }); it('builds addresses with a specified host', async () => { diff --git a/code/lib/core-server/src/utils/server-address.test.ts b/code/lib/core-server/src/utils/server-address.test.ts index 2139e0741ef7..96cc9a37b80b 100644 --- a/code/lib/core-server/src/utils/server-address.test.ts +++ b/code/lib/core-server/src/utils/server-address.test.ts @@ -1,12 +1,9 @@ import { describe, it, expect, vi } from 'vitest'; import { getServerAddresses, getServerPort, getServerChannelUrl } from './server-address'; import detectPort from 'detect-port'; -import internalIP from 'internal-ip'; vi.mock('internal-ip', () => ({ - default: { - internalIpV4Sync: vi.fn(), - }, + internalIpV4Sync: vi.fn(), })); vi.mock('detect-port'); vi.mock('@storybook/node-logger'); @@ -57,7 +54,7 @@ describe('getServerAddresses', () => { const expectedAddress = `${proto}://localhost:${port}/?path=/foo/bar`; const expectedNetworkAddress = `${proto}://${mockedNetworkIP}:${port}/?path=/foo/bar`; - vi.mocked(internalIP).internalIpV4Sync.mockReturnValue('192.168.0.5'); + vi.mocked(await import('internal-ip')).internalIpV4Sync.mockReturnValue('192.168.0.5'); const result = await getServerAddresses(port, undefined, proto, initialPath); diff --git a/code/lib/core-server/src/utils/server-address.ts b/code/lib/core-server/src/utils/server-address.ts index 5e7873f8977a..ca3f9504f9ec 100644 --- a/code/lib/core-server/src/utils/server-address.ts +++ b/code/lib/core-server/src/utils/server-address.ts @@ -11,7 +11,7 @@ export async function getServerAddresses( const networkAddress = new URL( // importing purely ESM 'internal-ip' package asynchronously to support CommonJS outputs - `${proto}://${host || (await import('internal-ip')).default.internalIpV4Sync()}:${port}/` + `${proto}://${host || (await import('internal-ip')).internalIpV4Sync()}:${port}/` ); if (initialPath) {