InternetAddress constructor parsing of ipv6 addresses is more lenient on macOS

[jonahwilliams]

Given fe80::ec4:7aff:fecc:ea8f%eno2, on Linux and Windows the InternetAddress constructor will throw an InvalidArgument error. On macOS this value is accepted.

example:

void main() {
  InternetAddress('fe80::ec4:7aff:fecc:ea8f%eno2');
}

on Linux/macOS/Fuchsia/Android) socket_base implementations, the actual parsing of this address is delegated to inet_pton. This function appears to have different behavior on macOS/Linux (Android/Fuchsia not tested), which can be reproduced with the following poorly written C code:

#include <arpa/inet.h>
#include <stdio.h>

int main() {
  char* addr = "fe80::ec4:7aff:fecc:ea8f%eno2";
  char* out;
  printf("%d\n", inet_pton(AF_INET6, addr, &out));
}

This prints 1 on macOS, and 0 on Linux.

Is there a boringssl function Dart could invoke instead of depending on system libraries? Perhaps https://github.com/google/boringssl/blob/master/crypto/x509v3/v3_utl.c#L1126

Dart VM version: 2.9.0-3.0.dev.flutter-b0d35855d8

[zanderso]

@zichangg

[natebosch]

Another difference from the linked thread - mac allows parsing 01.02.03.04. Values with leading zeroes are rejected on linux.

[natebosch]

from @lrhn at #41727 (comment)

I'd prefer to just require a strict input format (no unnecessary leading zeros), and reject bad inputs early, before passing them to native code.
It's usually never a good idea to change user input. If nothing else, it makes it hard to report errors afterwards, if the errors are reported on a different string than what the user provided.

[sortie]

Yes let's be strict. It seems the inet_pton function is more tailored for system use and allows some additional notations that we don't.

I suggest we write a IPv4/IPv6 parser in Dart based on the formats described by the appropriate RFCs. I don't see any reason to call into native code.

@zichangg Might you be interested in taking this one?

[mraleph]

We should be considerate of binary size. Is there any really strong reason to add this to our core libraries? If not - we should not write our own parsers.

[sortie]

Compelling point about code size.

I don't see a way of becoming stricter without more code. But perhaps it could be offset by removing the native code calling inet_pton. If we have a parser in Dart, we can tree shake it, and we can remove SocketBase::ParseAddress and the InternetAddress_Parse native function. We would have to measure the size difference.

[zanderso]

I'd encourage (at least) investigating @jonahwilliams' suggestion of using parsing calls from boringssl. Size increase should be ~0 there since the VM is already linking against boringssl on all platforms.

[zichangg]

I'd encourage (at least) investigating @jonahwilliams' suggestion of using parsing calls from boringssl. Size increase should be ~0 there since the VM is already linking against boringssl on all platforms.

Unfortunately, boringssl doesn't handle IPv6 with scoped ID. The implementation doesn't check for potential %scopedID.

This is a special case for inet_pton(Some comments from https://stackoverflow.com/questions/10329908/bsd-sockets-ip6-inet-pton-and-how-to-retrieve-the-scope-id). We do have a way to handle it instead of rewriting a parser. The current dart sidelookup is able to parse it by calling getaddrinfo.
This is my mistake. I added to Socket.startConnect() and forgot to put the same check in InternetAddress.