From 4ff41316adf4a52fcc37d9e19d5b286947e50bff Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Thu, 15 Aug 2024 20:06:03 +0100 Subject: [PATCH 1/5] replace url.parse with new URL --- src/lib/connect/index.ts | 48 +++++++++++++++++----------------------- src/lib/connect/ws.ts | 16 +++++++++++++- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/lib/connect/index.ts b/src/lib/connect/index.ts index a25863f78..0d271458e 100644 --- a/src/lib/connect/index.ts +++ b/src/lib/connect/index.ts @@ -18,24 +18,6 @@ const debug = _debug('mqttjs') let protocols: Record = null -/** - * Parse the auth attribute and merge username and password in the options object. - * - * @param {Object} [opts] option object - */ -function parseAuthOptions(opts: IClientOptions) { - let matches: RegExpMatchArray | null - if (opts.auth) { - matches = opts.auth.match(/^(.+):(.+)$/) - if (matches) { - opts.username = matches[1] - opts.password = matches[2] - } else { - opts.username = opts.auth - } - } -} - /** * connect - connect to an MQTT broker. */ @@ -56,21 +38,34 @@ function connect( // try to parse the broker url if (brokerUrl && typeof brokerUrl === 'string') { - // eslint-disable-next-line - const parsedUrl = url.parse(brokerUrl, true) + const parsedUrl = new URL(brokerUrl) const parsedOptions: Partial = {} if (parsedUrl.port != null) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore parsedOptions.port = Number(parsedUrl.port) } parsedOptions.host = parsedUrl.hostname - parsedOptions.query = parsedUrl.query as Record - parsedOptions.auth = parsedUrl.auth + parsedOptions.query = Object.fromEntries( + parsedUrl.searchParams, + ) as Record + + if (parsedUrl.username) { + parsedOptions.username = parsedUrl.username + parsedOptions.auth = parsedOptions.username // TODO: is auth still needed? + if (parsedUrl.password) { + parsedOptions.password = parsedUrl.password + parsedOptions.auth = `${parsedOptions.username}:${parsedOptions.password}` // TODO: is auth still needed? + } + } + parsedOptions.protocol = parsedUrl.protocol as MqttProtocol - parsedOptions.path = parsedUrl.path + parsedOptions.path = parsedUrl.pathname // TODO: See note below + // NOTE: new URL().pathname is not the same as url.parse().path. URL.pathname does not include the query string. + // To make it compatible with url.parse().path, we need to append the query string to the path. + // if (parsedUrl.search) { + // parsedOptions.path += parsedUrl.search + // } parsedOptions.protocol = parsedOptions.protocol?.replace( /:$/, @@ -99,9 +94,6 @@ function connect( delete opts.path } - // merge in the auth options if supplied - parseAuthOptions(opts) - // support clientId passed in the query string of the url if (opts.query && typeof opts.query.clientId === 'string') { opts.clientId = opts.query.clientId diff --git a/src/lib/connect/ws.ts b/src/lib/connect/ws.ts index 5be4bfce6..fe7efdda2 100644 --- a/src/lib/connect/ws.ts +++ b/src/lib/connect/ws.ts @@ -19,7 +19,21 @@ const WSS_OPTIONS = [ ] function buildUrl(opts: IClientOptions, client: MqttClient) { - let url = `${opts.protocol}://${opts.hostname}:${opts.port}${opts.path}` + const urlBuilder = new URL(`${opts.protocol}://${opts.host}`) + if (opts.port) { + urlBuilder.port = opts.port.toString() + } + urlBuilder.pathname = opts.path || '/' + Object.keys(opts.query || {}).forEach((key) => { + urlBuilder.searchParams.append(key, opts.query[key]) + }) + if (opts.username) { + urlBuilder.username = opts.username + if (opts.password) { + urlBuilder.password = opts.password.toString() + } + } + let url = urlBuilder.toString() if (typeof opts.transformWsUrl === 'function') { url = opts.transformWsUrl(url, opts, client) } From 1cc8f7b5c2d101b6e34876d5d9f31c881d7cd4de Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Thu, 15 Aug 2024 20:07:06 +0100 Subject: [PATCH 2/5] update test of ::1 should have square brackets --- test/mqtt.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mqtt.ts b/test/mqtt.ts index c76f90873..c33a31f93 100644 --- a/test/mqtt.ts +++ b/test/mqtt.ts @@ -59,7 +59,7 @@ describe('mqtt', () => { c.should.be.instanceOf(mqtt.MqttClient) c.options.should.not.have.property('path') - c.options.should.have.property('host', '::1') + c.options.should.have.property('host', '[::1]') c.end((err) => done(err)) }) From 19ab1c090a23d7b3bcec2d6721e9e2a2c3e04552 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Thu, 15 Aug 2024 20:09:20 +0100 Subject: [PATCH 3/5] ignore test artifacts --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 13052c8f8..226dbd0c9 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ coverage test/typescript/.idea/* test/typescript/*.js test/typescript/*.map +test-store*/* # VS Code stuff **/typings/** .vscode/ From 4cb3900840426b127b476621fcee262b671aa11b Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Thu, 15 Aug 2024 20:23:41 +0100 Subject: [PATCH 4/5] Remove unused import --- src/lib/connect/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/connect/index.ts b/src/lib/connect/index.ts index 0d271458e..0484a04b7 100644 --- a/src/lib/connect/index.ts +++ b/src/lib/connect/index.ts @@ -1,6 +1,5 @@ /* eslint-disable @typescript-eslint/no-var-requires */ import _debug from 'debug' -import url from 'url' import MqttClient, { IClientOptions, MqttClientEventCallbacks, From b9a782d1c44de134e69bab973648e90c651f14ef Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Thu, 15 Aug 2024 20:31:29 +0100 Subject: [PATCH 5/5] TODO clarification --- src/lib/connect/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/connect/index.ts b/src/lib/connect/index.ts index 0484a04b7..cf7a6297c 100644 --- a/src/lib/connect/index.ts +++ b/src/lib/connect/index.ts @@ -61,7 +61,8 @@ function connect( parsedOptions.protocol = parsedUrl.protocol as MqttProtocol parsedOptions.path = parsedUrl.pathname // TODO: See note below // NOTE: new URL().pathname is not the same as url.parse().path. URL.pathname does not include the query string. - // To make it compatible with url.parse().path, we need to append the query string to the path. + // To make this field align with url.parse().path, we would need to append the query string to the path as below + // However I am not sure if this is required or used later in the code. // if (parsedUrl.search) { // parsedOptions.path += parsedUrl.search // }