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.

Sign up for GitHub

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

Jump to bottom

When connecting ws://[::1]:8883, an error is thrown - Uncaught TypeError TypeError [ERR_INVALID_URL]: Invalid URL #1569

Open
Steve-Mcl opened this issue Jan 16, 2023 · 8 comments · May be fixed by #1927
Open

When connecting ws://[::1]:8883, an error is thrown - Uncaught TypeError TypeError [ERR_INVALID_URL]: Invalid URL #1569

Steve-Mcl opened this issue Jan 16, 2023 · 8 comments · May be fixed by #1927

Comments

@Steve-Mcl
Copy link

Hi, I am getting an exception when connecting to IPv6 localhost address ::1

I have found this was reported and fixed in #1130 / #1147 (commit 70a247c) however it seems to have been reverted and the current V4.3.7 is once more using the depreciated url.parse as you can see here: https://github.com/mqttjs/MQTT.js/blob/main/lib/connect/index.js#L64

Here is a minimal repro...

        const userEnteredURL = 'ws://[::1]:8883'
        const client = mqtt.connect(userEnteredURL, this.brokerConfig)

Here is why I think the underlying issue exists: mqtt.js manually rebuilds the broker URL using hostname + port in
https://github.com/mqttjs/MQTT.js/blob/main/lib/connect/ws.js#L20

let url = opts.protocol + '://' + opts.hostname + ':' + opts.port + opts.path

... but, url.parse().hostname drops the squarebrackets off [::1] which causes the above code to generate the url http://::1:8883. Further downstream, the ws package uses new URL(...) with this re-assembled URL string & the ERR_INVALID_URL error occurs.

For anyone else reading, here is a minimal (hacky) workaround...

        const userEnteredURL = 'ws://[::1]:8883'
        const tempNewURL= new URL(userEnteredURL)
        const brokerURL = url.parse(userEnteredURL)
        brokerURL.hostname = tempNewURL.hostname // force the hostname back to [::1]
        const client = mqtt.connect(brokerURL, this.brokerConfig)

Additional info...

  • Calling mqtt.connect(new URL('http://[::1]:8883')) fails because connect only accepts a string or url.parse object
  • Calling mqtt.connect(url.parse('http://[::1]:8883') fails because unlike new URL, url.parse drops the square brackets off hostname and mqtt.js uses hostnameto reassemble theurl`
  • websocket code uses new URL so when mqtt.js passes the reassembled IP http://::1:8883 it failes with error ERR_INVALID_URL

url.parse vs new URL

url.parse('http://[::1]:8883')
Url {
  protocol: 'ws:',
  slashes: true,
  auth: null,
  host: '[::1]:8883',
  port: '8883',
  hostname: '::1', //<< square brackets are dropped
  hash: null,
  search: null,
  query: null,
  pathname: '/',
  path: '/',
  href: 'ws://[::1]:8883/'
}
new URL('http://[::1]:8883')
URL {
  href: 'ws://[::1]:8883/',
  origin: 'ws://[::1]:8883',
  protocol: 'ws:',
  username: '',
  password: '',
  host: '[::1]:8883',
  hostname: '[::1]', //<< square brackets are kept
  port: '8883',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}
This was referenced Jan 16, 2023
Closed
Closed
@mklinke
Copy link

mklinke commented Mar 8, 2023

Might be relevant: nodejs/node#44731

@Steve-Mcl
Copy link
Author

@mklinke no, I dont think so.

I strongly believe this is a bug in how this lib (MQTT.js) breaks down and re-assembles the URL as detailed in my initial post:

I have found this was reported and fixed in #1130 / #1147 (commit 70a247c) however it seems to have been reverted and the current V4.3.7 is once more using the depreciated url.parse as you can see here: https://github.com/mqttjs/MQTT.js/blob/main/lib/connect/index.js#L64

Here is a minimal repro...

        const userEnteredURL = 'ws://[::1]:8883'
        const client = mqtt.connect(userEnteredURL, this.brokerConfig)

Here is why I think the underlying issue exists: mqtt.js manually rebuilds the broker URL using hostname + port in https://github.com/mqttjs/MQTT.js/blob/main/lib/connect/ws.js#L20

@robertsLando
Copy link
Member

Could you submit a PR to fix this?

@robertsLando
Copy link
Member

MQTT 5.0.0 BETA is now available! Try it out and give us feedback: npm i mqtt@beta. It may fix your issues

@kmitchel
Copy link

kmitchel commented Apr 6, 2024 •
edited
Loading

While testing using a severs ipv6 address, I've noticed this issue. I construct a websocket url using window.location.hostname, when the error is thrown the URL is missing brackets. IPv6 connections using a name work properly, just not bracket notation. Using 5.5.0

@rockey2020
Copy link

same question.. when my url be mqtts://::1:14509 it cant be working , but i change to mqtts://127.0.0.1:14509 it work

@robertsLando
Copy link
Member

Can anyone submit a PR to fix this issue? I think it's just a matter of url parsing here:

MQTT.js/src/lib/connect/index.ts

Line 71 in 9fc79df

const parsed = url.parse(brokerUrl, true)

@Steve-Mcl Steve-Mcl mentioned this issue Aug 15, 2024
Merged
10 tasks
@Steve-Mcl
Copy link
Author

MQTT 5.0.0 BETA is now available! Try it out and give us feedback: npm i mqtt@beta. It may fix your issues

My sincere apologies. I will spare you the reasons but we are only just now looking to make the switch from MQTT 4.x to 5.x

I tested this issue today and unfortunately it remains in the 5.x stream.

Details

I have looked at the src for V5 and it still uses the depreciated url.parse to deconstruct the URL then reconstructs it manually resulting in the loss of square brackets which is what the workaround in my OP was fixing.


Can anyone submit a PR to fix this issue? I think it's just a matter of url parsing here:

MQTT.js/src/lib/connect/index.ts

Line 71 in 9fc79df

const parsed = url.parse(brokerUrl, true)

I believe the correct solution is to use the non depreciated way of URL parsing AKA "WHATWG URL API"
REF: https://nodejs.org/docs/latest-v12.x/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
REF: https://github.com/orgs/nodejs/discussions/23694

To provide some weight to this, the ws package uses WHATWG URL API (new URL(...)) with the URL that MQTT LIB has re-assembled & thus an error occurs.

Let me look at what is involved in a PR (hopefully not too many places where url.parse is being used.

@Steve-Mcl Steve-Mcl linked a pull request Aug 15, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: replace deprecated url.parse with new URL Steve-Mcl/MQTT.js
5 participants
@mklinke @kmitchel @robertsLando @Steve-Mcl @rockey2020