diff --git a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java index 957b804ff59..8988c8a1804 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java @@ -100,6 +100,9 @@ public class TestAllFiles { // stress docs "document/deep-table-cell.docx", + // invalid files + "spreadsheet/bug69769.xlsx", + // NOTE: Expected failures should usually be added in file "stress.xls" instead // of being listed here in order to also verify the expected exception details! }; diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFSheetXMLHandler.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFSheetXMLHandler.java index dbe3bcdee6a..110d343d7b1 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFSheetXMLHandler.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/XSSFSheetXMLHandler.java @@ -253,7 +253,7 @@ public void startElement(String uri, String localName, String qName, } else if ("row".equals(localName)) { String rowNumStr = attributes.getValue("r"); if (rowNumStr != null) { - rowNum = Integer.parseInt(rowNumStr) - 1; + rowNum = parseInt(rowNumStr) - 1; } else { rowNum = nextRowNum; } @@ -284,7 +284,7 @@ else if ("str".equals(cellType)) XSSFCellStyle style = null; if (stylesTable != null) { if (cellStyleStr != null) { - int styleIndex = Integer.parseInt(cellStyleStr); + int styleIndex = parseInt(cellStyleStr); style = stylesTable.getStyleAt(styleIndex); } else if (stylesTable.getNumCellStyles() > 0) { style = stylesTable.getStyleAt(0); @@ -392,7 +392,7 @@ private void outputCell() { if (this.formatString != null) { try { // Try to use the value as a formattable number - double d = Double.parseDouble(fv); + double d = parseDouble(fv); thisStr = formatter.formatRawCellContents(d, this.formatIndex, this.formatString); } catch (Exception e) { // Formula is a String result not a Numeric one @@ -416,10 +416,10 @@ private void outputCell() { break; case SST_STRING: - String sstIndex = value.toString(); + String sstIndex = value.toString().trim(); if (!sstIndex.isEmpty()) { try { - int idx = Integer.parseInt(sstIndex); + int idx = parseInt(sstIndex); RichTextString rtss = sharedStringsTable.getItemAt(idx); thisStr = rtss.toString(); } catch (NumberFormatException ex) { @@ -433,7 +433,7 @@ private void outputCell() { if (this.formatString != null && !n.isEmpty()) { try { thisStr = formatter.formatRawCellContents( - Double.parseDouble(n), this.formatIndex, this.formatString); + parseDouble(n), this.formatIndex, this.formatString); } catch (Exception e) { LOG.atInfo().log( "Error formatting cell '{}' - will use its raw value instead (format '{}')", @@ -515,7 +515,6 @@ private void checkForEmptyCellComments(EmptyCellCommentsCheckType type) { } } - /** * Output an empty-cell comment. */ @@ -524,6 +523,14 @@ private void outputEmptyCellComment(CellAddress cellRef) { output.cell(cellRef.formatAsString(), null, comment); } + private static int parseInt(String value) throws NumberFormatException { + return Integer.parseInt(value.trim()); + } + + private static double parseDouble(String value) throws NumberFormatException { + return Double.parseDouble(value.trim()); + } + private enum EmptyCellCommentsCheckType { CELL, END_OF_ROW, diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java index 29088a5b482..d23375fda55 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -21,6 +21,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Calendar; import java.util.Date; +import org.apache.logging.log4j.Logger; +import org.apache.poi.logging.PoiLogManager; import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.FormulaParser; import org.apache.poi.ss.formula.FormulaRenderer; @@ -73,6 +75,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ public final class XSSFCell extends CellBase { + private static final Logger LOG = PoiLogManager.getLogger(XSSFCell.class); private static final String FALSE_AS_STRING = "0"; private static final String TRUE_AS_STRING = "1"; private static final String FALSE = "FALSE"; @@ -244,7 +247,7 @@ public double getNumericCellValue() { return 0.0; } try { - return Double.parseDouble(v); + return parseDouble(v); } catch(NumberFormatException e) { throw typeMismatch(CellType.NUMERIC, CellType.STRING, false); } @@ -330,12 +333,13 @@ private XSSFRichTextString findStringValue() { } else { if (_cell.isSetV()) { try { - int idx = Integer.parseInt(_cell.getV()); + int idx = parseInt(_cell.getV()); rt = (XSSFRichTextString)_sharedStringSource.getItemAt(idx); } catch (Throwable t) { if (ExceptionUtil.isFatal(t)) { ExceptionUtil.rethrow(t); } + LOG.atError().withThrowable(t).log("Failed to parse SST index '{}'", _cell.getV()); rt = new XSSFRichTextString(""); } } else { @@ -1122,12 +1126,12 @@ private boolean convertCellValueToBoolean() { case BOOLEAN: return TRUE_AS_STRING.equals(_cell.getV()); case STRING: - int sstIndex = Integer.parseInt(_cell.getV()); + int sstIndex = parseInt(_cell.getV()); RichTextString rt = _sharedStringSource.getItemAt(sstIndex); String text = rt.getString(); return Boolean.parseBoolean(text); case NUMERIC: - return Double.parseDouble(_cell.getV()) != 0; + return parseDouble(_cell.getV()) != 0; case ERROR: // fall-through @@ -1149,13 +1153,14 @@ private String convertCellValueToString() { return TRUE_AS_STRING.equals(_cell.getV()) ? TRUE : FALSE; case STRING: try { - int sstIndex = Integer.parseInt(_cell.getV()); + int sstIndex = parseInt(_cell.getV()); RichTextString rt = _sharedStringSource.getItemAt(sstIndex); return rt.getString(); } catch (Throwable t) { if (ExceptionUtil.isFatal(t)) { ExceptionUtil.rethrow(t); } + LOG.atError().withThrowable(t).log("Failed to parse SST index '{}'", _cell.getV()); return ""; } case NUMERIC: @@ -1227,4 +1232,12 @@ public void updateCellReferencesForShifting(String msg){ ctCell.setR(r); } + private static int parseInt(String value) throws NumberFormatException { + return Integer.parseInt(value.trim()); + } + + private static double parseDouble(String value) throws NumberFormatException { + return Double.parseDouble(value.trim()); + } + } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFSheetXMLHandler.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFSheetXMLHandler.java index b3f12159256..7e5a3071f2c 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFSheetXMLHandler.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFSheetXMLHandler.java @@ -26,7 +26,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more import org.xml.sax.XMLReader; import java.io.InputStream; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -100,4 +102,39 @@ public void cell(final String cellReference, final String formattedValue, } } } + + @Test + void testSstStrayWhitespace() throws Exception { + try (OPCPackage xlsxPackage = OPCPackage.open(_ssTests.openResourceAsStream("bug69769.xlsx"))) { + final XSSFReader reader = new XSSFReader(xlsxPackage); + final Iterator iter = reader.getSheetsData(); + final Map cellValues = new HashMap<>(); + + try (InputStream stream = iter.next()) { + final XMLReader sheetParser = XMLHelper.getSaxParserFactory().newSAXParser().getXMLReader(); + + sheetParser.setContentHandler(new XSSFSheetXMLHandler(reader.getStylesTable(), + new ReadOnlySharedStringsTable(xlsxPackage), new SheetContentsHandler() { + @Override + public void startRow(final int rowNum) { + } + + @Override + public void endRow(final int rowNum) { + } + + @Override + public void cell(final String cellReference, final String formattedValue, + final XSSFComment comment) { + cellValues.put(cellReference, formattedValue); + } + }, false)); + + assertDoesNotThrow(() -> sheetParser.parse(new InputSource(stream))); + + assertEquals(4, cellValues.size()); + assertEquals("Mustermann", cellValues.get("B2")); + } + } + } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index ef8c8542a08..22a9ec10e23 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -3910,6 +3910,34 @@ void testBug66827() throws Exception { } } + @Test + void testBug69769() throws Exception { + final int expectedCount = 3; + try (XSSFWorkbook wb = openSampleWorkbook("bug69769.xlsx")) { + SharedStringsTable sst = wb.getSharedStringSource(); + assertNotNull(sst); + assertEquals(expectedCount, sst.getCount()); + for (int i = 0; i < expectedCount; i++) { + assertNotNull(sst.getItemAt(i)); + } + XSSFSheet ws = wb.getSheetAt(0); + int nRowCount = ws.getLastRowNum(); + DataFormatter df = new DataFormatter(); + for (int r = 0; r <= nRowCount; r++) { + XSSFRow row = ws.getRow(r); + if (row != null) { + for (Cell cell : row) { + String cellValue = df.formatCellValue(cell); + assertNotNull(cellValue, "Cell value should not be null"); + if (cell.getRowIndex() == 1 && cell.getColumnIndex() == 1) { + assertEquals("Mustermann", cellValue); + } + } + } + } + } + } + private static void readByCommonsCompress(File temp_excel_poi) throws IOException { /* read by commons-compress*/ try (ZipFile zipFile = ZipFile.builder().setFile(temp_excel_poi).get()) { diff --git a/test-data/spreadsheet/bug69769.xlsx b/test-data/spreadsheet/bug69769.xlsx new file mode 100644 index 00000000000..39262dfff3a Binary files /dev/null and b/test-data/spreadsheet/bug69769.xlsx differ