]> source.dussan.org Git - poi.git/commitdiff
bug 58443: prevent corrupted workbooks by checking for overlapping merged regions...
authorJaven O'Neal <onealj@apache.org>
Sat, 31 Oct 2015 10:22:19 +0000 (10:22 +0000)
committerJaven O'Neal <onealj@apache.org>
Sat, 31 Oct 2015 10:22:19 +0000 (10:22 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711586 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java

index f1412af42b646cd22e101c5189d1f55c48ecefd3..a97c8782f590bc05c2328e2399d9eb551c0e8cc1 100644 (file)
@@ -673,22 +673,13 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
         _sheet.setGridsPrinted(value);
     }
 
-    /**
-     * @deprecated (Aug-2008) use <tt>CellRangeAddress</tt> instead of <tt>Region</tt>
-     */
-    public int addMergedRegion(org.apache.poi.ss.util.Region region) {
-        return _sheet.addMergedRegion(region.getRowFrom(),
-                region.getColumnFrom(),
-                //(short) region.getRowTo(),
-                region.getRowTo(),
-                region.getColumnTo());
-    }
-
     /**
      * adds a merged region of cells (hence those cells form one)
      *
      * @param region (rowfrom/colfrom-rowto/colto) to merge
      * @return index of this region
+     * @throws IllegalStateException if region intersects with an existing merged region
+     * or multi-cell array formula on this sheet
      */
     public int addMergedRegion(CellRangeAddress region) {
         region.validate(SpreadsheetVersion.EXCEL97);
@@ -696,6 +687,10 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
         // throw IllegalStateException if the argument CellRangeAddress intersects with
         // a multi-cell array formula defined in this sheet
         validateArrayFormulas(region);
+        
+        // Throw IllegalStateException if the argument CellRangeAddress intersects with
+        // a merged region already in this sheet
+        validateMergedRegions(region);
 
         return _sheet.addMergedRegion(region.getFirstRow(),
                 region.getFirstColumn(),
@@ -730,6 +725,15 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
         }
 
     }
