Author: centic
Date: Thu Sep 28 09:56:45 2017
New Revision: 1809967
URL: http://svn.apache.org/viewvc?rev=1809967&view=rev
Log:
Fix bug 61516: when copying cells with formulas we should properly check for
references that are invalid afterwards.
Modified:
poi/site/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
Modified: poi/site/src/documentation/content/xdocs/status.xml
URL:
http://svn.apache.org/viewvc/poi/site/src/documentation/content/xdocs/status.xml?rev=1809967&r1=1809966&r2=1809967&view=diff
==============================================================================
--- poi/site/src/documentation/content/xdocs/status.xml (original)
+++ poi/site/src/documentation/content/xdocs/status.xml Thu Sep 28 09:56:45 2017
@@ -82,6 +82,7 @@
<summary-item>SXSSF: fix XML processing - unicode surrogates
and line breaks (#61048, #61246)</summary-item>
</summary>
<actions>
+ <action dev="PD" type="fix" fixes-bug="61516" module="SS
Common">Correctly handle references that end up outside the workbook when cells
with formulas are copied</action>
<action dev="PD" type="fix" fixes-bug="61478" module="OPC">POI
OOXML-Schema lookup uses wrong classloader</action>
<action dev="PD" type="fix" fixes-bug="61470" module="XWPF">Handle
ruby (phonetic) elements in XWPFRun</action>
<action dev="PD" type="fix" fixes-bug="61381"
module="POIFS">PushbackInputStreams passed to ZipHelper may not hold 8
bytes</action>
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java?rev=1809967&r1=1809966&r2=1809967&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java Thu Sep 28
09:56:45 2017
@@ -122,14 +122,12 @@ public final class FormulaShifter {
@Override
public String toString() {
- StringBuffer sb = new StringBuffer();
-
- sb.append(getClass().getName());
- sb.append(" [");
- sb.append(_firstMovedIndex);
- sb.append(_lastMovedIndex);
- sb.append(_amountToMove);
- return sb.toString();
+ return getClass().getName() +
+ " [" +
+ _firstMovedIndex +
+ _lastMovedIndex +
+ _amountToMove +
+ "]";
}
/**
@@ -463,18 +461,27 @@ public final class FormulaShifter {
/**
* Modifies rptg in-place and return a reference to rptg if the cell
reference
* would move due to a row copy operation
- * Returns <code>null</code> or {@link #RefErrorPtg} if no change was made
+ * Returns <code>null</code> or {@link RefErrorPtg} if no change was made
*
- * @param aptg
+ * @param rptg The REF that is copied
* @return The Ptg reference if the cell would move due to copy, otherwise
null
*/
private Ptg rowCopyRefPtg(RefPtgBase rptg) {
final int refRow = rptg.getRow();
if (rptg.isRowRelative()) {
+ // check new location where the ref is located
final int destRowIndex = _firstMovedIndex + _amountToMove;
- if (destRowIndex < 0 || _version.getLastRowIndex() < destRowIndex)
+ if (destRowIndex < 0 || _version.getLastRowIndex() < destRowIndex)
{
return createDeletedRef(rptg);
- rptg.setRow(refRow + _amountToMove);
+ }
+
+ // check new location where the ref points to
+ final int newRowIndex = refRow + _amountToMove;
+ if(newRowIndex < 0 || _version.getLastRowIndex() < newRowIndex) {
+ return createDeletedRef(rptg);
+ }
+
+ rptg.setRow(newRowIndex);
return rptg;
}
return null;
@@ -483,9 +490,9 @@ public final class FormulaShifter {
/**
* Modifies aptg in-place and return a reference to aptg if the first or
last row of
* of the Area reference would move due to a row copy operation
- * Returns <code>null</code> or {@link #AreaErrPtg} if no change was made
+ * Returns <code>null</code> or {@link AreaErrPtg} if no change was made
*
- * @param aptg
+ * @param aptg The Area that is copied
* @return null, AreaErrPtg, or modified aptg
*/
private Ptg rowCopyAreaPtg(AreaPtgBase aptg) {
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java?rev=1809967&r1=1809966&r2=1809967&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
Thu Sep 28 09:56:45 2017
@@ -64,17 +64,21 @@ import org.apache.poi.openxml4j.opc.Pack
import org.apache.poi.openxml4j.util.ZipSecureFile;
import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.ss.SpreadsheetVersion;
+import org.apache.poi.ss.formula.FormulaParser;
+import org.apache.poi.ss.formula.FormulaRenderer;
+import org.apache.poi.ss.formula.FormulaShifter;
+import org.apache.poi.ss.formula.FormulaType;
import org.apache.poi.ss.formula.WorkbookEvaluator;
import org.apache.poi.ss.formula.eval.ErrorEval;
import org.apache.poi.ss.formula.eval.NumberEval;
-import org.apache.poi.ss.formula.eval.ValueEval;
import org.apache.poi.ss.formula.functions.Function;
+import org.apache.poi.ss.formula.ptg.Ptg;
import org.apache.poi.ss.usermodel.*;
import org.apache.poi.ss.util.AreaReference;
import org.apache.poi.ss.util.CellRangeAddress;
import org.apache.poi.ss.util.CellReference;
import org.apache.poi.ss.util.CellUtil;
-import org.apache.poi.util.IOUtils;
import org.apache.poi.util.LocaleUtil;
import org.apache.poi.util.NullOutputStream;
import org.apache.poi.util.TempFile;
@@ -292,8 +296,7 @@ public final class TestXSSFBugs extends
*/
@Test
public void bug48539() throws IOException {
- XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("48539.xlsx");
- try {
+ try (XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("48539.xlsx")) {
assertEquals(3, wb.getNumberOfSheets());
assertEquals(0, wb.getNumberOfNames());
@@ -321,8 +324,6 @@ public final class TestXSSFBugs extends
// Now all of them
XSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
- } finally {
- wb.close();
}
}
@@ -358,7 +359,6 @@ public final class TestXSSFBugs extends
assertEquals("FFFF0000", cs.getFillForegroundXSSFColor().getARGBHex());
assertEquals("FFFF0000",
cs.getFillForegroundColorColor().getARGBHex());
- assertNotNull(cs.getFillBackgroundColor());
assertEquals(64, cs.getFillBackgroundColor());
assertEquals(null, cs.getFillBackgroundXSSFColor().getARGBHex());
assertEquals(null, cs.getFillBackgroundColorColor().getARGBHex());
@@ -1432,7 +1432,7 @@ public final class TestXSSFBugs extends
// KY: SUM(B1: IZ1)
/*double ky1Value =*/
-
evaluator.evaluate(wb.getSheetAt(0).getRow(0).getCell(310)).getNumberValue();
+ assertEquals(259.0,
evaluator.evaluate(wb.getSheetAt(0).getRow(0).getCell(310)).getNumberValue(),
0.0001);
// Assert
assertEquals(259.0, a1Value, 0.0);
@@ -1443,12 +1443,7 @@ public final class TestXSSFBugs extends
public void bug54436() throws IOException {
Workbook wb = XSSFTestDataSamples.openSampleWorkbook("54436.xlsx");
if
(!WorkbookEvaluator.getSupportedFunctionNames().contains("GETPIVOTDATA")) {
- Function func = new Function() {
- @Override
- public ValueEval evaluate(ValueEval[] args, int srcRowIndex,
int srcColumnIndex) {
- return ErrorEval.NA;
- }
- };
+ Function func = (args, srcRowIndex, srcColumnIndex) ->
ErrorEval.NA;
WorkbookEvaluator.registerFunction("GETPIVOTDATA", func);
}
@@ -1463,12 +1458,9 @@ public final class TestXSSFBugs extends
@Test(expected = EncryptedDocumentException.class)
public void bug55692_poifs() throws IOException {
// Via a POIFSFileSystem
- POIFSFileSystem fsP = new POIFSFileSystem(
-
POIDataSamples.getPOIFSInstance().openResourceAsStream("protect.xlsx"));
- try {
+ try (POIFSFileSystem fsP = new POIFSFileSystem(
+
POIDataSamples.getPOIFSInstance().openResourceAsStream("protect.xlsx"))) {
WorkbookFactory.create(fsP);
- } finally {
- fsP.close();
}
}
@@ -1763,15 +1755,11 @@ public final class TestXSSFBugs extends
}
}
- FileOutputStream fileOutStream = new FileOutputStream(outFile);
- try {
+ try (FileOutputStream fileOutStream = new FileOutputStream(outFile)) {
wb.write(fileOutStream);
- } finally {
- fileOutStream.close();
}
- FileInputStream is = new FileInputStream(outFile);
- try {
+ try (FileInputStream is = new FileInputStream(outFile)) {
Workbook newWB = null;
try {
if (wb instanceof XSSFWorkbook) {
@@ -1789,8 +1777,6 @@ public final class TestXSSFBugs extends
newWB.close();
}
}
- } finally {
- is.close();
}
}
@@ -2196,8 +2182,7 @@ public final class TestXSSFBugs extends
@Test
public void test57165() throws IOException {
- XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx");
- try {
+ try (XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx")) {
removeAllSheetsBut(3, wb);
wb.cloneSheet(0); // Throws exception here
wb.setSheetName(1, "New Sheet");
@@ -2205,15 +2190,12 @@ public final class TestXSSFBugs extends
XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb);
wbBack.close();
- } finally {
- wb.close();
}
}
@Test
public void test57165_create() throws IOException {
- XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx");
- try {
+ try (XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx")) {
removeAllSheetsBut(3, wb);
wb.createSheet("newsheet"); // Throws exception here
wb.setSheetName(1, "New Sheet");
@@ -2221,12 +2203,10 @@ public final class TestXSSFBugs extends
XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb);
wbBack.close();
- } finally {
- wb.close();
}
}
- private static void removeAllSheetsBut(int sheetIndex, Workbook wb) {
+ private static void
removeAllSheetsBut(@SuppressWarnings("SameParameterValue") int sheetIndex,
Workbook wb) {
int sheetNb = wb.getNumberOfSheets();
// Move this sheet at the first position
wb.setSheetOrder(wb.getSheetName(sheetIndex), 0);
@@ -2258,8 +2238,7 @@ public final class TestXSSFBugs extends
*/
@Test
public void testBug56820_Formula1() throws IOException {
- Workbook wb = new XSSFWorkbook();
- try {
+ try (Workbook wb = new XSSFWorkbook()) {
FormulaEvaluator evaluator =
wb.getCreationHelper().createFormulaEvaluator();
Sheet sh = wb.createSheet();
@@ -2274,8 +2253,6 @@ public final class TestXSSFBugs extends
assertEquals(2, A1, 0);
assertEquals(4, A2, 0); //<-- FAILS EXPECTATIONS
- } finally {
- wb.close();
}
}
@@ -2288,8 +2265,7 @@ public final class TestXSSFBugs extends
*/
@Test
public void testBug56820_Formula2() throws IOException {
- Workbook wb = new XSSFWorkbook();
- try {
+ try (Workbook wb = new XSSFWorkbook()) {
FormulaEvaluator evaluator =
wb.getCreationHelper().createFormulaEvaluator();
Sheet sh = wb.createSheet();
@@ -2304,15 +2280,12 @@ public final class TestXSSFBugs extends
assertEquals(2, A1, 0);
assertEquals(4, A2, 0);
- } finally {
- wb.close();
}
}
@Test
public void test56467() throws IOException {
- Workbook wb = XSSFTestDataSamples.openSampleWorkbook("picture.xlsx");
- try {
+ try (Workbook wb =
XSSFTestDataSamples.openSampleWorkbook("picture.xlsx")) {
Sheet orig = wb.getSheetAt(0);
assertNotNull(orig);
@@ -2325,8 +2298,6 @@ public final class TestXSSFBugs extends
}
}
- } finally {
- wb.close();
}
}
@@ -2966,7 +2937,6 @@ public final class TestXSSFBugs extends
@Ignore("bug 59442")
@Test
public void testSetRGBBackgroundColor() throws IOException {
-
XSSFWorkbook workbook = new XSSFWorkbook();
XSSFCell cell = workbook.createSheet().createRow(0).createCell(0);
@@ -3157,7 +3127,7 @@ public final class TestXSSFBugs extends
// we currently only populate the dimension during writing out
// to avoid having to iterate all rows/cells in each add/remove of a
row or cell
- IOUtils.write(wb, new NullOutputStream());
+ wb.write(new NullOutputStream());
assertEquals("B2:I5", ((XSSFSheet)
sheet).getCTWorksheet().getDimension().getRef());
@@ -3181,4 +3151,57 @@ public final class TestXSSFBugs extends
wb.close();
}
-}
\ No newline at end of file
+
+ @Test
+ public void bug61516(){
+ final String initialFormula = "A1";
+ final String expectedFormula = "#REF!"; // from ms excel
+
+ Workbook wb = new XSSFWorkbook();
+ Sheet sheet = wb.createSheet("sheet1");
+ sheet.createRow(0).createCell(0).setCellValue(1); // A1 = 1
+
+ {
+ Cell c3 = sheet.createRow(2).createCell(2);
+ c3.setCellFormula(initialFormula); // C3 = =A1
+ FormulaEvaluator evaluator =
wb.getCreationHelper().createFormulaEvaluator();
+ CellValue cellValue = evaluator.evaluate(c3);
+ assertEquals(1, cellValue.getNumberValue(), 0.0001);
+ }
+
+ {
+ FormulaShifter formulaShifter = FormulaShifter.createForRowCopy(0,
"sheet1", 2/*firstRowToShift*/, 2/*lastRowToShift*/
+ , -1/*step*/, SpreadsheetVersion.EXCEL2007); //
parameters 2, 2, -1 should mean : move row range [2-2] one level up
+ XSSFEvaluationWorkbook fpb =
XSSFEvaluationWorkbook.create((XSSFWorkbook) wb);
+ Ptg[] ptgs = FormulaParser.parse(initialFormula, fpb,
FormulaType.CELL, 0); // [A1]
+ formulaShifter.adjustFormula(ptgs, 0); // adjusted to [A]
+ String shiftedFmla = FormulaRenderer.toFormulaString(fpb, ptgs);
//A
+ //System.out.println(String.format("initial formula : A1; expected
formula value after shifting up : #REF!; actual formula value : %s",
shiftedFmla));
+ assertEquals("On copy we expect the formula to be adjusted, in
this case it would point to row -1, which is an invalid REF",
+ expectedFormula, shiftedFmla);
+ }
+
+ {
+ FormulaShifter formulaShifter =
FormulaShifter.createForRowShift(0, "sheet1", 2/*firstRowToShift*/,
2/*lastRowToShift*/
+ , -1/*step*/, SpreadsheetVersion.EXCEL2007); //
parameters 2, 2, -1 should mean : move row range [2-2] one level up
+ XSSFEvaluationWorkbook fpb =
XSSFEvaluationWorkbook.create((XSSFWorkbook) wb);
+ Ptg[] ptgs = FormulaParser.parse(initialFormula, fpb,
FormulaType.CELL, 0); // [A1]
+ formulaShifter.adjustFormula(ptgs, 0); // adjusted to [A]
+ String shiftedFmla = FormulaRenderer.toFormulaString(fpb, ptgs);
//A
+ //System.out.println(String.format("initial formula : A1; expected
formula value after shifting up : #REF!; actual formula value : %s",
shiftedFmla));
+ assertEquals("On move we expect the formula to stay the same, thus
expecting the initial formula A1 here",
+ initialFormula, shiftedFmla);
+ }
+
+ sheet.shiftRows(2, 2, -1);
+ {
+ Cell c2 = sheet.getRow(1).getCell(2);
+ assertNotNull("cell C2 needs to exist now", c2);
+ assertEquals(CellType.FORMULA, c2.getCellType());
+ assertEquals(initialFormula, c2.getCellFormula());
+ FormulaEvaluator evaluator =
wb.getCreationHelper().createFormulaEvaluator();
+ CellValue cellValue = evaluator.evaluate(c2);
+ assertEquals(1, cellValue.getNumberValue(), 0.0001);
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
