Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use the host in options to check if port is available #4385

Merged
merged 1 commit into from Apr 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/Server.js
Expand Up @@ -385,9 +385,10 @@ class Server {

/**
* @param {Port} port
* @param {string} host
* @returns {Promise<number | string>}
*/
static async getFreePort(port) {
static async getFreePort(port, host) {
if (typeof port !== "undefined" && port !== null && port !== "auto") {
return port;
}
Expand All @@ -406,7 +407,7 @@ class Server {
? parseInt(process.env.WEBPACK_DEV_SERVER_PORT_RETRY, 10)
: 3;

return pRetry(() => getPort(basePort), {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure retry is going to help a lot as the getPort() (and portfinder) before is going to try every port up to the maximum port number anyway, but it was introduced on purpose in this commit back in 2019, so there might be a reason.

return pRetry(() => getPort(basePort, host), {
retries: defaultPortRetry,
});
}
Expand Down Expand Up @@ -3200,7 +3201,8 @@ class Server {
/** @type {Host} */ (this.options.host)
);
this.options.port = await Server.getFreePort(
/** @type {Port} */ (this.options.port)
/** @type {Port} */ (this.options.port),
this.options.host
);
}

Expand Down
15 changes: 13 additions & 2 deletions lib/getPort.js
Expand Up @@ -88,15 +88,26 @@ const getAvailablePort = async (port, hosts) => {

/**
* @param {number} basePort
* @param {string=} host
* @return {Promise<number>}
*/
async function getPorts(basePort) {
async function getPorts(basePort, host) {
if (basePort < minPort || basePort > maxPort) {
throw new Error(`Port number must lie between ${minPort} and ${maxPort}`);
}

let port = basePort;
const hosts = getLocalHosts();
const localhosts = getLocalHosts();
let hosts;
if (host && !localhosts.has(host)) {
hosts = new Set([host]);
} else {
/* If the host is equivalent to localhost
we need to check every equivalent host
else the port might falsely appear as available
on some operating systems */
hosts = localhosts;
}
/** @type {Set<string | undefined>} */
const portUnavailableErrors = new Set(["EADDRINUSE", "EACCES"]);
while (port <= maxPort) {
Expand Down
19 changes: 19 additions & 0 deletions test/server/get-port.test.js
Expand Up @@ -16,6 +16,7 @@ it("should pick the next port if the preferred port is unavailable", async () =>
server.unref();
await util.promisify(server.listen.bind(server))(preferredPort);
const port = await getPort(preferredPort);
server.close();
expect(port).toBe(preferredPort + 1);
});

Expand All @@ -34,3 +35,21 @@ it("should reject too high port numbers", async () => {
expect(e.message).toBeDefined();
}
});

describe("when passing a host", () => {
it("should bind to the preferred port", async () => {
const preferredPort = 8080;
const port = await getPort(8080, "127.0.0.1");
expect(port).toBe(preferredPort);
});

it("should pick the next port if the preferred port is unavailable", async () => {
const preferredPort = 8345;
const server = net.createServer();
server.unref();
await util.promisify(server.listen.bind(server))(preferredPort);
const port = await getPort(preferredPort, "0.0.0.0");
server.close();
expect(port).toBe(preferredPort + 1);
});
});
70 changes: 38 additions & 32 deletions types/lib/Server.d.ts
Expand Up @@ -664,6 +664,7 @@ declare class Server {
};
/**
* @param {Port} port
* @param {string} host
* @returns {Promise<number | string>}
*/
https: {
Expand Down Expand Up @@ -716,9 +717,9 @@ declare class Server {
description: string;
multiple: boolean;
path: string;
type: string;
type: string /** @type {WebSocketURL} */;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regenerating the Server types introduces a lot unrelated comments in the type definitions, but running build:types on master does not create any diff.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, types changed here since you added * @param {string} host

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is bug in ts, reported about it, but still not fixed

}[];
/** @type {ClientConfiguration} */ description: string;
description: string;
multiple: boolean;
simpleType: string;
};
Expand All @@ -730,7 +731,7 @@ declare class Server {
path: string;
}[];
description: string;
simpleType: string;
/** @type {ServerConfiguration} */ simpleType: string;
multiple: boolean;
};
"https-cert-reset": {
Expand Down Expand Up @@ -881,7 +882,7 @@ declare class Server {
configs: (
| {
type: string;
/** @type {MultiCompiler} */ multiple: boolean;
multiple: boolean;
description: string;
path: string;
}
Expand All @@ -905,6 +906,10 @@ declare class Server {
path: string;
}[];
description: string;
/**
* @private
* @returns {Promise<void>}
*/
simpleType: string;
multiple: boolean;
};
Expand Down Expand Up @@ -952,6 +957,10 @@ declare class Server {
simpleType: string;
multiple: boolean;
};
/**
* @param {string | Static | undefined} [optionsForStatic]
* @returns {NormalizedStatic}
*/
"open-target-reset": {
configs: {
type: string;
Expand Down Expand Up @@ -1118,7 +1127,7 @@ declare class Server {
"server-options-pfx-reset": {
configs: {
description: string;
multiple: boolean;
/** @type {ServerConfiguration} */ multiple: boolean;
path: string;
type: string;
}[];
Expand All @@ -1131,7 +1140,7 @@ declare class Server {
description: string;
negatedDescription: string;
multiple: boolean;
/** @type {ServerOptions} */ path: string;
path: string;
type: string;
}[];
description: string;
Expand All @@ -1144,10 +1153,10 @@ declare class Server {
multiple: boolean;
path: string;
type: string;
values: string[];
/** @type {ServerOptions} */ values: string[];
}[];
description: string;
multiple: boolean;
/** @type {Array<keyof ServerOptions>} */ multiple: boolean;
simpleType: string;
};
static: {
Expand All @@ -1167,7 +1176,7 @@ declare class Server {
}
)[];
description: string;
simpleType: string;
/** @type {ServerOptions} */ simpleType: string;
multiple: boolean;
};
"static-directory": {
Expand Down Expand Up @@ -1279,7 +1288,7 @@ declare class Server {
}
| {
description: string;
multiple: boolean;
/** @type {ServerOptions & { cacert?: ServerOptions["ca"] }} */ multiple: boolean;
path: string;
type: string;
}
Expand All @@ -1292,6 +1301,7 @@ declare class Server {
configs: (
| {
description: string;
/** @type {ServerOptions & { cacert?: ServerOptions["ca"] }} */
multiple: boolean;
path: string;
type: string;
Expand All @@ -1300,13 +1310,13 @@ declare class Server {
| {
description: string;
multiple: boolean;
/** @type {ServerOptions} */ path: string;
type: string;
path: string;
/** @type {ServerOptions} */ type: string;
}
)[];
description: string;
simpleType: string;
multiple: boolean;
/** @type {ServerOptions} */ multiple: boolean;
};
};
readonly processArguments: (
Expand Down Expand Up @@ -2033,6 +2043,9 @@ declare class Server {
instanceof?: undefined;
}
)[];
/**
* @returns {string}
*/
};
instanceof?: undefined;
}
Expand Down Expand Up @@ -2073,6 +2086,9 @@ declare class Server {
exclude: boolean;
};
};
/**
* @type {string[]}
*/
Headers: {
anyOf: (
| {
Expand Down Expand Up @@ -2111,7 +2127,7 @@ declare class Server {
}
| {
type: string;
description: string;
/** @type {ClientConfiguration} */ description: string;
link: string;
cli?: undefined /** @typedef {import("express").Request} Request */;
}
Expand All @@ -2122,7 +2138,6 @@ declare class Server {
Host: {
description: string;
link: string;
/** @type {ServerConfiguration} */
anyOf: (
| {
enum: string[];
Expand Down Expand Up @@ -2152,7 +2167,7 @@ declare class Server {
}
)[];
description: string;
link: string;
/** @type {string} */ link: string;
};
IPC: {
anyOf: (
Expand Down Expand Up @@ -2261,12 +2276,6 @@ declare class Server {
minLength: number;
};
minItems: number;
/**
* prependEntry Method for webpack 4
* @param {any} originalEntry
* @param {any} newAdditionalEntries
* @returns {any}
*/
minLength?: undefined;
}
| {
Expand Down Expand Up @@ -2317,6 +2326,7 @@ declare class Server {
enum?: undefined;
}
| {
/** @type {any} */
type: string;
minLength: number;
minimum?: undefined;
Expand Down Expand Up @@ -2372,7 +2382,7 @@ declare class Server {
ServerEnum: {
enum: string[];
cli: {
exclude: boolean;
exclude: boolean /** @type {MultiCompiler} */;
};
};
ServerString: {
Expand Down Expand Up @@ -2403,10 +2413,6 @@ declare class Server {
passphrase: {
type: string;
description: string;
/**
* @private
* @returns {Promise<void>}
*/
};
requestCert: {
type: string;
Expand Down Expand Up @@ -2776,7 +2782,7 @@ declare class Server {
| {
type: string;
minLength: number;
items?: undefined;
/** @type {ServerConfiguration} */ items?: undefined;
}
)[];
description: string;
Expand All @@ -2798,8 +2804,8 @@ declare class Server {
anyOf: {
$ref: string;
}[];
description: string;
/** @type {Array<keyof ServerOptions>} */ link: string;
/** @type {ServerOptions} */ description: string;
link: string;
};
WebSocketServerType: {
enum: string[];
Expand Down Expand Up @@ -2857,7 +2863,6 @@ declare class Server {
bonjour: {
$ref: string;
};
/** @type {any} */
client: {
$ref: string;
};
Expand Down Expand Up @@ -2959,9 +2964,10 @@ declare class Server {
static getHostname(hostname: Host): Promise<string>;
/**
* @param {Port} port
* @param {string} host
* @returns {Promise<number | string>}
*/
static getFreePort(port: Port): Promise<number | string>;
static getFreePort(port: Port, host: string): Promise<number | string>;
/**
* @returns {string}
*/
Expand Down
6 changes: 5 additions & 1 deletion types/lib/getPort.d.ts
@@ -1,6 +1,10 @@
export = getPorts;
/**
* @param {number} basePort
* @param {string=} host
* @return {Promise<number>}
*/
declare function getPorts(basePort: number): Promise<number>;
declare function getPorts(
basePort: number,
host?: string | undefined
): Promise<number>;