+    
+    private void validateMergedRegions(CellRangeAddress candidateRegion) {
+        for (final CellRangeAddress existingRegion : getMergedRegions()) {
+            if (existingRegion.intersects(candidateRegion)) {
+                throw new IllegalStateException("Cannot add merged region " + candidateRegion.formatAsString() +
+                        " to sheet because it overlaps with an existing merged region (" + existingRegion.formatAsString() + ").");
+            }
+        }
+    }
 
     /**
      * Control if Excel should be asked to recalculate all formulas on this sheet
index 9561cb9f5bc889578645f55ada16b4e94028e968..92267fce72006ed9afb298a9af6c51341109d33a 100644 (file)
@@ -17,6 +17,8 @@
 
 package org.apache.poi.ss.util;
 
+import java.awt.Rectangle;
+
 import org.apache.poi.ss.SpreadsheetVersion;
 
 
@@ -123,6 +125,44 @@ public abstract class CellRangeAddressBase {
                return _firstRow <= rowInd && rowInd <= _lastRow &&
                                _firstCol <= colInd && colInd <= _lastCol;
        }
+       
+       /**
+        * Determines whether or not this CellRangeAddress and the specified CellRangeAddress intersect.
+        *
+        * @param other
+        * @return
+        */
+       public boolean intersects(CellRangeAddressBase other) {
+               // see java.awt.Rectangle.intersects
+               // http://stackoverflow.com/questions/13390333/two-rectangles-intersection
+           
+               // TODO: Replace with an intersection code that doesn't rely on java.awt
+               return getRectangle().intersects(other.getRectangle());
+       }
+       
+       // TODO: Replace with an intersection code that doesn't rely on java.awt
+       // Don't let this temporary implementation detail leak outside of this class
+       private final Rectangle getRectangle() {
+               int firstRow, firstCol, lastRow, lastCol;
+               
+               if (!isFullColumnRange()) {
+                       firstRow = Math.min(_firstRow, _lastRow);
+                       lastRow = Math.max(_firstRow,  _lastRow);
+               }
+               else {
+                       firstRow = 0;
+                       lastRow = Integer.MAX_VALUE;
+               }
+               if (!isFullRowRange()) {
+                       firstCol = Math.min(_firstCol, _lastCol);
+                       lastCol = Math.max(_firstCol, _lastCol);
+               }
+               else {
+                       firstCol = 0;
+                       lastCol = Integer.MAX_VALUE;
+               }
+               return new Rectangle(firstRow, firstCol, lastRow-firstRow+1, lastCol-firstCol+1);
+       }
 
        /**
         * @param firstCol column number for the upper left hand corner
index 1406ee2bf16e306fde23dce9a3362d320c7f1fbb..4651a204ea32459245e4617a37ba40c709147ed9 100644 (file)
@@ -327,6 +327,8 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      *
      * @param region (rowfrom/colfrom-rowto/colto) to merge
      * @return index of this region
+     * @throws IllegalStateException if region intersects with a multi-cell array formula
+     * @throws IllegalStateException if region intersects with an existing region on this sheet
      */
     @Override
     public int addMergedRegion(CellRangeAddress region) {
@@ -335,6 +337,10 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         // throw IllegalStateException if the argument CellRangeAddress intersects with
         // a multi-cell array formula defined in this sheet
         validateArrayFormulas(region);
+        
+        // Throw IllegalStateException if the argument CellRangeAddress intersects with
+        // a merged region already in this sheet 
+        validateMergedRegions(region);
 
         CTMergeCells ctMergeCells = worksheet.isSetMergeCells() ? worksheet.getMergeCells() : worksheet.addNewMergeCells();
         CTMergeCell ctMergeCell = ctMergeCells.addNewMergeCell();
@@ -342,6 +348,12 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         return ctMergeCells.sizeOfMergeCellArray();
     }
 
+    /**
+     * Verify that the candidate region does not intersect with an existing multi-cell array formula in this sheet
+     *
+     * @param region
+     * @throws IllegalStateException if candidate region intersects an existing array formula in this sheet
+     */
     private void validateArrayFormulas(CellRangeAddress region){
         int firstRow = region.getFirstRow();
         int firstColumn = region.getFirstColumn();
@@ -369,6 +381,20 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         }
 
     }
+    /**
+     * Verify that candidate region does not intersect with an existing merged region in this sheet
+     *
+     * @param candidateRegion
+     * @throws IllegalStateException if candidate region intersects an existing merged region in this sheet
+     */
+    private void validateMergedRegions(CellRangeAddress candidateRegion) {
+        for (final CellRangeAddress existingRegion : getMergedRegions()) {
+            if (existingRegion.intersects(candidateRegion)) {
+                throw new IllegalStateException("Cannot add merged region " + candidateRegion.formatAsString() +
+                        " to sheet because it overlaps with an existing merged region (" + existingRegion.formatAsString() + ").");
+            }
+        }
+    }
 
     /**
      * Adjusts the column width to fit the contents.
index 945bdf86a83e5ef52b0f95c3bea7958c8f8e849d..875a780d67adede204ea140e968966738174da1f 100644 (file)
@@ -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();
index 24c596b4ffc1207d6fd6b8f4d366ee4f4d2ec7d5..e4151360fb451fd4288c8b5ba9dae3c3284b8d23 100644 (file)
@@ -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);
 
index ca69fa626670e40f1dd52f156346be7c25163291..9c3ca693add5c3d11f27a81dcd373174dbfc8c63 100644 (file)
@@ -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());
 
     }
 
index 5616f79f38e0c7c319aa65a36357de9cd5363939..5c35a2dff105481881d73b9b1affe660d43a5925 100644 (file)
@@ -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));
+    }
 }