Open eserte opened 1 year ago
the patch looks reasonable and the additional dep for IO::Socket::IP sounds fine to me. Not sure what else you need to support ipv6 though.
About testing starman with IPv6: assuming your /etc/hosts
already contains an entry like this one
::1 ip6-localhost ip6-loopback
then you can already test IPv6 using a hostname, not an address. This looks good:
$ starman --listen 'ip6-localhost:5000' app.psgi
...
Resolved [ip6-localhost]:5000 to [::1]:5000, IPv6
Binding to TCP port 5000 on host ::1 with IPv6
...
netstat sees only a IPv6 bind, no tcp (without "6") entry:
$ sudo netstat -tulpen | grep starman
tcp6 0 0 ::1:5000 :::* LISTEN 1000 15945944 1757861/starman mas
Accessing using the IPv4 localhost address does not work, as expected:
$ curl 'http://127.0.0.1:5000'
curl: (7) Failed to connect to 127.0.0.1 port 5000: Connection refused
IPv6 works, both with address and hostname:
$ curl 'http://ip6-localhost:5000'
... response ...
$ curl 'http://[::1]:5000'
... response ...
strace also shows that IPv6 is in use:
connect(5, {sa_family=AF_INET6, sin6_port=htons(5000), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_scope_id=0}, 28) = -1 EINPROGRESS (Operation now in progress)
getpeername(5, {sa_family=AF_INET6, sin6_port=htons(5000), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_scope_id=0}, [128->28]) = 0
getsockname(5, {sa_family=AF_INET6, sin6_port=htons(55814), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_scope_id=0}, [128->28]) = 0
Patching Starman::Server is not enough, also https://github.com/plack/Plack/blob/ae1fb1d2fc603f11bdd2fb162438a103007d12ac/lib/Plack/Runner.pm#L93-L115 needs proper IPv6 handling.
Yeah, what about requiring square bracket for ipv6 addresses like you do in URI?
If using an IPv6 address in
--listen
or--host
(any, because every IPv6 address contains at least one colon), then starman fails like this:(depending on the used address different error messages like
may happen)
Problem is the usage of split in https://github.com/miyagawa/Starman/blob/1fb6f667138e8137b529139edd24d93240803ba0/lib/Starman/Server.pm#L77 A proper solution would implement something similar like IO::Socket::IP->split_addr. However, a solution would also need to support the "...:$option" (i.e. "...:ssl") notation.
An informal patch exists in https://github.com/eserte/Starman/commit/6ffcd597048bbf67898cea2c3baf4f4b53539370 However I do not propose to apply this patch, as it would require an additional dependency.