From 940a16e9c1875c89061e3f12dd2b006cd32140e7 Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Wed, 16 Mar 2022 14:18:11 +1300 Subject: [PATCH 1/3] Fixes #2580 - parse port to number type if provided in a different type --- src/packages/core/src/server.ts | 19 +++++++--- src/packages/core/tests/server.test.ts | 49 +++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/packages/core/src/server.ts b/src/packages/core/src/server.ts index 67542c11b4..3280778996 100644 --- a/src/packages/core/src/server.ts +++ b/src/packages/core/src/server.ts @@ -185,6 +185,17 @@ export class Server< host = null; } const callbackIsFunction = typeof callback === "function"; + + // Method signature specifies port: number, but we parse a non-number if provided + // Matches the behaviour of http.server.listen https://www.w3schools.com/nodejs/met_server_listen.asp + if (isNaN(port)) { + const err = new Error(`Provided port is not a valid value: ${port}.`); + return callbackIsFunction + ? process.nextTick(callback!, err) + : Promise.reject(err); + } + const portNumber = typeof port === "number" ? port : parseInt(port); + const status = this.#status; if (status === ServerStatus.closing) { // if closing @@ -195,7 +206,7 @@ export class Server< } else if ((status & ServerStatus.openingOrOpen) !== 0) { // if opening or open const err = new Error( - `Server is already open, or is opening, on port: ${port}.` + `Server is already open, or is opening, on port: ${portNumber}.` ); return callbackIsFunction ? process.nextTick(callback!, err) @@ -214,11 +225,11 @@ export class Server< host ? (this.#app as any).listen( host, - port, + portNumber, LIBUS_LISTEN_EXCLUSIVE_PORT, resolve ) - : this.#app.listen(port, LIBUS_LISTEN_EXCLUSIVE_PORT, resolve); + : this.#app.listen(portNumber, LIBUS_LISTEN_EXCLUSIVE_PORT, resolve); } ).then(listenSocket => { if (listenSocket) { @@ -228,7 +239,7 @@ export class Server< this.#status = ServerStatus.closed; const err = new Error( `listen EADDRINUSE: address already in use ${host || DEFAULT_HOST - }:${port}.` + }:${portNumber}.` ); throw err; } diff --git a/src/packages/core/tests/server.test.ts b/src/packages/core/tests/server.test.ts index 86c3f97713..1493ad43d4 100644 --- a/src/packages/core/tests/server.test.ts +++ b/src/packages/core/tests/server.test.ts @@ -70,10 +70,10 @@ describe("server", () => { (process.env.GITHUB_ACTION ? describe : describe.skip)("listen", function () { /** * Sends a post request to the server and returns the response. - * @param address - * @param port - * @param json - * @returns + * @param address + * @param port + * @param json + * @returns */ function post(host: string, port: number, json: any) { const data = JSON.stringify(json); @@ -299,6 +299,47 @@ describe("server", () => { }); }); + it("accepts port as string type (decimal)", async () => { + const portString = "5432"; + + s = Ganache.server(defaultOptions); + await s.listen(portString); + + try { + const req = request.post("http://localhost:5432"); + await req.send(jsonRpcJson); + } finally { + await teardown(); + } + }); + + it("accepts port as string type (hexidecimal)", async () => { + const portString = "0x1538"; + + s = Ganache.server(defaultOptions); + await s.listen(portString); + + try { + const req = request.post("http://localhost:5432"); + await req.send(jsonRpcJson); + } finally { + await teardown(); + } + }); + + it("fails with non-numeric port", async () => { + const portString = "not a number"; + + s = Ganache.server(defaultOptions); + try { + await assert.rejects(s.listen(portString), { + message: `Provided port is not a valid value: ${portString}.` + }); + } finally { + await teardown(); + } + }); + it("fails to `.listen()` twice, Promise", async () => { await setup(); try { From e523a376ca8685038113ed517dcc2f54ec2d59eb Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Thu, 17 Mar 2022 15:44:04 +1300 Subject: [PATCH 2/3] Fixes for #2610 * Improved validation and parsing of port value - inspiration from nodejs internal port validator * Wider range of postive and negative test values, including octal and binary * Fixed spelling of hexadecimal --- src/packages/core/src/server.ts | 16 ++++--- src/packages/core/tests/server.test.ts | 62 +++++++++++++------------- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/packages/core/src/server.ts b/src/packages/core/src/server.ts index 3280778996..9daaa566dc 100644 --- a/src/packages/core/src/server.ts +++ b/src/packages/core/src/server.ts @@ -186,15 +186,21 @@ export class Server< } const callbackIsFunction = typeof callback === "function"; - // Method signature specifies port: number, but we parse a non-number if provided - // Matches the behaviour of http.server.listen https://www.w3schools.com/nodejs/met_server_listen.asp - if (isNaN(port)) { - const err = new Error(`Provided port is not a valid value: ${port}.`); + // Method signature specifies port: number, but we parse a string if provided + // inspiration taken from nodejs internal port validator + // https://github.com/nodejs/node/blob/8c4b8b201ada6b76d5306c9c7f352e45087fb4a9/lib/internal/validators.js#L208-L219 + if ((typeof port !== 'number' && typeof port !== 'string') || + (typeof port === 'string' && String.prototype.trim.apply(port).length === 0) || + +port !== (+port >>> 0) || + port > 0xFFFF || + port === 0) { + const err = new Error(`Port should be >= 0 and < 65536. Received ${port}.`); + return callbackIsFunction ? process.nextTick(callback!, err) : Promise.reject(err); } - const portNumber = typeof port === "number" ? port : parseInt(port); + const portNumber = +port; const status = this.#status; if (status === ServerStatus.closing) { diff --git a/src/packages/core/tests/server.test.ts b/src/packages/core/tests/server.test.ts index 1493ad43d4..a75923b2c5 100644 --- a/src/packages/core/tests/server.test.ts +++ b/src/packages/core/tests/server.test.ts @@ -299,44 +299,44 @@ describe("server", () => { }); }); - it("accepts port as string type (decimal)", async () => { - const portString = "5432"; + it("accepts port as number type or binary, octal, decimal or hexadecimal string", async () => { + const validPorts = [ + port, `0b${port.toString(2)}`, + `0o${port.toString(8)}`, port.toString(10), + `0x${port.toString(16)}` + ]; - s = Ganache.server(defaultOptions); - await s.listen(portString); + for (const specificPort of validPorts) { + s = Ganache.server(defaultOptions); + await s.listen(specificPort); - try { - const req = request.post("http://localhost:5432"); - await req.send(jsonRpcJson); - } finally { - await teardown(); + try { + const req = request.post(`http://localhost:${+specificPort}`); + await req.send(jsonRpcJson); + } finally { + await teardown(); + } } }); - it("accepts port as string type (hexidecimal)", async () => { - const portString = "0x1538"; - - s = Ganache.server(defaultOptions); - await s.listen(portString); - - try { - const req = request.post("http://localhost:5432"); - await req.send(jsonRpcJson); - } finally { - await teardown(); - } - }); + it("fails with invalid ports", async () => { + const invalidPorts = [ + -1, 'a', {}, [], false, true, + 0xFFFF + 1, Infinity, -Infinity, NaN, + undefined, null, '', ' ', 1.1, '0x', + '-0x1', '-0o1', '-0b1', '0o', '0b', 0 + ]; - it("fails with non-numeric port", async () => { - const portString = "not a number"; + for (const specificPort of invalidPorts) { + s = Ganache.server(defaultOptions); - s = Ganache.server(defaultOptions); - try { - await assert.rejects(s.listen(portString), { - message: `Provided port is not a valid value: ${portString}.` - }); - } finally { - await teardown(); + try { + await assert.rejects(s.listen(specificPort), { + message: `Port should be >= 0 and < 65536. Received ${specificPort}.` + }); + } finally { + await teardown(); + } } }); From c5f90bff5e4d79caaed6f5be3c1e0a5e431376e6 Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Fri, 18 Mar 2022 10:09:46 +1300 Subject: [PATCH 3/3] fix: #2610 replace string.prototype.trim.apply with string.trim --- src/packages/core/src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packages/core/src/server.ts b/src/packages/core/src/server.ts index 9daaa566dc..9c2a7139c3 100644 --- a/src/packages/core/src/server.ts +++ b/src/packages/core/src/server.ts @@ -190,7 +190,7 @@ export class Server< // inspiration taken from nodejs internal port validator // https://github.com/nodejs/node/blob/8c4b8b201ada6b76d5306c9c7f352e45087fb4a9/lib/internal/validators.js#L208-L219 if ((typeof port !== 'number' && typeof port !== 'string') || - (typeof port === 'string' && String.prototype.trim.apply(port).length === 0) || + (typeof port === 'string' && (port).trim().length === 0) || +port !== (+port >>> 0) || port > 0xFFFF || port === 0) {