fix: enclose ipv6 host with brackets on client request

[aldy505]

Closes #1241

[trim21]

can't we just check : like standard library does?

[aldy505]

can't we just check : like standard library does?

I don't think that makes that much of a difference.

See this sample code:

const net = require("net");

const input = ["localhost", "postgres.arpa", "172.16.0.1", "valid-domain.com", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "::1234:5678", "::", "::1234:5678:91.123.4.56", "::11.22.33.44"];

for (const i of input) {
  console.log(`Input: ${i}; IsIP? ${net.isIP(i)};`);
}

// Output:
// Input: localhost; IsIP? 0;
// Input: postgres.arpa; IsIP? 0;
// Input: 172.16.0.1; IsIP? 4;
// Input: valid-domain.com; IsIP? 0;
// Input: 2001:0db8:85a3:0000:0000:8a2e:0370:7334; IsIP? 6;
// Input: ::1234:5678; IsIP? 6;
// Input: ::; IsIP? 6;
// Input: ::1234:5678:91.123.4.56; IsIP? 6;
// Input: ::11.22.33.44; IsIP? 6;

[trim21]

can't we just check : like standard library does?

I don't think that makes that much of a difference.

See this sample code:

const net = require("net");

const input = ["localhost", "postgres.arpa", "172.16.0.1", "valid-domain.com", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "::1234:5678", "::", "::1234:5678:91.123.4.56", "::11.22.33.44"];

for (const i of input) {
  console.log(`Input: ${i}; IsIP? ${net.isIP(i)};`);
}

// Output:
// Input: localhost; IsIP? 0;
// Input: postgres.arpa; IsIP? 0;
// Input: 172.16.0.1; IsIP? 4;
// Input: valid-domain.com; IsIP? 0;
// Input: 2001:0db8:85a3:0000:0000:8a2e:0370:7334; IsIP? 6;
// Input: ::1234:5678; IsIP? 6;
// Input: ::; IsIP? 6;
// Input: ::1234:5678:91.123.4.56; IsIP? 6;
// Input: ::11.22.33.44; IsIP? 6;

some downstream users are using this package in web environment, using node:net may need more polyfill.

[aldy505]

can't we just check : like standard library does?

I don't think that makes that much of a difference.

See this sample code:

const net = require("net");

const input = ["localhost", "postgres.arpa", "172.16.0.1", "valid-domain.com", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "::1234:5678", "::", "::1234:5678:91.123.4.56", "::11.22.33.44"];

for (const i of input) {

  console.log(`Input: ${i}; IsIP? ${net.isIP(i)};`);

}

// Output:

// Input: localhost; IsIP? 0;

// Input: postgres.arpa; IsIP? 0;

// Input: 172.16.0.1; IsIP? 4;

// Input: valid-domain.com; IsIP? 0;

// Input: 2001:0db8:85a3:0000:0000:8a2e:0370:7334; IsIP? 6;

// Input: ::1234:5678; IsIP? 6;

// Input: ::; IsIP? 6;

// Input: ::1234:5678:91.123.4.56; IsIP? 6;

// Input: ::11.22.33.44; IsIP? 6;

some downstream users are using this package in web environment, using node:net may need more polyfill.

I feel like I'm missing something here. What about all those node:http and node:https imports?

Another side note: We should make this clear on README for what environment we support.

[trim21]

I feel like I'm missing something here. What about all those node:http and node:https imports?

there are some issues about this:

https://github.com/minio/minio-js/issues/1002 https://github.com/minio/minio-js/issues/1053

Another side note: We should make this clear on README for what environment we support.

plus 1 for this

[aldy505]

I feel like I'm missing something here. What about all those node:http and node:https imports?

there are some issues about this:

1002 #1053

Another side note: We should make this clear on README for what environment we support.

plus 1 for this

Okay, will implement that tomorrow (or next weekend)

[aldy505]

Request for review @trim21 @prakashsvmx

[prakashsvmx]

Adding some unit test would be beneficial @aldy505

[aldy505]

Adding some unit test would be beneficial @aldy505

Done

[trim21]

Lgtm

[prakashsvmx]

Thank you @aldy505 for the contribution.