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

Merged

Nandika-Gupta merged 1 commit from fix/ip-range into master

2026-10-06 16:20:09 -03:00

Nandika-Gupta commented

2026-10-06 16:05:21 -03:00

(Migrated from github.com)

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).

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 2**32-1 still work, -1 and 2**32 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 (Migrated from github.com) approved these changes

2026-10-06 16:20:06 -03:00

marcos-mendez (Migrated from github.com) left a comment

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.

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 2**32-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 2**32 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.

Nandika-Gupta commented

2026-10-06 16:39:00 -03:00

(Migrated from github.com)

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