You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2017/09/28 09:56:46 UTC
svn commit: r1809967 - in /poi:
site/src/documentation/content/xdocs/status.xml
trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java
trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
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: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org