-
Notifications
You must be signed in to change notification settings - Fork 19
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
Proposal for toString representation of AsnRange and AbstractIpRange #19
Comments
Hi @dadepo, the current implementation of Asn.parse(Asn.of("AS3333").toString()); // good
Ipv4.parse(Ipv4.parse("192.188.1.1").toString()); // good
// however
AsnRange.parse("AS3333"); // throws IllegalArgumentException
Ipv4Range.parse("192.168.1.1"); // throws IllegalArgumentException Moreover, for IP ranges it is not correct to have a toString() method that omits the prefix length even when you have a range of one address. For example, if So, the only option we could consider is adding a new method to AsnRange (only) that does the check of the length for you e.g.
But I think that if user-facing software is the main concern here, then the application should take care of the condensed representation as the library cannot cater for all cases. |
Regarding the In a world where the
if it matters, I observed IANA does use the As regards the other observation with IP ranges and prefix length, I feel this is a result of a fundamental tension between the design choice to conflate ranges that can be represented via CIDR and ranges that are just arbitrary contiguous IP numbers.
Yes, but it is also not correct to have a range of 3 IP numbers...that is or not? Well, maybe it depends? If you intend to work with CIDR then it is not correct, if you just want contiguous IP numbers, then maybe it is correct? And then, having But that also leads to the ambiguity where you ask: when is For ip-num, which I am working on, I currently have support for only CIDR-able range, and still thinking of how to introduce another abstraction for contiguous IP numbers in such a way that the two work together as intuitively as possible. I am quite certain there must have been discussions along these lines during the early development of commons-ip-math. Would be curious to know why the current design choice was made :) But yes, with the same abstraction used for CIDR-able ranges and non CIDR ranges, I am not sure it is possible (or how) to resolve the conflict/ambiguity my proposal to the toString implementation would lead to.
This would not help that much with the use case I ran into at work that leads to making this proposition, which is having a collection of different ranges (Asn, IPv4, IPv6) treating them all as
Yeah. This is what we are currently doing. It might just be that there isn't a feasible way to make the library cater for these cases, then I guess there is not much that can be done. |
In practice, we have had to render a collection of different ip numbers (asn, ipv4, ipv6) as strings.
The current string representation is in the format
startIp - endIp
and since the library also supports constructing a range with both values as start and end, or converting a single ASN to a range, you run into the situation where the string representation becomes a little bit redundant.For example
new Asn(3333l).asRange();
givesAS3333-AS3333
which appears redundant, and IMHO, strictly speaking, is not even a range if it starts and end with the same value.Will it be an acceptable proposal to have the toString representation return just a value in cases where a range contains just one value?
The text was updated successfully, but these errors were encountered: