bug 58775: set an upper limit on number of data formats, default 250.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1722054 13f79535-47bb-0310-9956-ffa450edef68

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<Integer, String> numberFormats = new TreeMap<Integer,String>();
+    private final SortedMap<Short, String> numberFormats = new TreeMap<Short,String>();
     private final List<XSSFFont> fonts = new ArrayList<XSSFFont>();
     private final List<XSSFCellFill> fills = new ArrayList<XSSFCellFill>();
     private final List<XSSFCellBorder> borders = new ArrayList<XSSFCellBorder>();
@@ -80,14 +80,49 @@ 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;
-    
+    // Is this right? Number formats (XSSFDataFormat) and cell styles (XSSFCellStyle) are different. What's up with the plus 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
     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 <code>num</code> < 0
+     * @throws IllegalStateException if <code>num</code> < 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;
     private ThemesTable theme;
@@ -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<Short,String> numFmt : numberFormats.entrySet()) {
+            if(numFmt.getValue().equals(fmt)) {
+                return numFmt.getKey();
+            }
+        }
+        throw new IllegalStateException("Number format not in style table: " + fmt);
     }
 
     /**
@@ -221,18 +283,19 @@ public class StylesTable extends POIXMLDocumentPart {
      *
      * @param fmt the number format to add to number format style table
      * @return the index of <code>fmt</code> 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
+            try {
-            for (Entry<Integer,String> numFmt : numberFormats.entrySet()) {
+                return getNumberFormatId(fmt);
-                if(numFmt.getValue().equals(fmt)) {
+            } catch (final IllegalStateException e) {
-                    return numFmt.getKey();
-                }
-            }
                 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. " +
@@ -240,18 +303,23 @@ public class StylesTable extends POIXMLDocumentPart {
         }
 
         // 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
+            // Assumption: gaps in number format ids are acceptable
-            formatIndex = Math.max(
+            // to catch arithmetic overflow, nextKey's data type
-                    numberFormats.lastKey() + 1,
+            // must match numberFormat's key data type
-                    FIRST_USER_DEFINED_NUMBER_FORMAT_ID);
+            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<Integer, String> getNumberFormats(){
+    public Map<Short, String> 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<Integer, String> entry : numberFormats.entrySet()) {
+        for (final Entry<Short, String> entry : numberFormats.entrySet()) {
             CTNumFmt ctFmt = formats.addNewNumFmt();
             ctFmt.setNumFmtId(entry.getKey());
             ctFmt.setFormatCode(entry.getValue());

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;

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());

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());
@@ -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 <K,V> void assertNotContainsKey(Map<K,V> map, K key) {
+        assertFalse(map.containsKey(key));
+    }
+    private static final <K,V> void assertNotContainsValue(Map<K,V> 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<Short,String> 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();
+        }
+    }
 }

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);

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());