diff --git a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java index 66605e6a1..b18a100e4 100644 --- a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java +++ b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java @@ -68,7 +68,7 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.StyleSheetDocument; * Table of styles shared across all sheets in a workbook. */ public class StylesTable extends POIXMLDocumentPart { - private final SortedMap numberFormats = new TreeMap(); + private final SortedMap numberFormats = new TreeMap(); private final List fonts = new ArrayList(); private final List fills = new ArrayList(); private final List borders = new ArrayList(); @@ -80,13 +80,48 @@ public class StylesTable extends POIXMLDocumentPart { /** * The first style id available for use as a custom style */ - // Is this right? Number formats (XSSFDataFormat) and cell styles (XSSFCellStyle) are different. - // What's up with the plus 1? public static final int FIRST_CUSTOM_STYLE_ID = BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX + 1; - - private static final int FIRST_USER_DEFINED_NUMBER_FORMAT_ID = BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX; - private static final int MAXIMUM_NUMBER_OF_DATA_FORMATS = SpreadsheetVersion.EXCEL2007.getMaxCellStyles(); //FIXME: should be 250 + // Is this right? Number formats (XSSFDataFormat) and cell styles (XSSFCellStyle) are different. What's up with the plus 1? private static final int MAXIMUM_STYLE_ID = SpreadsheetVersion.EXCEL2007.getMaxCellStyles(); + + private static final short FIRST_USER_DEFINED_NUMBER_FORMAT_ID = BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX; + /** + * Depending on the version of Excel, the maximum number of number formats in a workbook is between 200 and 250 + * See https://support.office.com/en-us/article/excel-specifications-and-limits-1672b34d-7043-467e-8e27-269d656771c3 + * POI defaults this limit to 250, but can be increased or decreased on a per-StylesTable basis with + * {@link #setMaxNumberOfDataFormats(int)} if needed. + */ + private int MAXIMUM_NUMBER_OF_DATA_FORMATS = 250; + + /** + * Changes the maximum number of data formats that may be in a style table + * + * @param num the upper limit on number of data formats in the styles table when adding new data formats + * @throws IllegalArgumentException if num < 0 + * @throws IllegalStateException if num < current number of data formats in the style table. + * Data formats must be explicitly removed before the limit can be decreased. + */ + public void setMaxNumberOfDataFormats(int num) { + if (num < getNumDataFormats()) { + if (num < 0) { + throw new IllegalArgumentException("Maximum Number of Data Formats must be greater than or equal to 0"); + } else { + throw new IllegalStateException("Cannot set the maximum number of data formats less than the current quantity." + + "Data formats must be explicitly removed (via StylesTable.removeNumberFormat) before the limit can be decreased."); + } + } + MAXIMUM_NUMBER_OF_DATA_FORMATS = num; + } + + /** + * Get the upper limit on the number of data formats that has been set for the style table. + * To get the current number of data formats in use, use {@link #getNumDataFormats()}. + * + * @return the maximum number of data formats allowed in the workbook + */ + public int getMaxNumberOfDataFormats() { + return MAXIMUM_NUMBER_OF_DATA_FORMATS; + } private StyleSheetDocument doc; private XSSFWorkbook workbook; @@ -163,7 +198,7 @@ public class StylesTable extends POIXMLDocumentPart { CTNumFmts ctfmts = styleSheet.getNumFmts(); if( ctfmts != null){ for (CTNumFmt nfmt : ctfmts.getNumFmtArray()) { - int formatId = (int)nfmt.getNumFmtId(); + short formatId = (short)nfmt.getNumFmtId(); numberFormats.put(formatId, nfmt.getFormatCode()); } } @@ -210,8 +245,35 @@ public class StylesTable extends POIXMLDocumentPart { // Start of style related getters and setters // =========================================================== + /** + * Get number format string given its id + * + * @param idx number format id + * @return number format code + * @deprecated POI 3.14-beta2. Use {@link #getNumberFormatAt(short)} instead. + */ public String getNumberFormatAt(int idx) { - return numberFormats.get(idx); + return getNumberFormatAt((short) idx); + } + + /** + * Get number format string given its id + * + * @param fmtId number format id + * @return number format code + */ + public String getNumberFormatAt(short fmtId) { + return numberFormats.get(fmtId); + } + + private short getNumberFormatId(String fmt) { + // Find the key, and return that + for (Entry numFmt : numberFormats.entrySet()) { + if(numFmt.getValue().equals(fmt)) { + return numFmt.getKey(); + } + } + throw new IllegalStateException("Number format not in style table: " + fmt); } /** @@ -221,38 +283,44 @@ public class StylesTable extends POIXMLDocumentPart { * * @param fmt the number format to add to number format style table * @return the index of fmt in the number format style table + * @throws IllegalStateException if adding the number format to the styles table + * would exceed the {@link #MAXIMUM_NUMBER_OF_DATA_FORMATS} allowed. */ public int putNumberFormat(String fmt) { // Check if number format already exists if (numberFormats.containsValue(fmt)) { - // Find the key, and return that - for (Entry numFmt : numberFormats.entrySet()) { - if(numFmt.getValue().equals(fmt)) { - return numFmt.getKey(); - } + try { + return getNumberFormatId(fmt); + } catch (final IllegalStateException e) { + throw new IllegalStateException("Found the format, but couldn't figure out where - should never happen!"); } - throw new IllegalStateException("Found the format, but couldn't figure out where - should never happen!"); } + if (numberFormats.size() >= MAXIMUM_NUMBER_OF_DATA_FORMATS) { throw new IllegalStateException("The maximum number of Data Formats was exceeded. " + "You can define up to " + MAXIMUM_NUMBER_OF_DATA_FORMATS + " formats in a .xlsx Workbook."); } // Find a spare key, and add that - final int formatIndex; + final short formatIndex; if (numberFormats.isEmpty()) { formatIndex = FIRST_USER_DEFINED_NUMBER_FORMAT_ID; } else { // get next-available numberFormat index. - // Assumption: there are never gaps in numberFormats indices - formatIndex = Math.max( - numberFormats.lastKey() + 1, - FIRST_USER_DEFINED_NUMBER_FORMAT_ID); + // Assumption: gaps in number format ids are acceptable + // to catch arithmetic overflow, nextKey's data type + // must match numberFormat's key data type + short nextKey = (short) (numberFormats.lastKey() + 1); + if (nextKey < 0) { + throw new IllegalStateException( + "Cowardly avoiding creating a number format with a negative id." + + "This is probably due to arithmetic overflow."); + } + formatIndex = (short) Math.max(nextKey, FIRST_USER_DEFINED_NUMBER_FORMAT_ID); } - numberFormats.put(formatIndex, fmt); return formatIndex; } @@ -268,7 +336,40 @@ public class StylesTable extends POIXMLDocumentPart { * @param fmt the number format code */ public void putNumberFormat(short index, String fmt) { - numberFormats.put((int)index, fmt); + numberFormats.put(index, fmt); + } + + /** + * Remove a number format from the style table if it exists. + * All cell styles with this number format will be modified to use the default number format. + * + * @param fmt the number format to remove + * @return true if the number format was removed + */ + public boolean removeNumberFormat(short index) { + String fmt = numberFormats.remove(index); + boolean removed = (fmt != null); + if (removed) { + for (final CTXf style : xfs) { + if (style.isSetNumFmtId() && style.getNumFmtId() == index) { + style.unsetApplyNumberFormat(); + style.unsetNumFmtId(); + } + } + } + return removed; + } + + /** + * Remove a number format from the style table if it exists + * All cell styles with this number format will be modified to use the default number format + * + * @param fmt the number format to remove + * @return true if the number format was removed + */ + public boolean removeNumberFormat(String fmt) { + short id = getNumberFormatId(fmt); + return removeNumberFormat(id); } public XSSFFont getFontAt(int idx) { @@ -359,7 +460,7 @@ public class StylesTable extends POIXMLDocumentPart { return Collections.unmodifiableList(fonts); } - public Map getNumberFormats(){ + public Map getNumberFormats(){ return Collections.unmodifiableMap(numberFormats); } @@ -440,12 +541,20 @@ public class StylesTable extends POIXMLDocumentPart { return xfs.size(); } + /** + * @return number of data formats in the styles table + */ + public int getNumDataFormats() { + return numberFormats.size(); + } + /** * For unit testing only + * @deprecated POI 3.14 beta 2. Use {@link #getNumDataFormats()} instead. */ @Internal public int _getNumberFormatSize() { - return numberFormats.size(); + return getNumDataFormats(); } /** @@ -492,7 +601,7 @@ public class StylesTable extends POIXMLDocumentPart { // Formats CTNumFmts formats = CTNumFmts.Factory.newInstance(); formats.setCount(numberFormats.size()); - for (final Entry entry : numberFormats.entrySet()) { + for (final Entry entry : numberFormats.entrySet()) { CTNumFmt ctFmt = formats.addNewNumFmt(); ctFmt.setNumFmtId(entry.getKey()); ctFmt.setFormatCode(entry.getValue()); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java index 21534a86f..a2c265f1b 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java @@ -58,9 +58,9 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPatternType; public class XSSFCellStyle implements CellStyle { private int _cellXfId; - private StylesTable _stylesSource; + private final StylesTable _stylesSource; private CTXf _cellXf; - private CTXf _cellStyleXf; + private final CTXf _cellStyleXf; private XSSFFont _font; private XSSFCellAlignment _cellAlignment; private ThemesTable _theme; diff --git a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java index b964ced1d..0eebf89ff 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java @@ -58,7 +58,7 @@ public final class TestXSSFReader extends TestCase { XSSFReader r = new XSSFReader(pkg); assertEquals(3, r.getStylesTable().getFonts().size()); - assertEquals(0, r.getStylesTable()._getNumberFormatSize()); + assertEquals(0, r.getStylesTable().getNumDataFormats()); // The Styles Table should have the themes associated with it too assertNotNull(r.getStylesTable().getTheme()); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java b/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java index 70af338d6..03e232ad2 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java @@ -17,17 +17,33 @@ package org.apache.poi.xssf.model; +import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.io.IOException; +import java.util.Map; + +import org.apache.poi.ss.usermodel.BuiltinFormats; +import org.apache.poi.ss.usermodel.Cell; +import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFCellStyle; import org.apache.poi.xssf.usermodel.XSSFWorkbook; public final class TestStylesTable { private static final String testFile = "Formatting.xlsx"; + private static final String customDataFormat = "YYYY-mm-dd"; + + @BeforeClass + public static void assumeCustomDataFormatIsNotBuiltIn() { + assertEquals(-1, BuiltinFormats.getBuiltinFormat(customDataFormat)); + } @Test public void testCreateNew() { @@ -37,7 +53,7 @@ public final class TestStylesTable { assertNotNull(st.getCTStylesheet()); assertEquals(1, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(0, st._getNumberFormatSize()); + assertEquals(0, st.getNumDataFormats()); } @Test @@ -48,14 +64,14 @@ public final class TestStylesTable { assertNotNull(st.getCTStylesheet()); assertEquals(1, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(0, st._getNumberFormatSize()); + assertEquals(0, st.getNumDataFormats()); st = XSSFTestDataSamples.writeOutAndReadBack(wb).getStylesSource(); assertNotNull(st.getCTStylesheet()); assertEquals(1, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(0, st._getNumberFormatSize()); + assertEquals(0, st.getNumDataFormats()); assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } @@ -89,7 +105,7 @@ public final class TestStylesTable { assertNotNull(st.getCTStylesheet()); assertEquals(11, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(8, st._getNumberFormatSize()); + assertEquals(8, st.getNumDataFormats()); assertEquals(2, st.getFonts().size()); assertEquals(2, st.getFills().size()); @@ -101,7 +117,7 @@ public final class TestStylesTable { assertNotNull(st.getStyleAt(0)); assertNotNull(st.getStyleAt(1)); assertNotNull(st.getStyleAt(2)); - + assertEquals(0, st.getStyleAt(0).getDataFormat()); assertEquals(14, st.getStyleAt(1).getDataFormat()); assertEquals(0, st.getStyleAt(2).getDataFormat()); @@ -118,7 +134,7 @@ public final class TestStylesTable { assertNotNull(st.getCTStylesheet()); assertEquals(1, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(0, st._getNumberFormatSize()); + assertEquals(0, st.getNumDataFormats()); int nf1 = st.putNumberFormat("yyyy-mm-dd"); int nf2 = st.putNumberFormat("yyyy-mm-DD"); @@ -132,7 +148,7 @@ public final class TestStylesTable { assertNotNull(st.getCTStylesheet()); assertEquals(2, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(2, st._getNumberFormatSize()); + assertEquals(2, st.getNumDataFormats()); assertEquals("yyyy-mm-dd", st.getNumberFormatAt(nf1)); assertEquals(nf1, st.putNumberFormat("yyyy-mm-dd")); @@ -149,7 +165,7 @@ public final class TestStylesTable { StylesTable st = workbook.getStylesSource(); assertEquals(11, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(8, st._getNumberFormatSize()); + assertEquals(8, st.getNumDataFormats()); int nf1 = st.putNumberFormat("YYYY-mm-dd"); int nf2 = st.putNumberFormat("YYYY-mm-DD"); @@ -159,7 +175,7 @@ public final class TestStylesTable { assertEquals(11, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); - assertEquals(10, st._getNumberFormatSize()); + assertEquals(10, st.getNumDataFormats()); assertEquals("YYYY-mm-dd", st.getNumberFormatAt(nf1)); assertEquals(nf1, st.putNumberFormat("YYYY-mm-dd")); @@ -167,4 +183,175 @@ public final class TestStylesTable { assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(workbook)); } + + @Test + public void exceedNumberFormatLimit() throws IOException { + XSSFWorkbook wb = new XSSFWorkbook(); + try { + StylesTable styles = wb.getStylesSource(); + for (int i = 0; i < styles.getMaxNumberOfDataFormats(); i++) { + wb.getStylesSource().putNumberFormat("\"test" + i + " \"0"); + } + try { + wb.getStylesSource().putNumberFormat("\"anotherformat \"0"); + } catch (final IllegalStateException e) { + if (e.getMessage().startsWith("The maximum number of Data Formats was exceeded.")) { + //expected + } + else { + throw e; + } + } + } finally { + wb.close(); + } + } + + private static final void assertNotContainsKey(Map map, K key) { + assertFalse(map.containsKey(key)); + } + private static final void assertNotContainsValue(Map map, V value) { + assertFalse(map.containsValue(value)); + } + + @Test + public void removeNumberFormat() throws IOException { + XSSFWorkbook wb = new XSSFWorkbook(); + try { + final String fmt = customDataFormat; + final short fmtIdx = (short) wb.getStylesSource().putNumberFormat(fmt); + + Cell cell = wb.createSheet("test").createRow(0).createCell(0); + cell.setCellValue(5.25); + CellStyle style = wb.createCellStyle(); + style.setDataFormat(fmtIdx); + cell.setCellStyle(style); + + assertEquals(fmt, cell.getCellStyle().getDataFormatString()); + assertEquals(fmt, wb.getStylesSource().getNumberFormatAt(fmtIdx)); + + // remove the number format from the workbook + wb.getStylesSource().removeNumberFormat(fmt); + + // number format in CellStyles should be restored to default number format + final short defaultFmtIdx = 0; + final String defaultFmt = BuiltinFormats.getBuiltinFormat(0); + assertEquals(defaultFmtIdx, style.getDataFormat()); + assertEquals(defaultFmt, style.getDataFormatString()); + + // The custom number format should be entirely removed from the workbook + Map numberFormats = wb.getStylesSource().getNumberFormats(); + assertNotContainsKey(numberFormats, fmtIdx); + assertNotContainsValue(numberFormats, fmt); + + // The default style shouldn't be added back to the styles source because it's built-in + assertEquals(0, wb.getStylesSource().getNumDataFormats()); + + cell = null; style = null; numberFormats = null; + wb = XSSFTestDataSamples.writeOutCloseAndReadBack(wb); + + cell = wb.getSheet("test").getRow(0).getCell(0); + style = cell.getCellStyle(); + + // number format in CellStyles should be restored to default number format + assertEquals(defaultFmtIdx, style.getDataFormat()); + assertEquals(defaultFmt, style.getDataFormatString()); + + // The custom number format should be entirely removed from the workbook + numberFormats = wb.getStylesSource().getNumberFormats(); + assertNotContainsKey(numberFormats, fmtIdx); + assertNotContainsValue(numberFormats, fmt); + + // The default style shouldn't be added back to the styles source because it's built-in + assertEquals(0, wb.getStylesSource().getNumDataFormats()); + + } finally { + wb.close(); + } + } + + @Test + public void maxNumberOfDataFormats() throws IOException { + XSSFWorkbook wb = new XSSFWorkbook(); + + try { + StylesTable styles = wb.getStylesSource(); + + // Check default limit + int n = styles.getMaxNumberOfDataFormats(); + // https://support.office.com/en-us/article/excel-specifications-and-limits-1672b34d-7043-467e-8e27-269d656771c3 + assertTrue(200 <= n); + assertTrue(n <= 250); + + // Check upper limit + n = Integer.MAX_VALUE; + styles.setMaxNumberOfDataFormats(n); + assertEquals(n, styles.getMaxNumberOfDataFormats()); + + // Check negative (illegal) limits + try { + styles.setMaxNumberOfDataFormats(-1); + fail("Expected to get an IllegalArgumentException(\"Maximum Number of Data Formats must be greater than or equal to 0\")"); + } catch (final IllegalArgumentException e) { + if (e.getMessage().startsWith("Maximum Number of Data Formats must be greater than or equal to 0")) { + // expected + } else { + throw e; + } + } + } + finally { + wb.close(); + } + } + + @Test + public void addDataFormatsBeyondUpperLimit() throws IOException { + XSSFWorkbook wb = new XSSFWorkbook(); + + try { + StylesTable styles = wb.getStylesSource(); + styles.setMaxNumberOfDataFormats(0); + + // Try adding a format beyond the upper limit + try { + styles.putNumberFormat("\"test \"0"); + fail("Expected to raise IllegalStateException"); + } catch (final IllegalStateException e) { + if (e.getMessage().startsWith("The maximum number of Data Formats was exceeded.")) { + // expected + } else { + throw e; + } + } + } + finally { + wb.close(); + } + } + + @Test + public void decreaseUpperLimitBelowCurrentNumDataFormats() throws IOException { + XSSFWorkbook wb = new XSSFWorkbook(); + + try { + StylesTable styles = wb.getStylesSource(); + styles.putNumberFormat(customDataFormat); + + // Try decreasing the upper limit below the current number of formats + try { + styles.setMaxNumberOfDataFormats(0); + fail("Expected to raise IllegalStateException"); + } catch (final IllegalStateException e) { + if (e.getMessage().startsWith("Cannot set the maximum number of data formats less than the current quantity.")) { + // expected + } else { + throw e; + } + } + } + finally { + wb.close(); + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java index fd19c4b1e..b876518ef 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java @@ -67,6 +67,9 @@ public final class TestXSSFDataFormat extends BaseTestDataFormat { doTest58532Core(wb); } + /** + * [Bug 58778] Built-in number formats can be overridden with XSSFDataFormat.putFormat(int id, String fmt) + */ public void test58778() throws IOException { XSSFWorkbook wb = new XSSFWorkbook(); Cell cell = wb.createSheet("bug58778").createRow(0).createCell(0); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java index cdd20995a..14cc6f5f5 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -268,7 +268,7 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { StylesTable st = ss; // Has 8 number formats - assertEquals(8, st._getNumberFormatSize()); + assertEquals(8, st.getNumDataFormats()); // Has 2 fonts assertEquals(2, st.getFonts().size()); // Has 2 fills @@ -283,7 +283,7 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { st.putNumberFormat("testFORMAT")); assertEquals(StylesTable.FIRST_CUSTOM_STYLE_ID + 9, st.putNumberFormat("testFORMAT2")); - assertEquals(10, st._getNumberFormatSize()); + assertEquals(10, st.getNumDataFormats()); // Save, load back in again, and check @@ -293,7 +293,7 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { ss = wb2.getStylesSource(); assertNotNull(ss); - assertEquals(10, st._getNumberFormatSize()); + assertEquals(10, st.getNumDataFormats()); assertEquals(2, st.getFonts().size()); assertEquals(2, st.getFills().size()); assertEquals(1, st.getBorders().size());