The Wayback Machine - https://web.archive.org/web/20220411104424/https://github.com/apple/swift-nio/issues/1650
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

Provide IPAddress type #1650

Open
Lukasa opened this issue Sep 24, 2020 · 8 comments
Open

Provide IPAddress type #1650

Lukasa opened this issue Sep 24, 2020 · 8 comments
Labels
enhancement good first issue

Comments

@Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Sep 24, 2020

If you want to work with a complete socket address type including port, we have a helper in SocketAddress. However, the moment you want to talk only about an IP address we force you to hold an in_addr or in_addr6 structure: hardly the friendliest versions of these data types.

We should provide a helpful wrapper IPAddress type that can be converted into those types as needed. This would be a much friendlier way to work with IP addresses. This is a good issue for those who are familiar with either IP addresses or Swift.

@shekhar-rajak
Copy link
Contributor

@shekhar-rajak shekhar-rajak commented Jul 6, 2021

Hi, I would like to work on this issue. I think we want a structure inherent from SocketAddress to only work on IP address with name IPAddress

@Lukasa
Copy link
Contributor Author

@Lukasa Lukasa commented Jul 6, 2021

I don't think we need to involve SocketAddress: an independent type called NIOIPAddress is probably sufficient.

@shekhar-rajak
Copy link
Contributor

@shekhar-rajak shekhar-rajak commented Jul 6, 2021

Thanks I will work on it.

@shekhar-rajak
Copy link
Contributor

@shekhar-rajak shekhar-rajak commented Jul 7, 2021

I was exploring the codebase, here is my findings :

  • init(ipAddress: String, port: Int) is the constructor that takes IP in string & port in Int type. So we want a new type named NIOIPAddress to have the same behaviour to check ip4 and ip6 with other attributes related to IP address
  • This new type NIOIPAddress should be class I think.

I want to understand some of the test cases and requirements for this new type NIOIPAddress so that I can work on it as per the unit tests.

@Lukasa
Copy link
Contributor Author

@Lukasa Lukasa commented Jul 7, 2021

Yes, we absolutely want an init(string: String) constructor. I think we need a few others:

  • init<Bytes: Collection>(packedBytes bytes: Bytes) where Element == UInt8
  • init(posixIPv4Address: in_addr)
  • init(posixIPv6Address: in6_addr)

I don't think NIOIPAddress should be a class though, I think it should be an enum with two cases, each of which should be structs. It certainly wants to have value semantics.

For our test cases, we need to confirm that it can accept the wide range of string formats for IP addresses:

We also need to be able to produce these string representations when requested, as well as have a default implementation of CustomStringConvertible. The type also needs to be Hashable. Finally, we need to able to produce in_addr and in6_addr as appropriate.

@shekhar-rajak
Copy link
Contributor

@shekhar-rajak shekhar-rajak commented Jul 7, 2021

Thanks a lot @Lukasa ! I understood the scenario. I have good amount of test cases in mind to work in implementation part.

@SeJV
Copy link
Contributor

@SeJV SeJV commented Feb 19, 2022

Hey @Lukasa thanks for the good description. As I was working on this I came to the topic of zone/scope-id of the IPv6Address.

IPv6 segmented with scope ID (see https://datatracker.ietf.org/doc/html/rfc4007.html#section-11)

As I was reading through this: https://docs.huihoo.com/doxygen/linux/kernel/3.7/uapi_2linux_2in6_8h_source.html I noticed that the zone/scope-id is part of the IPv6SocketAddress and the IPv6Address only contains the bytes. Should there be an optional zone: String? as part of the IPv6Address struct or should this and the string initialiser with zone/scope-id part of the IPv6SocketAddress?

@Lukasa
Copy link
Contributor Author

@Lukasa Lukasa commented Feb 21, 2022

Yeah that's a good question. I'm ok with the idea of kicking the scope zone ID to the curb for the IPv6Address on the grounds that it is not really part of the address itself but is part of the contextual information on a specific machine. So it belongs in IPv6SocketAddress and not IPv6Address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue
3 participants