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

summarize not working correctly #6

Closed
giladfapri opened this issue Apr 14, 2020 · 4 comments
Closed

summarize not working correctly #6

giladfapri opened this issue Apr 14, 2020 · 4 comments

Comments

@giladfapri
Copy link

summarize([77.138.225.244/32,77.138.225.245]) will result: [77.138.225.244/31], when clearly that isn't right

@astellingwerf
Copy link
Contributor

Can you explain why you closed the issue? I seem to notice the same behavior.
My understanding of summarize is that it would condense adjacent addresses or CIDrs into bigger CIDRs, without including addresses that weren't part of the input.

@demskie
Copy link
Owner

demskie commented Jul 12, 2021

@astellingwerf can you provide some input that reproduces the issue?

Note that in his example the second address has an implied /32 that makes the output valid.

I can see a potential issue with assuming a network address is /32 if a CIDR is not present. Maybe a better behavior would be to take Classful Addressing into account to assume the correct mask length.

astellingwerf added a commit to astellingwerf/netparser that referenced this issue Jul 13, 2021
@astellingwerf
Copy link
Contributor

astellingwerf commented Jul 13, 2021

The primary issue that I saw is that the function internally generates invalid Networks, which then causes invalid decisions to be made, and therefor with unexpected results.

The last paragraph of RFC 4632 says:

   x is a 1- to 7-bit value, based on the prefix length, shifted into
   the most significant bits of the octet and converted into decimal
   form; the least significant bits of the octet are zero.

It means that for example a /31 must end with an even number, and a /30 ends in a number divisible by 4.

@demskie
Copy link
Owner

demskie commented Aug 9, 2021

merged and fixed: #17

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

3 participants