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.

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

Support passing in inet6 to gen_tcp:connect #84

Open
acco opened this issue Dec 24, 2024 · 2 comments
Open

Support passing in inet6 to gen_tcp:connect #84

acco opened this issue Dec 24, 2024 · 2 comments

Comments

@acco
Copy link

acco commented Dec 24, 2024

Hey there,

As far as I can tell, the {socket_options, SocketOpts} are gen_tcp:connect_option:

https://github.com/Nordix/eredis/blob/master/src/eredis_client.erl#L455

Note it's not a list of tuples – one of the options is a bare atom!

-type connect_option() ::
          {fd, Fd :: non_neg_integer()} |
          inet:address_family() |
          {ifaddr, socket:sockaddr_in() | socket:sockaddr_in6() | inet:socket_address()} |
          {ip, inet:socket_address()} |
          {port, inet:port_number()} |
          {tcp_module, module()} |
          {netns, file:filename_all()} |
          {bind_to_device, binary()} |
          option().

Namely, inet:address_family():

-type address_family() :: inet | inet6 | local.

So, to setup inet6, one needs to be able to pass in {socket_options, [inet6]} to eredis.

However, these lines assume a keyword list:
https://github.com/Nordix/eredis/blob/master/src/eredis_client.erl#L438-L442

So, we get an ArgumentError: lists.erl:1510: :lists.ukeymerge_1/3

Happy to open a PR here. As I understand it, current behavior is just trying to merge in default ?SOCKET_OPTS to provided opts

@acco
Copy link
Author

acco commented Dec 24, 2024

Ah I see. The address family is automatically parsed and used to setup SocketOptions:

https://github.com/Nordix/eredis/blob/master/src/eredis_client.erl#L433

Issue I ran into is a common one in Docker: inet:getaddrs(Hostname, inet6) will resolve for host.docker.internal, but you can't connect without special networking setup. inet:getaddrs(Hostname, inet) will resolve too, and connect by default.

One easy way around this is to flip the check so we check inet:getaddrs(Hostname, inet) first and see if that's valid an resolves. If it is, use ipv4. Otherwise, try ipv6.

After all, ipv4 is far more widely supported anyway.

@zuiderkwast
Copy link
Collaborator

zuiderkwast commented Dec 24, 2024

IMO we could just let gen_tcp:connect do the DNS lookup and accept the inet and inet6 options. We should fix the code that assumes a list of tuples. We can probabaly use functions from proplists instead of lists.

@zuiderkwast zuiderkwast changed the title Support passing in :inet6 to gen_tcp:connect Support passing in inet6 to gen_tcp:connect Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants