Merge pull request #17 from astellingwerf/bug/6

fix: summarize not working correctly
demskie · Jul 26, 2021 · a035884 · a035884
2 parents 25dd058 + b479f79
commit a035884
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 18 deletions.
diff --git a/src/shared.ts b/src/shared.ts
@@ -19,26 +19,28 @@ export function sortNetworks(networks: Network[]) {
   sort.nativeSort(networks);
 }
 
-export function summarizeSortedNetworks(sorted: Network[]) {
-  const summarized = [] as Network[];
-  for (let idx = 0; idx < sorted.length; idx++) {
+function increaseSizeByOneBit(network: Network): Network {
+  const wider = network.setCIDR(network.cidr() - 1);
+  wider.addr.applySubnetMask(wider.cidr());
+  return wider;
+}
+
+export function summarizeSortedNetworks(sorted: Network[]): Network[] {
+  const summarized: Network[] = [sorted[0]];
+  for (let idx = 1; idx < sorted.length; idx++) {
+    if (summarized[summarized.length - 1].contains(sorted[idx])) {
+      continue;
+    }
     summarized.push(sorted[idx]);
-    let skipped = 0;
-    for (let i = idx + 1; i < sorted.length; i++) {
-      if (sorted[idx].contains(sorted[i])) {
-        skipped++;
-        continue;
-      }
-      if (sorted[idx].cidr() === sorted[i].cidr()) {
-        if (sorted[idx].adjacent(sorted[i])) {
-          sorted[idx].setCIDR(sorted[idx].cidr() - 1);
-          skipped++;
-          continue;
-        }
+    while (summarized.length >= 2) {
+      const a = summarized[summarized.length - 2];
+      const b = summarized[summarized.length - 1];
+      if (a.cidr() != b.cidr() || !a.addr.isBaseAddress(a.cidr() - 1) || !a.adjacent(b)) {
+        break;
       }
-      break;
+      increaseSizeByOneBit(a);
+      summarized.pop();
     }
-    idx += skipped;
   }
   return summarized;
 }

diff --git a/src/tests/index.test.ts b/src/tests/index.test.ts
@@ -1,5 +1,18 @@
 import * as index from "../index";
 
+expect.extend({
+  toContainAddresses(received: string[], addresses: string[]) {
+    const notFound = addresses.filter((i: string) => {
+      return undefined === received.find((value: string) => index.networkContainsAddress(value, i));
+    });
+    if (notFound.length > 0) {
+      return { message: () => `expected to find ${notFound} in ${received}`, pass: false };
+    } else {
+      return { message: () => `expected not to find ${addresses} in ${received}`, pass: true };
+    }
+  }
+});
+
 test("sanity check baseAddress #1", () => {
   const output = index.baseAddress("192.168.200.113/24", true);
   expect(output).toEqual("192.168.200.0");
@@ -397,14 +410,40 @@ test("sanity check summarize #2", () => {
 
 test("sanity check summarize #3", () => {
   const output = index.summarize(["192.168.0.0", "192.168.0.2/31", "192.168.0.3", "192.168.0.4/31"], true);
-  expect(output).toEqual(["192.168.0.0/32", "192.168.0.2/30"]);
+  expect(output).toEqual(["192.168.0.0/32", "192.168.0.2/31", "192.168.0.4/31"]);
 });
 
 test("sanity check summarize #4", () => {
   const output = index.summarize(["192.168.0.0/31", "192.168.0.2/31", "192.168.0.3", "192.168.0.5/32"], true);
   expect(output).toEqual(["192.168.0.0/30", "192.168.0.5/32"]);
 });
 
+test("sanity check summarize #5", () => {
+  const input = [
+    "10.9.201.68",
+    "10.9.201.71",
+    "10.9.201.70",
+    "10.9.201.65",
+    "10.9.201.72",
+    "10.9.201.75",
+    "10.9.201.67",
+    "10.9.201.66",
+    "10.9.201.69",
+    "10.9.201.74"
+  ]; // Shuffled, but effectively ranging from 10.9.201.65 to 10.9.201.72 + 10.9.201.74 + 10.9.201.75
+
+  const output = index.summarize(input, true);
+  expect(output).toBeDefined();
+  expect(output).not.toContainAddresses(["192.168.0.0"]);
+  expect(output).not.toContainAddresses(["10.9.201.64"]);
+  expect(output).not.toContainAddresses(["10.9.201.73"]);
+  expect(output).not.toContainAddresses(["10.9.201.76"]);
+  expect(output).toContainAddresses(input);
+
+  const expectedOutput = ["10.9.201.65/32", "10.9.201.66/31", "10.9.201.68/30", "10.9.201.72/32", "10.9.201.74/31"];
+  expect(output).toEqual(expectedOutput);
+});
+
 // https://tools.ietf.org/html/rfc5952#section-4
 
 test("sanity check IPv6 parsing #1", () => {