Skip to content

fix: IP() refuses integers out of range - #30

Merged
marcos-mendez merged 1 commit into
Keel-Linux:masterfrom
Nandika-Gupta:fix/ip-range
Oct 6, 2026
Merged

marcos-mendez merged 1 commit into
Keel-Linux:masterfrom
Nandika-Gupta:fix/ip-range

Conversation

@Nandika-Gupta

Copy link
Copy Markdown

Follow-up from #29, as suggested there.

IP() took any integer, so IP(2**32) or IP(-1) was created fine and only failed later in str() with a struct.error. It now raises Error("ip out of range") at construction, like the "illegal ip" error for bad strings.

IPRange built its broadcast address as network + 2**32 - netmask - 1, which goes past 2**32 for a moment with IP's operators, so that line now does the maths on plain ints. Same result as before.

Tests: 0 and 232-1 still work, -1 and 232 raise, and 255.255.255.255 + 1 raises. Full suite: 956 passed, ipaddr.py stays at 100%. Added a changelog entry (2.2.3+keel16).

@marcos-mendez marcos-mendez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Nandika-Gupta, this is a clean follow-up.

I ran it: 956 passed, ipaddr.py stays at 100%. The range check is right (0 to 232-1, both ends tested), and the error matches the existing "illegal ip" style. Good catch on IPRange.broadcast: with the new check, computing network + 2**32 - netmask - 1 through IP's operators would overshoot 232 in the middle of the expression and raise, so doing it on plain ints is exactly the right fix. The test that 255.255.255.255 + 1 now raises is a nice touch too.

A correction to something I wrote on #29: I said the network screens use IPRange to validate host addresses. I checked, and they don't: confconsole.py only uses ipaddr.is_legal_ip, and the final network checks use Python's stdlib ipaddress. So IP and IPRange have no runtime callers today, and this change can't break a screen. The strict < in __contains__ is still worth documenting as you did, but my reason for it was wrong. Sorry about that.

Approving; I'll merge.

@marcos-mendez
marcos-mendez merged commit 5835130 into Keel-Linux:master Oct 6, 2026
2 checks passed
@Nandika-Gupta

Copy link
Copy Markdown
Author

Oh got it, thanks for clarifying! Glad the fix works.

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

Successfully merging this pull request may close these issues.

2 participants