Fix snet.UDPAddress parsing & serialization #3648
Conversation
Allow <IPv4>:port without []. Use net library functions instead of manual parsing. Fixes scionproto#3647
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker and @oncilla)
go/lib/snet/udpaddr_test.go, line 139 at r1 (raw file):
{address: "", isError: true}, {address: "1-ff00:0:301,1.2.3.4", isError: true}, {address: "1-ff00:0:300,[1.2.3.4]:80",
Shouldn't this one be raising an error?
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @mkowalski and @oncilla)
go/lib/snet/udpaddr_test.go, line 139 at r1 (raw file):
Previously, mkowalski (Mateusz Kowalski) wrote…
Shouldn't this one be raising an error?
It would make the code much more complicated to make this raise an error. Not sure it's worth the gain. It would also break everyone who uses it currently like this. I think the more important part is that we allow IPv4 without [].
Or what would be the advantage of disallowing this?
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @oncilla)
go/lib/snet/udpaddr_test.go, line 139 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
It would make the code much more complicated to make this raise an error. Not sure it's worth the gain. It would also break everyone who uses it currently like this. I think the more important part is that we allow IPv4 without [].
Or what would be the advantage of disallowing this?
No any obvious advantage apart from the fact currently we are accepting syntax which is incorrect. Anyway, I'm marking this as non-blocking as I don't really want to push for disallowing what is currently working
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker and @oncilla)
go/lib/snet/udpaddr_test.go, line 158 at r1 (raw file):
port: 5, }, {address: "1-ff00:0:302,[::1]:60000",
I would be very happy to see also tests for 1-ff00:0:302,[::1] and 1-ff00:0:302,::1 and "1-ff00:0:302,::1:60000"
|
Closing in favor of #3650 |
Allow :port without [] for IPv4.
Use net library functions instead of manual parsing.
```
Recommended:
- isd-as,ipv4:port (e.g., 1-ff00:0:300,192.168.1.1:8080)
- isd-as,[ipv6]:port (e.g., 1-ff00:0:300,[f00d::1337]:808)
Others:
- isd-as,[ipv4]:port (e.g., 1-ff00:0:300,[192.168.1.1]:80)
- isd-as,[ipv4] (e.g., 1-ff00:0:300,[192.168.1.1])
- isd-as,[ipv6] (e.g., 1-ff00:0:300,[f00d::1337])
- isd-as,ipv4 (e.g., 1-ff00:0:300,192.168.1.1)
- isd-as,ipv6 (e.g., 1-ff00:0:300,f00d::1337)
Not supported:
- isd-as,ipv6:port (caveat if ipv6:port builds a valid ipv6 address,
it will successfully parse as ipv6 without error)
```
Fixes #3647
based on #3648
Co-authored-by: Lukas Vogel <lukedirtwalker@gmail.com>
Allow :port without [].
Use net library functions instead of manual parsing.
Fixes #3647
This change is