Bug 62108

Summary: ArrayIndexOfBounds exception when getColumnWidth()
Product: POI Reporter: Dmytro Sylaiev <dmytro.sylaiev>
Component: XSSFAssignee: POI Developers List <dev>
Status: NEEDINFO ---    
Severity: major    
Priority: P2    
Version: 3.17-FINAL   
Target Milestone: ---   
Hardware: PC   
OS: All   

Description Dmytro Sylaiev 2018-02-16 09:03:22 UTC
XSSFCellStyle::getFontIndex return short and actually cast return of getFontId() (actually int) to short.
Moreover, getFontId cast long CTXfImpl::getFontId to int.
So in some cases we have double-overflow (long -> int -> short) which cause ArrayIndexOfBoundsException while trying to autoSizeColumns

Stacktrace:


java.lang.ArrayIndexOutOfBoundsException: -32765
	at java.util.ArrayList.elementData(Unknown Source)
	at java.util.ArrayList.get(Unknown Source)
	at org.apache.poi.xssf.model.StylesTable.getFontAt(StylesTable.java:204)
	at org.apache.poi.xssf.usermodel.XSSFWorkbook.getFontAt(XSSFWorkbook.java:809)
	at org.apache.poi.xssf.usermodel.XSSFWorkbook.getFontAt(XSSFWorkbook.java:106)
	at org.apache.poi.ss.util.SheetUtil.getCellWidth(SheetUtil.java:124)
	at org.apache.poi.ss.util.SheetUtil.getColumnWidth(SheetUtil.java:229)
	at org.apache.poi.xssf.usermodel.XSSFSheet.autoSizeColumn(XSSFSheet.java:398)
Comment 1 Nick Burch 2018-02-16 11:50:49 UTC
I've had a go at creating a unit test for this problem in r1824451, but it isn't triggering the bug, despite creating 50,000 fonts

Could you take a look at that test, and suggest what we need to do to re-produce the bug you're seeing?
Comment 2 PJ Fanning 2018-02-16 12:26:04 UTC
I independently put together a change to use ints instead of shorts for indexing fonts. https://github.com/apache/poi/pull/102
Still would be good to have a reproducible test case though.
Comment 3 Mark Murphy 2018-02-16 13:25:01 UTC
Since the font index value is an xsd:unsignedInt, maybe a long is the appropriate data type. I know Java 8 kinda halfheartedly supports unsigned ints by adding some methods that treat an int as unsigned to the Integer class. maybe that will be sufficient here. It just seems to be a kludge to me. If we go with the Java 8 view of unsigned int we need to make sure to document that the font index is an unsigned int rather than the typical signed int.
Comment 4 PJ Fanning 2018-02-16 14:28:25 UTC
Java 8's unsigned int support is not at all nice.
We have limitations about supporting long - eg a lot of the XSSF clases used Lists and this has the method `E get(int index);`
I would argue that the pragmatic solution is to just support ints and not worry too much about the the fact that max-unsigned-int is aprox twice max-signed-int. It's taken this long for someone to run into the `short` limitation.
Comment 5 PJ Fanning 2018-03-19 09:29:38 UTC
Dmytro Sylaiev - could you test with the latest code? Some changes were added to support ints instead of shorts?
Comment 6 Dmytro Sylaiev 2018-03-19 13:01:08 UTC
Could you please share latest snapshot?
Comment 7 Dominik Stadler 2018-04-02 21:08:26 UTC
Look at https://builds.apache.org/view/P/view/POI/job/POI-DSL-1.8/lastSuccessfulBuild/artifact/ for latest build results.
Comment 8 Andreas Beeker 2018-08-31 00:31:07 UTC
added a fix for the junit test via r1839709
and probably fixed the issue via r1839710