bug 58443: prevent corrupted workbooks by checking for overlapping merged regions before adding a merged region to a sheet; fix unit tests that produced corrupt workbooks; remove deprecated HSSFSheet#addMergedRegion(Region)

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711586 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 79 insertions, 9 deletions

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
index 945bdf86a8..875a780d67 100644
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
@@ -143,7 +143,7 @@ public abstract class BaseTestBugzillaIssues {
        Sheet template = wb.getSheetAt(0);
 
        template.addMergedRegion(new CellRangeAddress(0, 1, 0, 2));
-       template.addMergedRegion(new CellRangeAddress(1, 2, 0, 2));
+       template.addMergedRegion(new CellRangeAddress(2, 3, 0, 2));
 
        Sheet clone = wb.cloneSheet(0);
        int originalMerged = template.getNumMergedRegions();
diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
index 24c596b4ff..e4151360fb 100644
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
@@ -257,6 +257,46 @@ public abstract class BaseTestSheet {
         assertEquals(3, sheetP.getPrintSetup().getCopies());
         wb2.close();
     }
+    
+    /**
+     * Dissallow creating wholly or partially overlapping merged regions
+     * as this results in a corrupted workbook
+     */
+    @Test
+    public void addOverlappingMergedRegions() {
+        final Workbook wb = _testDataProvider.createWorkbook();
+        final Sheet sheet = wb.createSheet();
+        
+        final CellRangeAddress baseRegion = new CellRangeAddress(0, 1, 0, 1);
+        sheet.addMergedRegion(baseRegion);
+        
+        try {
+            final CellRangeAddress duplicateRegion = new CellRangeAddress(0, 1, 0, 1);
+            sheet.addMergedRegion(duplicateRegion);
+            fail("Should not be able to add a merged region if sheet already contains the same merged region");
+        } catch (final IllegalStateException e) { } //expected
+        
+        try {
+            final CellRangeAddress partiallyOverlappingRegion = new CellRangeAddress(1, 2, 1, 2);
+            sheet.addMergedRegion(partiallyOverlappingRegion);
+            fail("Should not be able to add a merged region if it partially overlaps with an existing merged region");
+        } catch (final IllegalStateException e) { } //expected
+        
+        try {
+            final CellRangeAddress subsetRegion = new CellRangeAddress(0, 1, 0, 0);
+            sheet.addMergedRegion(subsetRegion);
+            fail("Should not be able to add a merged region if it is a formal subset of an existing merged region");
+        } catch (final IllegalStateException e) { } //expected
+        
+        try {
+            final CellRangeAddress supersetRegion = new CellRangeAddress(0, 2, 0, 2);
+            sheet.addMergedRegion(supersetRegion);
+            fail("Should not be able to add a merged region if it is a formal superset of an existing merged region");
+        } catch (final IllegalStateException e) { } //expected
+        
+        final CellRangeAddress disjointRegion = new CellRangeAddress(10, 11, 10, 11);
+        sheet.addMergedRegion(disjointRegion); //allowed
+    }
 
     /**
      * Test adding merged regions. If the region's bounds are outside of the allowed range
@@ -310,13 +350,13 @@ public abstract class BaseTestSheet {
         Sheet sheet = wb.createSheet();
         CellRangeAddress region = new CellRangeAddress(0, 1, 0, 1);
         sheet.addMergedRegion(region);
-        region = new CellRangeAddress(1, 2, 0, 1);
+        region = new CellRangeAddress(2, 3, 0, 1);
         sheet.addMergedRegion(region);
 
         sheet.removeMergedRegion(0);
 
         region = sheet.getMergedRegion(0);
-        assertEquals("Left over region should be starting at row 1", 1, region.getFirstRow());
+        assertEquals("Left over region should be starting at row 2", 2, region.getFirstRow());
 
         sheet.removeMergedRegion(0);
 
diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
index ca69fa6266..9c3ca693ad 100644
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
@@ -448,7 +448,7 @@ public abstract class BaseTestWorkbook {
         sheet.createRow(0).createCell(0).setCellValue("Test");
         sheet.createRow(1).createCell(0).setCellValue(36.6);
         sheet.addMergedRegion(new CellRangeAddress(0, 1, 0, 2));
-        sheet.addMergedRegion(new CellRangeAddress(1, 2, 0, 2));
+        sheet.addMergedRegion(new CellRangeAddress(2, 3, 0, 2));
         assertTrue(sheet.isSelected());
 
         Sheet clonedSheet = book.cloneSheet(0);
@@ -457,16 +457,16 @@ public abstract class BaseTestWorkbook {
         assertEquals(2, clonedSheet.getNumMergedRegions());
         assertFalse(clonedSheet.isSelected());
 
-        //cloned sheet is a deep copy, adding rows in the original does not affect the clone
+        //cloned sheet is a deep copy, adding rows or merged regions in the original does not affect the clone
         sheet.createRow(2).createCell(0).setCellValue(1);
-        sheet.addMergedRegion(new CellRangeAddress(0, 2, 0, 2));
-        assertEquals(2, clonedSheet.getPhysicalNumberOfRows());
+        sheet.addMergedRegion(new CellRangeAddress(4, 5, 0, 2));
         assertEquals(2, clonedSheet.getPhysicalNumberOfRows());
+        assertEquals(2, clonedSheet.getNumMergedRegions());
 
         clonedSheet.createRow(2).createCell(0).setCellValue(1);
-        clonedSheet.addMergedRegion(new CellRangeAddress(0, 2, 0, 2));
-        assertEquals(3, clonedSheet.getPhysicalNumberOfRows());
+        clonedSheet.addMergedRegion(new CellRangeAddress(6, 7, 0, 2));
         assertEquals(3, clonedSheet.getPhysicalNumberOfRows());
+        assertEquals(3, clonedSheet.getNumMergedRegions());
 
     }
 
diff --git a/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java b/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java
index 5616f79f38..5c35a2dff1 100644
--- a/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java
+++ b/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java
@@ -17,6 +17,8 @@ limitations under the License.
 
 package org.apache.poi.ss.util;
 
+import static org.junit.Assert.fail;
+
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 
@@ -233,4 +235,32 @@ public final class TestCellRangeAddress extends TestCase {
         assertEquals(4, ref.getMinColumn());
         assertEquals(10, ref.getMaxColumn());
     }
+    
+    public void testIntersects() {
+        final CellRangeAddress baseRegion = new CellRangeAddress(0, 1, 0, 1);
+        
+        final CellRangeAddress duplicateRegion = new CellRangeAddress(0, 1, 0, 1);
+        assertIntersects(baseRegion, duplicateRegion);
+        
+        final CellRangeAddress partiallyOverlappingRegion = new CellRangeAddress(1, 2, 1, 2);
+        assertIntersects(baseRegion, partiallyOverlappingRegion);
+        
+        final CellRangeAddress subsetRegion = new CellRangeAddress(0, 1, 0, 0);
+        assertIntersects(baseRegion, subsetRegion);
+    
+        final CellRangeAddress supersetRegion = new CellRangeAddress(0, 2, 0, 2);
+        assertIntersects(baseRegion, supersetRegion);
+        
+        final CellRangeAddress disjointRegion = new CellRangeAddress(10, 11, 10, 11);
+        assertNotIntersects(baseRegion, disjointRegion);
+    }
+    
+    private static void assertIntersects(CellRangeAddress regionA, CellRangeAddress regionB) {
+        assertTrue(regionA.intersects(regionB));
+        assertTrue(regionB.intersects(regionA));
+    }
+    private static void assertNotIntersects(CellRangeAddress regionA, CellRangeAddress regionB) {
+        assertFalse(regionA.intersects(regionB));
+        assertFalse(regionB.intersects(regionA));
+    }
 }