diff --git a/package-lock.json b/package-lock.json index 7038d050efa..23d371bd9a6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5912,15 +5912,6 @@ "dev": true, "license": "MIT" }, - "node_modules/async": { - "version": "2.6.4", - "resolved": "https://registry.npmjs.org/async/-/async-2.6.4.tgz", - "integrity": "sha512-mzo5dfJYwAn29PeiJ0zvwTo04zj8HDJj0Mn8TD7sno7q12prdbnasKJHhkm2c1LgrhlJ0teaea8860oxi51mGA==", - "license": "MIT", - "dependencies": { - "lodash": "^4.17.14" - } - }, "node_modules/async-function": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/async-function/-/async-function-1.0.0.tgz", @@ -11112,15 +11103,6 @@ "node": ">=0.12.0" } }, - "node_modules/is-number-like": { - "version": "1.0.8", - "resolved": "https://registry.npmjs.org/is-number-like/-/is-number-like-1.0.8.tgz", - "integrity": "sha512-6rZi3ezCyFcn5L71ywzz2bS5b2Igl1En3eTlZlvKjpz1n3IZLAYMbKYAIQgFmEu0GENg92ziU/faEOA/aixjbA==", - "license": "ISC", - "dependencies": { - "lodash.isfinite": "^3.3.2" - } - }, "node_modules/is-number-object": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/is-number-object/-/is-number-object-1.1.1.tgz", @@ -12371,12 +12353,6 @@ "dev": true, "license": "MIT" }, - "node_modules/lodash.isfinite": { - "version": "3.3.2", - "resolved": "https://registry.npmjs.org/lodash.isfinite/-/lodash.isfinite-3.3.2.tgz", - "integrity": "sha512-7FGG40uhC8Mm633uKW1r58aElFlBlxCrg9JfSi3P6aYiWmfiWF0PgMd86ZUsxE5GwWPdHoS2+48bwTh2VPkIQA==", - "license": "MIT" - }, "node_modules/log-update": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/log-update/-/log-update-8.0.0.tgz", @@ -14346,20 +14322,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/portscanner": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/portscanner/-/portscanner-2.2.0.tgz", - "integrity": "sha512-IFroCz/59Lqa2uBvzK3bKDbDDIEaAY8XJ1jFxcLWTqosrsc32//P4VuSB2vZXoHiHqOmx8B5L5hnKOxL/7FlPw==", - "license": "MIT", - "dependencies": { - "async": "^2.6.0", - "is-number-like": "^1.0.3" - }, - "engines": { - "node": ">=0.4", - "npm": ">=1.0.0" - } - }, "node_modules/possible-typed-array-names": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/possible-typed-array-names/-/possible-typed-array-names-1.1.0.tgz", @@ -19683,7 +19645,6 @@ "graceful-fs": "^4.2.11", "mime-types": "^3.0.2", "parseurl": "^1.3.3", - "portscanner": "^2.2.0", "router": "^2.2.0", "ws": "^8.21.3" }, diff --git a/packages/server/lib/serve/httpListener.js b/packages/server/lib/serve/httpListener.js index bafefbc6410..278ff03b32c 100644 --- a/packages/server/lib/serve/httpListener.js +++ b/packages/server/lib/serve/httpListener.js @@ -1,7 +1,7 @@ import os from "node:os"; +import net from "node:net"; import http from "node:http"; import https from "node:https"; -import portscanner from "portscanner"; /** * HTTP-listener helpers used by the {@link Supervisor}, which binds the port once and @@ -40,45 +40,103 @@ export function createServer({https: useHttps, key, cert}, requestHandler) { * @returns {Promise} Resolves with the bound port and the server instance * @private */ -export function listen(server, port, changePortIfInUse, acceptRemoteConnections) { - return new Promise(function(resolve, reject) { - const options = {}; +// Timeout (ms) for a single port probe. On localhost a port either accepts or refuses the +// connection immediately, so this only guards against a probe that hangs indefinitely. +const PORT_PROBE_TIMEOUT = 400; - if (!acceptRemoteConnections) { - // Unless remote connections are allowed, bind to the IPv4 loopback address - options.host = "127.0.0.1"; - } // If remote connections are allowed, do not set host so the server listens on all supported interfaces +/** + * Probes whether something is accepting TCP connections on the given host/port. + * + * Mirrors the connect-probe semantics of the previously used portscanner dependency: + * a successful connection means the port is in use; a refused connection or a timeout means it is + * free. Any other socket error (e.g. an unreachable host) is treated as a scan failure and rejects, + * so unexpected problems surface to the caller instead of being silently reported as "free". + * + * @param {string} host Host to probe + * @param {number} port Port to probe + * @returns {Promise} Resolves true if the port is in use, false if free + * @private + */ +function isPortInUse(host, port) { + return new Promise(function(resolve, reject) { + const socket = new net.Socket(); + const finish = function(settle, value) { + socket.removeAllListeners(); + socket.destroy(); + settle(value); + }; + socket.setTimeout(PORT_PROBE_TIMEOUT); + socket.once("connect", () => finish(resolve, true)); + socket.once("timeout", () => finish(resolve, false)); + socket.once("error", function(err) { + if (err.code === "ECONNREFUSED") { + finish(resolve, false); + } else { + finish(reject, err); + } + }); + socket.connect(port, host); + }); +} - const portScanHost = options.host || "127.0.0.1"; - const portMax = changePortIfInUse ? port + 30 : port; +/** + * Scans the inclusive port range [port, portMax] on the given host and returns the + * first port not in use, or null if every port in the range is taken. + * + * @param {number} port First port of the range + * @param {number} portMax Last port of the range (inclusive) + * @param {string} host Host to scan + * @returns {Promise} The first free port, or null if none is available + * @private + */ +async function findAPortNotInUse(port, portMax, host) { + for (let candidate = port; candidate <= portMax; candidate++) { + if (!await isPortInUse(host, candidate)) { + return candidate; + } + } + return null; +} - portscanner.findAPortNotInUse(port, portMax, portScanHost, function(error, foundPort) { - if (error) { - reject(error); - return; - } +export async function listen(server, port, changePortIfInUse, acceptRemoteConnections) { + // Unless remote connections are allowed, bind to the IPv4 loopback address. Otherwise leave + // host unset so the server listens on all supported interfaces. + const host = acceptRemoteConnections ? undefined : "127.0.0.1"; + const portScanHost = host ?? "127.0.0.1"; + const portMax = changePortIfInUse ? port + 30 : port; - if (!foundPort) { - const err = new Error(changePortIfInUse ? - `EADDRINUSE: Could not find available ports between ${port} and ${portMax}.` : - `EADDRINUSE: Port ${port} is already in use.`); - err.code = "EADDRINUSE"; - err.errno = "EADDRINUSE"; - err.address = portScanHost; - err.port = portMax; - reject(err); - return; - } + const foundPort = await findAPortNotInUse(port, portMax, portScanHost); + if (foundPort === null) { + const err = new Error(changePortIfInUse ? + `EADDRINUSE: Could not find available ports between ${port} and ${portMax}.` : + `EADDRINUSE: Port ${port} is already in use.`); + err.code = "EADDRINUSE"; + err.errno = "EADDRINUSE"; + err.address = portScanHost; + err.port = portMax; + throw err; + } - options.port = foundPort; - server.listen(options, function() { - resolve({port: options.port, server}); - }); + await listenOnce(server, {host, port: foundPort}); + return {port: foundPort, server}; +} - server.on("error", function(err) { - reject(err); - }); - }); +// server.listen signals success via a 'listening' event and failure via an 'error' event. +// Bridge both into a single promise, detaching the losing listener once one fires (the old code +// left the error listener attached on every successful bind). +function listenOnce(server, options) { + return new Promise(function(resolve, reject) { + const onError = function(err) { + server.removeListener("listening", onListening); + reject(err); + }; + const onListening = function() { + server.removeListener("error", onError); + resolve(); + }; + server.once("error", onError); + server.once("listening", onListening); + server.listen(options); }); } diff --git a/packages/server/package.json b/packages/server/package.json index 4a6c317e10a..39df02b9143 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -99,7 +99,6 @@ "graceful-fs": "^4.2.11", "mime-types": "^3.0.2", "parseurl": "^1.3.3", - "portscanner": "^2.2.0", "router": "^2.2.0", "ws": "^8.21.3" }, diff --git a/packages/server/test/lib/server/ports.js b/packages/server/test/lib/server/ports.js index 0dee288e112..02a0d623fdd 100644 --- a/packages/server/test/lib/server/ports.js +++ b/packages/server/test/lib/server/ports.js @@ -2,19 +2,10 @@ import test from "ava"; import supertest from "supertest"; import {serve} from "../../../lib/server.js"; import http from "node:http"; -import portscanner from "portscanner"; -import sinonGlobal from "sinon"; +import esmock from "esmock"; import {graphFromPackageDependencies} from "@ui5/project/graph"; import {isolatedUi5DataDir} from "../../utils/buildCacheIsolation.js"; -test.beforeEach((t) => { - t.context.sinon = sinonGlobal.createSandbox(); -}); - -test.afterEach.always((t) => { - t.context.sinon.restore(); -}); - test.serial("Start server - Port is already taken and an error occurs", async (t) => { t.plan(6); const port = 3360; @@ -102,31 +93,32 @@ test.serial("Start server together with node server - Port is already taken and server.close(); }); -test.serial("Start server - Port can not be determined and an error occurs", async (t) => { - const {sinon} = t.context; - - t.plan(2); - const portscannerFake = function(port, portMax, host, callback) { - return new Promise((resolve) => { - callback(new Error("testError"), false); - resolve(); - }); - }; - const portScannerStub = sinon.stub(portscanner, "findAPortNotInUse").callsFake(portscannerFake); - - const graph = await graphFromPackageDependencies({ - cwd: "./test/fixtures/application.a" - }); - const startServer = serve(graph, { - port: 3990, - changePortIfInUse: true, - ui5DataDir: isolatedUi5DataDir(t), +test.serial("listen - Port scan fails with a generic error", async (t) => { + t.plan(3); + // Simulate a non-ECONNREFUSED socket error during the port probe (e.g. an unreachable host): + // it must be treated as a scan failure and propagate unchanged rather than be reported as "free". + class FakeSocket { + setTimeout() {} + once(event, cb) { + this._handlers = this._handlers || {}; + this._handlers[event] = cb; + } + connect() { + const err = new Error("testError"); + err.code = "EHOSTUNREACH"; + queueMicrotask(() => this._handlers.error(err)); + } + removeAllListeners() {} + destroy() {} + } + const httpListener = await esmock("../../../lib/serve/httpListener.js", { + "node:net": {default: {Socket: FakeSocket}, Socket: FakeSocket}, }); - const error = await t.throwsAsync(startServer); - t.is(error.message, "testError", - "Server could not start, port is already taken and no other port is used."); - portScannerStub.restore(); + const server = httpListener.createServer({https: false}, (req, res) => res.end()); + const error = await t.throwsAsync(httpListener.listen(server, 3990, true, false)); + t.is(error.message, "testError", "Generic scan error is propagated unchanged"); + t.is(error.code, "EHOSTUNREACH", "Original error code is preserved"); });