diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts index db354ab7720..5b7f9a64db5 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts @@ -1,11 +1,11 @@ import {getProxyingWebServer} from './http-reverse-proxy.js' import {AbortController} from '@shopify/cli-kit/node/abort' -import {getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' import {describe, test, expect} from 'vitest' import fetch from 'node-fetch' import WebSocket, {WebSocketServer} from 'ws' import http from 'http' import https from 'https' +import net from 'net' const each = ['http', 'https'] as const @@ -17,29 +17,29 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { ? new http.Agent({keepAlive: false}) : new https.Agent({ca: localhostCert.cert, keepAlive: false}) - test('routes requests to the correct target based on path', {retry: 2}, async ({ports, servers}) => { - const response1 = await fetch(`${protocol}://localhost:${ports.proxyPort}/path1/test`, {agent}) + test('routes requests to the correct target based on path', {retry: 2}, async ({setup}) => { + const response1 = await fetch(`${protocol}://localhost:${setup.proxyPort}/path1/test`, {agent}) await expect(response1.text()).resolves.toBe('Response from target server 1') - const response2 = await fetch(`${protocol}://localhost:${ports.proxyPort}/path2/test`, {agent}) + const response2 = await fetch(`${protocol}://localhost:${setup.proxyPort}/path2/test`, {agent}) await expect(response2.text()).resolves.toBe('Response from target server 2') }) - test('routes requests to the default target when no matching path is found', {retry: 2}, async ({ports, servers}) => { - const response = await fetch(`${protocol}://localhost:${ports.proxyPort}/unknown/path`, {agent}) + test('routes requests to the default target when no matching path is found', {retry: 2}, async ({setup}) => { + const response = await fetch(`${protocol}://localhost:${setup.proxyPort}/unknown/path`, {agent}) await expect(response.text()).resolves.toBe('Response from target server 1') }) - test('handles websocket connections', {retry: 2}, async ({ports, servers}) => { + test('handles websocket connections', {retry: 2}, async ({setup}) => { return new Promise((resolve, reject) => { - const wss = new WebSocketServer({server: servers.targetServer1}) + const wss = new WebSocketServer({server: setup.targetServer1}) wss.on('connection', (ws) => { ws.on('message', (message) => { ws.send(`Echo: ${String(message)}`) }) }) - const ws = new WebSocket(`${wsProtocol}://localhost:${ports.proxyPort}/path1`, {agent}) + const ws = new WebSocket(`${wsProtocol}://localhost:${setup.proxyPort}/path1`, {agent}) ws.on('open', () => { ws.send('Hello, WebSocket!') @@ -55,28 +55,24 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { }) }) - test('closes the server when aborted', {retry: 2}, async ({ports, servers}) => { - servers.abortController.abort() + test('closes the server when aborted', {retry: 2}, async ({setup}) => { + setup.abortController.abort() // Try the assertion immediately, and if it fails, wait and retry try { - await expect(fetch(`${protocol}://localhost:${ports.proxyPort}/path1`, {agent})).rejects.toThrow() + await expect(fetch(`${protocol}://localhost:${setup.proxyPort}/path1`, {agent})).rejects.toThrow() // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { // If the assertion fails, wait a bit and try again await new Promise((resolve) => setTimeout(resolve, 10)) - await expect(fetch(`${protocol}://localhost:${ports.proxyPort}/path1`, {agent})).rejects.toThrow() + await expect(fetch(`${protocol}://localhost:${setup.proxyPort}/path1`, {agent})).rejects.toThrow() } }) }) function getTestReverseProxy(protocol: 'http' | 'https') { return test.extend<{ - ports: { + setup: { proxyPort: number - targetPort1: number - targetPort2: number - } - servers: { targetServer1: http.Server targetServer2: http.Server proxyServer: http.Server @@ -84,13 +80,7 @@ function getTestReverseProxy(protocol: 'http' | 'https') { } }>({ // eslint-disable-next-line no-empty-pattern - ports: async ({}, use) => { - const proxyPort = await getAvailableTCPPort() - const targetPort1 = await getAvailableTCPPort() - const targetPort2 = await getAvailableTCPPort() - await use({proxyPort, targetPort1, targetPort2}) - }, - servers: async ({ports}, use) => { + setup: async ({}, use) => { const targetServer1 = http.createServer((req, res) => { res.writeHead(200, {'Content-Type': 'text/plain'}) res.end('Response from target server 1') @@ -101,22 +91,30 @@ function getTestReverseProxy(protocol: 'http' | 'https') { res.end('Response from target server 2') }) - await new Promise((resolve) => targetServer1.listen(ports.targetPort1, 'localhost', resolve)) - await new Promise((resolve) => targetServer2.listen(ports.targetPort2, 'localhost', resolve)) + // Listen on port 0 to let the OS assign available ports, avoiding the + // TOCTOU race where getAvailableTCPPort() returns a port that gets + // claimed by another process before listen() is called. + await new Promise((resolve) => targetServer1.listen(0, 'localhost', resolve)) + await new Promise((resolve) => targetServer2.listen(0, 'localhost', resolve)) + + const targetPort1 = (targetServer1.address() as net.AddressInfo).port + const targetPort2 = (targetServer2.address() as net.AddressInfo).port const abortController = new AbortController() const {server: proxyServer} = await getProxyingWebServer( { - '/path1': `http://localhost:${ports.targetPort1}`, - '/path2': `http://localhost:${ports.targetPort2}`, - default: `http://localhost:${ports.targetPort1}`, + '/path1': `http://localhost:${targetPort1}`, + '/path2': `http://localhost:${targetPort2}`, + default: `http://localhost:${targetPort1}`, }, abortController.signal, protocol === 'https' ? localhostCert : undefined, ) - await new Promise((resolve) => proxyServer.listen(ports.proxyPort, 'localhost', resolve)) - await use({targetServer1, targetServer2, proxyServer, abortController}) + await new Promise((resolve) => proxyServer.listen(0, 'localhost', resolve)) + const proxyPort = (proxyServer.address() as net.AddressInfo).port + + await use({proxyPort, targetServer1, targetServer2, proxyServer, abortController}) proxyServer.closeAllConnections() await new Promise((resolve) => proxyServer.close(() => resolve()))