fix: ipv6 addresses parsing

perrin4869 commented 1 month ago

https://github.com/ladjs/superagent/pull/1803 broke the parsing of ipv6 addresses such as http://[::]:80. Before that PR, using node:url, we would get:

> const { parse } = require("node:url")
undefined
> parse("http://[::]:80")
Url {
  protocol: 'http:',
  slashes: true,
  auth: null,
  host: '[::]:80',
  port: '80',
  hostname: '::',
  hash: null,
  search: null,
  query: null,
  pathname: '/',
  path: '/',
  href: 'http://[::]:80/'
}

With the URL class, however, the hostname becomes:

> new URL("http://[::]:80")
URL {
  href: 'http://[::]/',
  origin: 'http://[::]',
  protocol: 'http:',
  username: '',
  password: '',
  host: '[::]',
  hostname: '[::]',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

This hostname [::] is not compatible with the http.request function. I added some normalization functionality here to return the previous behavior.

Edit: hm... it is a bit hard to add regression tests here, since all the tests seem to run against a single server that doesn't seem to listen on [::]

gramakri commented 3 weeks ago

Can confirm. This broke in 9.0.2 .

titanism commented 3 weeks ago

If you can get the tests to pass I can merge

perrin4869 commented 3 weeks ago

ok, I managed to add some regression tests! they don't seem to work on http2 so I disabled those

ladjs / superagent

fix: ipv6 addresses parsing #1805