You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by on...@apache.org on 2015/10/31 11:22:19 UTC
svn commit: r1711586 - in /poi/trunk/src:
java/org/apache/poi/hssf/usermodel/ java/org/apache/poi/ss/util/
ooxml/java/org/apache/poi/xssf/usermodel/
testcases/org/apache/poi/ss/usermodel/ testcases/org/apache/poi/ss/util/
Author: onealj
Date: Sat Oct 31 10:22:19 2015
New Revision: 1711586
URL: http://svn.apache.org/viewvc?rev=1711586&view=rev
Log:
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)
Modified:
poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java
Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java Sat Oct 31 10:22:19 2015
@@ -674,21 +674,12 @@ public final class HSSFSheet implements
}
/**
- * @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
// 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
}
}
+
+ 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
Modified: poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java Sat Oct 31 10:22:19 2015
@@ -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 CellRangeAddressBa
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
Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java Sat Oct 31 10:22:19 2015
@@ -327,6 +327,8 @@ public class XSSFSheet extends POIXMLDoc
*
* @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 POIXMLDoc
// 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 POIXMLDoc
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 POIXMLDoc
}
}
+ /**
+ * 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.
Modified: poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java Sat Oct 31 10:22:19 2015
@@ -143,7 +143,7 @@ public abstract class BaseTestBugzillaIs
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();
Modified: poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java Sat Oct 31 10:22:19 2015
@@ -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);
Modified: poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java Sat Oct 31 10:22:19 2015
@@ -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());
}
Modified: poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java?rev=1711586&r1=1711585&r2=1711586&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellRangeAddress.java Sat Oct 31 10:22:19 2015
@@ -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
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));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org