mirror of https://github.com/apache/poi.git
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
This commit is contained in:
parent
c158ddea1c
commit
6456105e37
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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();
|
||||
|
|
|
@ -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);
|
||||
|
||||
|
|
|
@ -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());
|
||||
|
||||
}
|
||||
|
||||
|
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue