From 8a6d6c8cba22bf8f1c796d7f97cef406472edcb2 Mon Sep 17 00:00:00 2001
From: Javen O'Neal <onealj@apache.org>
Date: Tue, 7 Nov 2017 07:21:24 +0000
Subject: [PATCH] bug 61730: remove CellRangeAddressBase which is eager. The
 lazy iterator is safer, less likely to cause an OOM/DoS.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1814461 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/ss/util/CellRangeAddressBase.java     | 21 +----------
 .../apache/poi/ss/util/TestCellRangeUtil.java | 35 +++++++++++++++----
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java b/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
index 0433475f75..1224e479d6 100644
--- a/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
+++ b/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
@@ -282,26 +282,7 @@ public abstract class CellRangeAddressBase implements Iterable<CellAddress> {
 	public int getNumberOfCells() {
 		return (_lastRow - _firstRow + 1) * (_lastCol - _firstCol + 1);
 	}
-	
-	public List<CellAddress> getCellAddresses(boolean rowMajorOrder) {
-		List<CellAddress> addresses = new ArrayList<>();
-		if (rowMajorOrder) {
-			for (int r = _firstRow; r <= _lastRow; r++) {
-				for (int c = _firstCol; c <= _lastCol; c++) {
-					addresses.add(new CellAddress(r, c));
-				}
-			}
-		}
-		else {
-			for (int c = _firstCol; c <= _lastCol; c++) {
-				for (int r = _firstRow; r <= _lastRow; r++) {
-					addresses.add(new CellAddress(r, c));
-				}
-			}
-		}
-		return Collections.unmodifiableList(addresses);
-	}
-	
+
 	/**
 	 * Returns an iterator over the CellAddresses in this cell range in row-major order.
 	 * @since POI 4.0.0
diff --git a/src/testcases/org/apache/poi/ss/util/TestCellRangeUtil.java b/src/testcases/org/apache/poi/ss/util/TestCellRangeUtil.java
index 8bac4b580e..786fdaaaf5 100644
--- a/src/testcases/org/apache/poi/ss/util/TestCellRangeUtil.java
+++ b/src/testcases/org/apache/poi/ss/util/TestCellRangeUtil.java
@@ -21,6 +21,10 @@ import org.junit.Test;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assume.assumeTrue;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Iterator;
+import org.apache.commons.collections4.IteratorUtils;
 
 /**
  * Tests CellRangeUtil.
@@ -47,23 +51,40 @@ public final class TestCellRangeUtil {
         //    A B
         //  1 x x   A1,A2,B1,B2 --> A1:B2
         //  2 x x
-        assertArrayEquals(asArray(A1_B2), merge(A1, B1, A2, B2));
-        assertArrayEquals(asArray(A1_B2), merge(A1, B2, A2, B1));
+        assertCellRangesEqual(asArray(A1_B2), merge(A1, B1, A2, B2));
+        assertCellRangesEqual(asArray(A1_B2), merge(A1, B2, A2, B1));
 
         // Partially mergeable: multiple possible mergings
         //    A B
         //  1 x x   A1,A2,B1 --> A1:B1,A2 or A1:A2,B1
         //  2 x 
-        assertArrayEquals(asArray(A1_B1, A2), merge(A1, B1, A2));
-        assertArrayEquals(asArray(A1_A2, B1), merge(A2, A1, B1));
-        assertArrayEquals(asArray(A1_B1, A2), merge(B1, A2, A1));
+        assertCellRangesEqual(asArray(A1_B1, A2), merge(A1, B1, A2));
+        assertCellRangesEqual(asArray(A1_A2, B1), merge(A2, A1, B1));
+        assertCellRangesEqual(asArray(A1_B1, A2), merge(B1, A2, A1));
 
         // Not mergeable
         //    A B
         //  1 x     A1,B2 --> A1,B2
         //  2   x
-        assertArrayEquals(asArray(A1, B2), merge(A1, B2));
-        assertArrayEquals(asArray(B2, A1), merge(B2, A1));
+        assertCellRangesEqual(asArray(A1, B2), merge(A1, B2));
+        assertCellRangesEqual(asArray(B2, A1), merge(B2, A1));
+    }
+
+    private void assertCellRangesEqual(CellRangeAddress[] a, CellRangeAddress[] b) {
+        assertEquals(getCellAddresses(a), getCellAddresses(b));
+        assertArrayEquals(a, b);
+    }
+
+    private static Set<CellAddress> getCellAddresses(CellRangeAddress[] ranges) {
+        final Set<CellAddress> set = new HashSet<>();
+        for (final CellRangeAddress range : ranges) {
+            set.addAll(asSet(range.iterator()));
+        }
+        return set;
+    }
+            
+    private static <T> Set<T> asSet(Iterator<T> iterator) {
+        return new HashSet<T>(IteratorUtils.toList(iterator));
     }
 
     private static <T> T[] asArray(T...ts) {