Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions cidr.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,9 @@ func nextSubnet(network *net.IPNet, prefixLen int) (*net.IPNet, error) {
// specific check.
//
// nolint:all
func isPowerOfTwoPlusOne(x int) bool {
return isPowerOfTwo(x - 1)
}
// func isPowerOfTwoPlusOne(x int) bool {
// return isPowerOfTwo(x - 1)
// }
Comment on lines +259 to +261
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Remove dead code instead of commenting it out.

While the documentation explaining why this function is obsolete is excellent, commented-out code should be deleted entirely. Version control preserves the history if needed.

Apply this diff to remove the dead code:

-// isPowerOfTwoPlusOne returns if a number is a power of 2 plus 1
-//
-// NOTE(dwisiswant0): This function is no longer used. The logic in
-// SplitIPNetIntoN was refactored to correctly handle non-power-of-two splits by
-// first splitting into the largest power-of-two less than or equal to n, and
-// then iteratively dividing the last subnet. This removed the need for this
-// specific check.
-//
-// nolint:all
-// func isPowerOfTwoPlusOne(x int) bool {
-// 	return isPowerOfTwo(x - 1)
-// }
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// func isPowerOfTwoPlusOne(x int) bool {
// return isPowerOfTwo(x - 1)
// }
🤖 Prompt for AI Agents
In cidr.go around lines 259 to 261, remove the commented-out dead function
isPowerOfTwoPlusOne (the three lines currently commented); delete the commented
code entirely from the file (leave the existing documentation/explanation
intact) so the dead code is not kept in comments — rely on VCS history if
needed.


// isPowerOfTwo returns if a number is a power of 2
func isPowerOfTwo(x int) bool {
Expand All @@ -268,11 +268,11 @@ func isPowerOfTwo(x int) bool {
// reverseIPNet reverses an ipnet slice
//
// nolint:all
func reverseIPNet(ipnets []*net.IPNet) {
for i, j := 0, len(ipnets)-1; i < j; i, j = i+1, j-1 {
ipnets[i], ipnets[j] = ipnets[j], ipnets[i]
}
}
// func reverseIPNet(ipnets []*net.IPNet) {
// for i, j := 0, len(ipnets)-1; i < j; i, j = i+1, j-1 {
// ipnets[i], ipnets[j] = ipnets[j], ipnets[i]
// }
// }
Comment on lines +271 to +275
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Remove dead code and verify no references remain.

Similar to isPowerOfTwoPlusOne, this commented-out function should be deleted entirely. Additionally, unlike the previous function, there's no explanation for why it's obsolete.

Apply this diff to remove the dead code:

-// reverseIPNet reverses an ipnet slice
-//
-// nolint:all
-// func reverseIPNet(ipnets []*net.IPNet) {
-// 	for i, j := 0, len(ipnets)-1; i < j; i, j = i+1, j-1 {
-// 		ipnets[i], ipnets[j] = ipnets[j], ipnets[i]
-// 	}
-// }
-

Run the following script to verify reverseIPNet is not referenced elsewhere:


🏁 Script executed:

#!/bin/bash
# Description: Search for any references to reverseIPNet in the codebase

# Search for function calls or mentions
rg -nP '\breverseIPNet\b' --type=go -C3

Length of output: 370


Remove the unused reverseIPNet code.

The commented-out function is unreferenced; delete it entirely.

--- a/cidr.go
+++ b/cidr.go
@@ -268,7 +268,0 @@
-// reverseIPNet reverses an ipnet slice
-//
-// nolint:all
-// func reverseIPNet(ipnets []*net.IPNet) {
-// 	for i, j := 0, len(ipnets)-1; i < j; i, j = i+1, j-1 {
-// 		ipnets[i], ipnets[j] = ipnets[j], ipnets[i]
-// 	}
-// }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// func reverseIPNet(ipnets []*net.IPNet) {
// for i, j := 0, len(ipnets)-1; i < j; i, j = i+1, j-1 {
// ipnets[i], ipnets[j] = ipnets[j], ipnets[i]
// }
// }
🤖 Prompt for AI Agents
In cidr.go around lines 271 to 275, there's an unused commented-out function
reverseIPNet; remove the entire commented block (the function and its
surrounding comment markers) so the file contains no dead/commented code, then
run gofmt/goimports to tidy imports and formatting.


// IPAddresses returns all the IP addresses in a CIDR
func IPAddresses(cidr string) ([]string, error) {
Expand Down
24 changes: 22 additions & 2 deletions cmd/mapcidr/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,28 @@ func process(wg *sync.WaitGroup, chancidr, outputchan chan string) {
}

for _, cidr := range cidrsToProcess {
// Add IPs into ipRangeList which are passed as input. Example - "192.168.0.0-192.168.0.5"

if strings.Contains(cidr, "-") {
// Try to parse as multi-octet range
if strings.Count(cidr, ".") == 3 {
ips, err := mapcidr.ExpandIPPattern(cidr)
if err != nil {
gologger.Fatal().Msgf("%s\n", err)
}

for _, ip := range ips {
ipCidr := ip.String() + "/32"
if options.Aggregate || options.Shuffle || hasSort || options.AggregateApprox || options.Count {
_, ipnet, _ := net.ParseCIDR(ipCidr)
allCidrs = append(allCidrs, ipnet)
} else {
commonFunc(ipCidr, outputchan)
}
}
continue
}

Comment on lines 421 to +442
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Apply MatchIP/FilterIP to expanded IPs in aggregate/sort/shuffle/count paths; also guard against ip:port and check ParseCIDR error

Currently, expanded IPs bypass the per‑IP Match/Filter logic when any of aggregate/sort/shuffle/aggregate‑approx/count are enabled, which changes semantics vs. non‑expanded inputs. This can leak filtered IPs into outputs. Also, avoid treating ip:port strings as multi‑octet patterns and harden ParseCIDR error handling.

Apply this diff:

-               // Try to parse as multi-octet range
-               if strings.Count(cidr, ".") == 3 {
+               // Try to parse as multi-octet range (IPv4 only; avoid ip:port or CIDR-with-slash here)
+               if strings.Count(cidr, ".") == 3 && !strings.Contains(cidr, ":") && !strings.Contains(cidr, "/") {
                    ips, err := mapcidr.ExpandIPPattern(cidr)
                    if err != nil {
                        gologger.Fatal().Msgf("%s\n", err)
                    }

-                   for _, ip := range ips {
-                       ipCidr := ip.String() + "/32"
+                   for _, ip := range ips {
+                       ipStr := ip.String()
+                       // Maintain parity with commonFunc filtering for expanded IPs
+                       if len(matchIPSet) > 0 {
+                           if _, ok := matchIPSet[ipStr]; !ok {
+                               continue
+                           }
+                       } else if len(filterIPSet) > 0 {
+                           if _, ok := filterIPSet[ipStr]; ok {
+                               continue
+                           }
+                       }
+                       ipCidr := ipStr + "/32"
                        if options.Aggregate || options.Shuffle || hasSort || options.AggregateApprox || options.Count {
-                           _, ipnet, _ := net.ParseCIDR(ipCidr)
-                           allCidrs = append(allCidrs, ipnet)
+                           if _, ipnet, err := net.ParseCIDR(ipCidr); err != nil {
+                               gologger.Fatal().Msgf("%s\n", err)
+                           } else {
+                               allCidrs = append(allCidrs, ipnet)
+                           }
                        } else {
                            commonFunc(ipCidr, outputchan)
                        }
                    }
                    continue
                }

Add the following helper initialization inside process(), once, just after the var block and before the main loop (reuse prepareIPsFromCidrFlagList):

// Precompute exact-match sets for expanded IP filtering parity
matchIPSet := map[string]struct{}{}
filterIPSet := map[string]struct{}{}
if options.MatchIP != nil {
    for _, ip := range prepareIPsFromCidrFlagList(options.MatchIP) {
        matchIPSet[ip] = struct{}{}
    }
}
if options.FilterIP != nil {
    for _, ip := range prepareIPsFromCidrFlagList(options.FilterIP) {
        filterIPSet[ip] = struct{}{}
    }
}
🤖 Prompt for AI Agents
In cmd/mapcidr/main.go around lines 421-442: expanded multi-octet ranges
currently bypass per-IP MatchIP/FilterIP when
Aggregate/Shuffle/Sort/AggregateApprox/Count are enabled and also can
misinterpret ip:port as a multi-octet pattern and ignore ParseCIDR errors; to
fix, add the suggested precomputed matchIPSet and filterIPSet once after the var
block and before the main loop (reuse prepareIPsFromCidrFlagList), then inside
the expanded-IP branch (the loop over ips) first skip patterns containing ':' so
we don't treat ip:port as a multi-octet range, for each expanded ip apply
MatchIP/FilterIP using the precomputed sets (skip or continue if filtered or
doesn't match), build ipCidr := ip.String()+"/32" and call net.ParseCIDR on
ipCidr and check for parse error (handle/log/continue on error) before appending
ipnet to allCidrs or calling commonFunc — this ensures expanded IPs follow the
same match/filter semantics and ParseCIDR failures are handled.

// Add IPs into ipRangeList which are passed as input. Example - "192.168.0.0-192.168.0.5"
var ipRange []net.IP
for _, ipstr := range strings.Split(cidr, "-") {
ipRange = append(ipRange, net.ParseIP(ipstr))
Expand Down Expand Up @@ -452,8 +472,8 @@ func process(wg *sync.WaitGroup, chancidr, outputchan chan string) {
continue
}

// In case of coalesce/shuffle we need to know all the cidrs and aggregate them by calling the proper function
if options.Aggregate || options.Shuffle || hasSort || options.AggregateApprox || options.Count {
// In case of coalesce/shuffle we need to know all the cidrs and aggregate them by calling the proper function
_ = ranger.Add(cidr)
allCidrs = append(allCidrs, pCidr)
} else {
Expand Down
61 changes: 61 additions & 0 deletions cmd/mapcidr/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,67 @@ func TestProcess(t *testing.T) {
Aggregate: true,
},
expectedOutput: []string{"10.0.0.0/32", "10.0.0.2/31"},
}, {
name: "MultiOctetRangeExpansion",
chancidr: make(chan string),
outputchan: make(chan string),
options: Options{
FileCidr: []string{"192.168.0-1.1-2"},
},
expectedOutput: []string{
"192.168.0.1", "192.168.0.2",
"192.168.1.1", "192.168.1.2",
},
},
{
name: "MultiOctetRangeWithFilter",
chancidr: make(chan string),
outputchan: make(chan string),
options: Options{
FileCidr: []string{"192.168.0-1.1-2"},
FilterIP: []string{"192.168.1.1"},
},
expectedOutput: []string{
"192.168.0.1", "192.168.0.2",
"192.168.1.2",
},
},
{
name: "MultiOctetRangeAggregate",
chancidr: make(chan string),
outputchan: make(chan string),
options: Options{
FileCidr: []string{"10.0-1.0-1.0-1"},
Aggregate: true,
},
expectedOutput: []string{
"10.0.0.0/31", "10.0.1.0/31",
"10.1.0.0/31", "10.1.1.0/31",
},
},
{
name: "MultiOctetRangeSortAscending",
chancidr: make(chan string),
outputchan: make(chan string),
options: Options{
FileCidr: []string{"10.0.0-0.2-3"},
SortAscending: true,
},
expectedOutput: []string{
"10.0.0.2", "10.0.0.3",
},
},
{
name: "MultiOctetRangeSortDescending",
chancidr: make(chan string),
outputchan: make(chan string),
options: Options{
FileCidr: []string{"10.0.1-2.1"},
SortDescending: true,
},
expectedOutput: []string{
"10.0.2.1", "10.0.1.1",
},
},
}
var wg sync.WaitGroup
Expand Down
68 changes: 68 additions & 0 deletions ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -1209,3 +1209,71 @@ func IpRangeToCIDR(start, end string) ([]string, error) {
}
return cidr, nil
}

/*
ExpandIPPattern expands an IPv4 pattern string into a list of net.IP addresses.
The pattern must be in the form of four octets separated by dots (a.b.c.d).
*/
func ExpandIPPattern(pattern string) ([]net.IP, error) {
parts := strings.Split(pattern, ".")
if len(parts) != 4 {
return nil, fmt.Errorf("invalid IP pattern: %s", pattern)
}

var octets [][]int
for _, part := range parts {
if strings.Contains(part, "-") {
bounds := strings.Split(part, "-")
if len(bounds) != 2 {
return nil, fmt.Errorf("invalid range in %s", part)
}

start, err1 := strconv.Atoi(bounds[0])
end, err2 := strconv.Atoi(bounds[1])

if err1 != nil || err2 != nil || start > end {
return nil, fmt.Errorf("invalid range: %s", part)
}

var nums []int
for i := start; i <= end; i++ {
nums = append(nums, i)
}
octets = append(octets, nums)

} else {
v, err := strconv.Atoi(part)
if err != nil {
return nil, fmt.Errorf("invalid octet: %s", part)
}

octets = append(octets, []int{v})
}
}

var ips []net.IP
for _, o1 := range octets[0] {
if o1 < 0 || o1 > 255 {
return nil, fmt.Errorf("invalid octet value: %d", o1)
}
for _, o2 := range octets[1] {
if o2 < 0 || o2 > 255 {
return nil, fmt.Errorf("invalid octet value: %d", o2)
}
for _, o3 := range octets[2] {
if o3 < 0 || o3 > 255 {
return nil, fmt.Errorf("invalid octet value: %d", o3)
}
for _, o4 := range octets[3] {
if o4 < 0 || o4 > 255 {
return nil, fmt.Errorf("invalid octet value: %d", o4)
}
ip := net.IPv4(byte(o1), byte(o2), byte(o3), byte(o4))
ips = append(ips, ip)
}
}
}
}

return ips, nil
}
Loading