| Summary: | Some paths can create an XSSFColor instance with a null CTColor reference | ||
|---|---|---|---|
| Product: | POI | Reporter: | Greg Woolsey <gwoolsey> |
| Component: | XSSF | Assignee: | POI Developers List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 3.17-FINAL | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | sample workbook with cell styles without colors | ||
My internal bug report appears to be against an older version of POI - I'm unable to reproduce this with a unit test making the same call as the error my user reported. However, I see we do have a lot of code where an inline null check or similar is done on the CTColor argument to decide whether to return null or create an XSSFColor instance. That pattern is definitely safer as a factory, so I'll still investigate that. |
Created attachment 35596 [details] sample workbook with cell styles without colors The attached workbook, saved from Excel 2016, contains a cell style with a fill with a pattern but no background or foreground color. Currently, POI will happily return an XSSFColor instance for this style's foreground and background color, with null for the CTColor field. This will then lead to later NPEs in unexpected places, of course. Since this is a normal and valid way to construct fills, and probably other format features, I don't think just checking for a null constructor parameter and throwing an exception is sufficient. We need to replace constructors that take a CTColor object directly with a factory method that can check the input and return null if needed instead of an invalid XSSFColor object.