Conversation
Replace iplib CIDR enumeration with net/netip in enumerateHosts(). IPv4 network/broadcast stripping behavior preserved for prefix < /31. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the github.com/c-robinson/iplib dependency by rewriting config.enumerateHosts() to use Go’s standard library net/netip for CIDR expansion, and adds targeted unit tests to validate parity across IPv4/IPv6 cases.
Changes:
- Replaced
enumerateHosts()implementation fromiplibtonetip.ParsePrefix+Addr.Next()iteration. - Added
TestEnumerateHoststable test covering IPv4, IPv6, and error cases. - Removed
github.com/c-robinson/iplibfromgo.modandgo.sum.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
config/tomlloader.go |
Reimplements CIDR host expansion using net/netip and preserves IPv4 network/broadcast stripping behavior. |
config/tomlloader_test.go |
Adds direct unit coverage for enumerateHosts() behavior across IPv4/IPv6 and invalid inputs. |
go.mod |
Drops direct dependency on github.com/c-robinson/iplib. |
go.sum |
Removes checksums for github.com/c-robinson/iplib. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
config/tomlloader.go
Outdated
| count := uint64(1) << hostBits | ||
| addrs := make([]string, 0, count) | ||
| addr := prefix.Masked().Addr() |
There was a problem hiding this comment.
count is a uint64, but make([]string, 0, count) requires an int capacity/length (per Go spec). As written this won’t compile (or will require an implicit conversion that Go doesn’t do). Convert count to int after checking it fits in math.MaxInt (and adjust the loop accordingly).
There was a problem hiding this comment.
Fixed in c35f046. Changed count from uint64 to int so make([]string, 0, count) receives the correct type directly. Additionally, the hard upper bound was tightened from 2^63 to 2^16 host bits, which guarantees count always fits comfortably in an int on any platform.
| hostBits := prefix.Addr().BitLen() - prefix.Bits() | ||
| if hostBits > 63 { | ||
| return nil, xerrors.Errorf("Failed to enumerate IP address. err: mask bitsize too big") | ||
| } | ||
|
|
||
| count := uint64(1) << hostBits | ||
| addrs := make([]string, 0, count) | ||
| addr := prefix.Masked().Addr() | ||
| for range count { | ||
| addrs = append(addrs, addr.String()) | ||
| addr = addr.Next() | ||
| } |
There was a problem hiding this comment.
This enumerates 1<<hostBits addresses and also preallocates the slice to that size. For inputs like IPv4 /0 (hostBits=32) or IPv6 /65 (hostBits=63), this will attempt to allocate/enumerate billions to quintillions of entries and will reliably OOM/hang. Consider adding a hard upper bound on the number of hosts that can be expanded (or at least reject prefixes where count exceeds a reasonable limit / math.MaxInt) to avoid config-triggered resource exhaustion.
There was a problem hiding this comment.
Fixed in c35f046. Tightened the upper bound from 2^63 to 2^16 host bits (maxHostBits = 16), so the maximum enumerable range is now a /16 for IPv4 (65,536 addresses) or /112 for IPv6. Wider prefixes like /0 or /8 now return an error immediately instead of attempting a huge allocation. Added test cases for 10.0.0.0/0 and 192.168.0.0/8 to verify.
- Change count from uint64 to int to avoid implicit conversion in make() - Lower the hard upper bound from 2^63 to 2^16 host bits to prevent OOM on wide prefixes like IPv4 /0 or /8 - Add test cases for oversized IPv4 prefixes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Why (motivation for removing this dependency)
c-robinson/iplibis a library for IP address/CIDR manipulationenumerateHosts()inconfig/tomlloader.goto expand CIDR notation into individual IP addressesnet/netip(added in Go 1.18) provides equivalent functionality withnetip.ParsePrefixandAddr.Next()What (replacement details)
enumerateHosts()usingnet/netip.ParsePrefix+Addr.Next()iterationTestEnumerateHostsunit test covering IPv4 (/30, /31, /32), IPv6 (/126, /127, /128), overflow (/32 IPv6), and non-CIDR inputsChanged files
config/tomlloader.goenumerateHosts()from iplib to net/netip (~30 lines), removed iplib importconfig/tomlloader_test.goTestEnumerateHostswith 9 test casesgo.mod/go.sumc-robinson/iplibSafety (why this is safe)
TestHosts(11 test cases covering IPv4/IPv6 CIDR ranges, ignore lists, edge cases)TestEnumerateHostsdirectly tests the rewritten functionTest plan
TestHosts— existing 11 test cases pass (behavioral parity)TestEnumerateHosts— new 9 test cases pass (direct function coverage)go build ./cmd/...— passgo test ./...— all passGOEXPERIMENT=jsonv2 golangci-lint run ./...— 0 issuesReview hint (how to review efficiently)
config/tomlloader.go— the core change, ~30 lines. Key logic:netip.ParsePrefix→ iterate withAddr.Next()→ strip network/broadcast for IPv4 < /31config/tomlloader_test.go— new test function, straightforward table-driven testsgo.mod— confirm iplib is removed entirely🤖 Generated with Claude Code