From 11e60eb77e4a12cacd0d42a4326ddb8ad76d1670 Mon Sep 17 00:00:00 2001
From: Greg Woolsey <gwoolsey@apache.org>
Date: Mon, 11 Dec 2017 17:30:04 +0000
Subject: [PATCH] Bug 61882 - Some paths can create an XSSFColor instance with
 a null CTColor reference

Protect against this in the future by introducing a factory method to create XSSFColor instances from a CTColor instance and the associated workbook style indexed color map.

If the CTColor instance is null, the factory returns null.  All callers already are prepared for a null instance, but many had their own null check on the CTColor object.  This centralizes that.

This also further forces the requirement for the indexed color map.  Any time a color is created, the workbook or styleTable is available in the same context, so passing this is extra parameter is trivial and allows XSSFColor to properly reference custom/themed indexed colors.

Did not remove any methods yet, only deprecated them.  Changed the signature to one internal test-only constructor.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1817796 13f79535-47bb-0310-9956-ffa450edef68
---
 .../xssf/usermodel/examples/CalendarDemo.java | 40 +++++++++----------
 .../examples/WorkingWithRichText.java         |  6 +--
 .../xssf/usermodel/XSSFBorderFormatting.java  |  2 +-
 .../poi/xssf/usermodel/XSSFCellStyle.java     | 13 +++---
 .../apache/poi/xssf/usermodel/XSSFColor.java  | 35 +++++++++++++---
 .../usermodel/XSSFColorScaleFormatting.java   |  4 +-
 .../xssf/usermodel/XSSFCreationHelper.java    |  2 +-
 .../xssf/usermodel/XSSFDataBarFormatting.java |  2 +-
 .../apache/poi/xssf/usermodel/XSSFFont.java   |  2 +-
 .../xssf/usermodel/XSSFFontFormatting.java    |  2 +-
 .../xssf/usermodel/XSSFPatternFormatting.java |  4 +-
 .../apache/poi/xssf/usermodel/XSSFSheet.java  |  2 +-
 .../usermodel/extensions/XSSFCellBorder.java  |  2 +-
 .../usermodel/extensions/XSSFCellFill.java    | 16 ++++++--
 .../poi/xssf/usermodel/TestXSSFBugs.java      |  4 +-
 .../poi/xssf/usermodel/TestXSSFCellStyle.java | 10 ++---
 .../TestXSSFConditionalFormatting.java        |  4 +-
 .../poi/xssf/usermodel/TestXSSFDrawing.java   | 10 ++---
 .../poi/xssf/usermodel/TestXSSFFont.java      |  2 +-
 .../poi/xssf/usermodel/TestXSSFSheet.java     |  3 +-
 .../xssf/usermodel/TestXSSFSimpleShape.java   |  2 +-
 .../xssf/usermodel/TestXSSFTextParagraph.java |  2 +-
 .../extensions/TestXSSFCellFill.java          | 12 ++++++
 23 files changed, 114 insertions(+), 67 deletions(-)

diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/CalendarDemo.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/CalendarDemo.java
index ac0dfc083..11ffafc63 100644
--- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/CalendarDemo.java
+++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/CalendarDemo.java
@@ -140,7 +140,7 @@ public class CalendarDemo {
         XSSFCellStyle style;
         XSSFFont titleFont = wb.createFont();
         titleFont.setFontHeightInPoints((short)48);
-        titleFont.setColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        titleFont.setColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.CENTER);
         style.setVerticalAlignment(VerticalAlignment.CENTER);
@@ -149,12 +149,12 @@ public class CalendarDemo {
 
         XSSFFont monthFont = wb.createFont();
         monthFont.setFontHeightInPoints((short)12);
-        monthFont.setColor(new XSSFColor(new java.awt.Color(255, 255, 255)));
+        monthFont.setColor(new XSSFColor(new java.awt.Color(255, 255, 255), wb.getStylesSource().getIndexedColors()));
         monthFont.setBold(true);
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.CENTER);
         style.setVerticalAlignment(VerticalAlignment.CENTER);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setFont(monthFont);
         styles.put("month", style);
@@ -165,64 +165,64 @@ public class CalendarDemo {
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.LEFT);
         style.setVerticalAlignment(VerticalAlignment.TOP);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(228, 232, 243)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(228, 232, 243), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setBorderLeft(BorderStyle.THIN);
-        style.setLeftBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setLeftBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setFont(dayFont);
         styles.put("weekend_left", style);
 
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.CENTER);
         style.setVerticalAlignment(VerticalAlignment.TOP);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(228, 232, 243)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(228, 232, 243), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setBorderRight(BorderStyle.THIN);
-        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         styles.put("weekend_right", style);
 
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.LEFT);
         style.setVerticalAlignment(VerticalAlignment.TOP);
         style.setBorderLeft(BorderStyle.THIN);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(255, 255, 255)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(255, 255, 255), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
-        style.setLeftBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setLeftBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setFont(dayFont);
         styles.put("workday_left", style);
 
         style = wb.createCellStyle();
         style.setAlignment(HorizontalAlignment.CENTER);
         style.setVerticalAlignment(VerticalAlignment.TOP);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(255, 255, 255)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(255, 255, 255), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setBorderRight(BorderStyle.THIN);
-        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         styles.put("workday_right", style);
 
         style = wb.createCellStyle();
         style.setBorderLeft(BorderStyle.THIN);
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(234, 234, 234)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(234, 234, 234), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         styles.put("grey_left", style);
 
         style = wb.createCellStyle();
-        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(234, 234, 234)));
+        style.setFillForegroundColor(new XSSFColor(new java.awt.Color(234, 234, 234), wb.getStylesSource().getIndexedColors()));
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
         style.setBorderRight(BorderStyle.THIN);
-        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setRightBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         style.setBorderBottom(BorderStyle.THIN);
-        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89)));
+        style.setBottomBorderColor(new XSSFColor(new java.awt.Color(39, 51, 89), wb.getStylesSource().getIndexedColors()));
         styles.put("grey_right", style);
 
         return styles;
diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithRichText.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithRichText.java
index 4965c7922..9b0221457 100644
--- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithRichText.java
+++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithRichText.java
@@ -37,17 +37,17 @@ public class WorkingWithRichText {
 
             XSSFFont font1 = wb.createFont();
             font1.setBold(true);
-            font1.setColor(new XSSFColor(new java.awt.Color(255, 0, 0)));
+            font1.setColor(new XSSFColor(new java.awt.Color(255, 0, 0), wb.getStylesSource().getIndexedColors()));
             rt.applyFont(0, 10, font1);
 
             XSSFFont font2 = wb.createFont();
             font2.setItalic(true);
             font2.setUnderline(XSSFFont.U_DOUBLE);
-            font2.setColor(new XSSFColor(new java.awt.Color(0, 255, 0)));
+            font2.setColor(new XSSFColor(new java.awt.Color(0, 255, 0), wb.getStylesSource().getIndexedColors()));
             rt.applyFont(10, 19, font2);
 
             XSSFFont font3 = wb.createFont();
-            font3.setColor(new XSSFColor(new java.awt.Color(0, 0, 255)));
+            font3.setColor(new XSSFColor(new java.awt.Color(0, 0, 255), wb.getStylesSource().getIndexedColors()));
             rt.append(" Jumped over the lazy dog", font3);
 
             cell.setCellValue(rt);
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFBorderFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFBorderFormatting.java
index 88a910b89..dc40bdcc3 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFBorderFormatting.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFBorderFormatting.java
@@ -364,6 +364,6 @@ public class XSSFBorderFormatting implements BorderFormatting  {
     }
     
     private XSSFColor getColor(CTBorderPr pr) {
-        return pr == null ? null : new XSSFColor(pr.getColor(), _colorMap);
+        return pr == null ? null : XSSFColor.from(pr.getColor(), _colorMap);
     }
 }
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 423baf8a5..bbbe08395 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java
@@ -39,6 +39,7 @@ import org.apache.xmlbeans.XmlException;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorder;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorderPr;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCellAlignment;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFill;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFont;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTPatternFill;
@@ -755,7 +756,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setBottomBorderColor(short color) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(color);
         setBottomBorderColor(clr);
     }
@@ -865,7 +866,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setFillBackgroundColor(short bg) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(bg);
         setFillBackgroundColor(clr);
     }
@@ -900,7 +901,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setFillForegroundColor(short fg) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(fg);
         setFillForegroundColor(clr);
     }
@@ -1027,7 +1028,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setLeftBorderColor(short color) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(color);
         setLeftBorderColor(clr);
     }
@@ -1082,7 +1083,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setRightBorderColor(short color) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(color);
         setRightBorderColor(clr);
     }
@@ -1139,7 +1140,7 @@ public class XSSFCellStyle implements CellStyle {
      */
     @Override
     public void setTopBorderColor(short color) {
-        XSSFColor clr = new XSSFColor();
+        XSSFColor clr = XSSFColor.from(CTColor.Factory.newInstance(), _stylesSource.getIndexedColors());
         clr.setIndexed(color);
         setTopBorderColor(clr);
     }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java
index e7dbbb99c..ae206f3c3 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java
@@ -32,6 +32,15 @@ public class XSSFColor extends ExtendedColor {
     private final CTColor ctColor;
     private final IndexedColorMap indexedColorMap;
 
+    /**
+     * @param color
+     * @param map
+     * @return null if color is null, new instance otherwise
+     */
+    public static XSSFColor from(CTColor color, IndexedColorMap map) {
+        return color == null ? null : new XSSFColor(color, map);
+    }
+    
     /**
      * Create an instance of XSSFColor from the supplied XML bean, with default color indexes
      * @param color The {@link CTColor} to use as color-value.
@@ -47,7 +56,9 @@ public class XSSFColor extends ExtendedColor {
      * Create an instance of XSSFColor from the supplied XML bean, with the given color indexes
      * @param color The {@link CTColor} to use as color-value.
      * @param map The IndexedColorMap to use instead of the default one
+     * @deprecated 4.0.0 - use the factory {@link #from(CTColor, IndexedColorMap)} method instead to check for null CTColor instances.  Make private eventually
      */
+    @Deprecated
     public XSSFColor(CTColor color, IndexedColorMap map) {
         this.ctColor = color;
         this.indexedColorMap = map;
@@ -56,17 +67,31 @@ public class XSSFColor extends ExtendedColor {
     /**
      * Create an new instance of XSSFColor, without knowledge of any custom indexed colors.
      * This is OK for just transiently setting indexes, etc. but is discouraged in read/get uses
+     * @deprecated as of 4.0.0, we want to have the indexed map, and all calling contexts have access to it.
+     * @see #XSSFColor(IndexedColorMap)
+     * @see #from(CTColor, IndexedColorMap)
      */
+    @Deprecated
+    @Removal(version="4.1")
     public XSSFColor() {
-        this(CTColor.Factory.newInstance(), null);
+        this(CTColor.Factory.newInstance(), new DefaultIndexedColorMap());
     }
 
     /**
-     * TEST ONLY - does not know about custom indexed colors
-     * @param clr awt Color
+     * new color with the given indexed color map
+     * @param colorMap
      */
-    public XSSFColor(java.awt.Color clr) {
-        this();
+    public XSSFColor(IndexedColorMap colorMap) {
+        this(CTColor.Factory.newInstance(), colorMap);
+    }
+    
+    /**
+     * TEST ONLY
+     * @param clr awt Color
+     * @param map 
+     */
+    public XSSFColor(java.awt.Color clr, IndexedColorMap map) {
+        this(map);
         setColor(clr);
     }
 
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColorScaleFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColorScaleFormatting.java
index 8a6b29d21..31f5f2837 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColorScaleFormatting.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColorScaleFormatting.java
@@ -56,7 +56,7 @@ public class XSSFColorScaleFormatting implements ColorScaleFormatting {
         CTColor[] ctcols = _scale.getColorArray();
         XSSFColor[] c = new XSSFColor[ctcols.length];
         for (int i=0; i<ctcols.length; i++) {
-            c[i] = new XSSFColor(ctcols[i], _indexedColorMap);
+            c[i] = XSSFColor.from(ctcols[i], _indexedColorMap);
         }
         return c;
     }
@@ -89,7 +89,7 @@ public class XSSFColorScaleFormatting implements ColorScaleFormatting {
      * @return color from scale
      */
     public XSSFColor createColor() {
-        return new XSSFColor(_scale.addNewColor(), _indexedColorMap);
+        return XSSFColor.from(_scale.addNewColor(), _indexedColorMap);
     }
     public XSSFConditionalFormattingThreshold createThreshold() {
         return new XSSFConditionalFormattingThreshold(_scale.addNewCfvo());
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCreationHelper.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCreationHelper.java
index 1c89d39a1..fedbc8d3d 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCreationHelper.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCreationHelper.java
@@ -54,7 +54,7 @@ public class XSSFCreationHelper implements CreationHelper {
 
     @Override
     public XSSFColor createExtendedColor() {
-        return new XSSFColor(CTColor.Factory.newInstance(), workbook.getStylesSource().getIndexedColors());
+        return XSSFColor.from(CTColor.Factory.newInstance(), workbook.getStylesSource().getIndexedColors());
     }
     
     /**
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDataBarFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDataBarFormatting.java
index 3e9e9ac1e..fecd4f022 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDataBarFormatting.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDataBarFormatting.java
@@ -66,7 +66,7 @@ public class XSSFDataBarFormatting implements DataBarFormatting {
     }
 
     public XSSFColor getColor() {
-        return new XSSFColor(_databar.getColor(), _colorMap);
+        return XSSFColor.from(_databar.getColor(), _colorMap);
     }
     public void setColor(Color color) {
         _databar.setColor( ((XSSFColor)color).getCTColor() );
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java
index 1c16ebc4b..483b7e2e9 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java
@@ -157,7 +157,7 @@ public class XSSFFont implements Font {
     public XSSFColor getXSSFColor() {
         CTColor ctColor = _ctFont.sizeOfColorArray() == 0 ? null : _ctFont.getColorArray(0);
         if(ctColor != null) {
-           XSSFColor color = new XSSFColor(ctColor, _indexedColorMap);
+           XSSFColor color = XSSFColor.from(ctColor, _indexedColorMap);
            if(_themes != null) {
               _themes.inheritFromThemeAsRequired(color);
            }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java
index 1a18775a4..89eea6ad4 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java
@@ -113,7 +113,7 @@ public class XSSFFontFormatting implements FontFormatting {
     public XSSFColor getFontColor() {
         if(_font.sizeOfColorArray() == 0) return null;
 
-        return new XSSFColor(_font.getColorArray(0), _colorMap);
+        return XSSFColor.from(_font.getColorArray(0), _colorMap);
     }
 
     @Override
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPatternFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPatternFormatting.java
index 3cf711c33..0f191c28f 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPatternFormatting.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPatternFormatting.java
@@ -39,12 +39,12 @@ public class XSSFPatternFormatting implements PatternFormatting {
 
     public XSSFColor getFillBackgroundColorColor() {
         if(!_fill.isSetPatternFill()) return null;
-        return new XSSFColor(_fill.getPatternFill().getBgColor(), _colorMap);
+        return XSSFColor.from(_fill.getPatternFill().getBgColor(), _colorMap);
     }
     public XSSFColor getFillForegroundColorColor() {
         if(!_fill.isSetPatternFill() || ! _fill.getPatternFill().isSetFgColor())
             return null;
-        return new XSSFColor(_fill.getPatternFill().getFgColor(), _colorMap);
+        return XSSFColor.from(_fill.getPatternFill().getFgColor(), _colorMap);
     }
 
     public short getFillPattern() {
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
index a604f1844..c2ee5f5be 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
@@ -4022,7 +4022,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
         if (!pr.isSetTabColor()) {
             return null;
         }
-        return new XSSFColor(pr.getTabColor(), getWorkbook().getStylesSource().getIndexedColors());
+        return XSSFColor.from(pr.getTabColor(), getWorkbook().getStylesSource().getIndexedColors());
     }
     
     /**
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java
index 5d4d67587..a602888c1 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java
@@ -133,7 +133,7 @@ public class XSSFCellBorder {
         CTBorderPr borderPr = getBorder(side);
         
         if(borderPr != null && borderPr.isSetColor()) { 
-            XSSFColor clr = new XSSFColor(borderPr.getColor(), _indexedColorMap);
+            XSSFColor clr = XSSFColor.from(borderPr.getColor(), _indexedColorMap);
             if(_theme != null) {
                _theme.inheritFromThemeAsRequired(clr);
             }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java
index 5914b1d73..d46049051 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java
@@ -60,7 +60,7 @@ public final class XSSFCellFill {
         if (ptrn == null) return null;
 
         CTColor ctColor = ptrn.getBgColor();
-        return ctColor == null ? null : new XSSFColor(ctColor, _indexedColorMap);
+        return XSSFColor.from(ctColor, _indexedColorMap);
     }
 
     /**
@@ -81,7 +81,11 @@ public final class XSSFCellFill {
      */
     public void setFillBackgroundColor(XSSFColor color) {
         CTPatternFill ptrn = ensureCTPatternFill();
-        ptrn.setBgColor(color.getCTColor());
+        if (color == null) {
+            ptrn.unsetBgColor();
+        } else {
+            ptrn.setBgColor(color.getCTColor());
+        }
     }
 
     /**
@@ -94,7 +98,7 @@ public final class XSSFCellFill {
         if (ptrn == null) return null;
 
         CTColor ctColor = ptrn.getFgColor();
-        return ctColor == null ? null : new XSSFColor(ctColor, _indexedColorMap);
+        return XSSFColor.from(ctColor, _indexedColorMap);
     }
 
     /**
@@ -115,7 +119,11 @@ public final class XSSFCellFill {
      */
     public void setFillForegroundColor(XSSFColor color) {
         CTPatternFill ptrn = ensureCTPatternFill();
-        ptrn.setFgColor(color.getCTColor());
+        if (color == null) {
+            ptrn.unsetFgColor();
+        } else {
+            ptrn.setFgColor(color.getCTColor());
+        }
     }
 
     /**
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
index 95e5e67e6..f788619ad 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
@@ -2921,7 +2921,7 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         XSSFWorkbook workbook = new XSSFWorkbook();
         XSSFCell cell = workbook.createSheet().createRow(0).createCell(0);
 
-        XSSFColor color = new XSSFColor(java.awt.Color.RED);
+        XSSFColor color = new XSSFColor(java.awt.Color.RED, workbook.getStylesSource().getIndexedColors());
         XSSFCellStyle style = workbook.createCellStyle();
         style.setFillForegroundColor(color);
         style.setFillPattern(FillPatternType.SOLID_FOREGROUND);
@@ -2941,7 +2941,7 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         XSSFWorkbook nwb = XSSFTestDataSamples.writeOutAndReadBack(workbook);
         workbook.close();
         XSSFCell ncell = nwb.getSheetAt(0).getRow(0).getCell(0);
-        XSSFColor ncolor = new XSSFColor(java.awt.Color.RED);
+        XSSFColor ncolor = new XSSFColor(java.awt.Color.RED, workbook.getStylesSource().getIndexedColors());
 
         // Now the cell is all black
         XSSFColor nactual = ncell.getCellStyle().getFillBackgroundColorColor();
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
index d7b632f38..cee157827 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
@@ -359,7 +359,7 @@ public class TestXSSFCellStyle {
 
         //setting XSSFColor
         num = stylesTable.getBorders().size();
-        clr = new XSSFColor(java.awt.Color.CYAN);
+        clr = new XSSFColor(java.awt.Color.CYAN, stylesTable.getIndexedColors());
         cellStyle.setBottomBorderColor(clr);
         assertEquals(clr.getCTColor().toString(), cellStyle.getBottomBorderXSSFColor().getCTColor().toString());
         byte[] rgb = cellStyle.getBottomBorderXSSFColor().getRGB();
@@ -400,7 +400,7 @@ public class TestXSSFCellStyle {
 
         //setting XSSFColor
         num = stylesTable.getBorders().size();
-        clr = new XSSFColor(java.awt.Color.CYAN);
+        clr = new XSSFColor(java.awt.Color.CYAN, stylesTable.getIndexedColors());
         cellStyle.setTopBorderColor(clr);
         assertEquals(clr.getCTColor().toString(), cellStyle.getTopBorderXSSFColor().getCTColor().toString());
         byte[] rgb = cellStyle.getTopBorderXSSFColor().getRGB();
@@ -441,7 +441,7 @@ public class TestXSSFCellStyle {
 
         //setting XSSFColor
         num = stylesTable.getBorders().size();
-        clr = new XSSFColor(java.awt.Color.CYAN);
+        clr = new XSSFColor(java.awt.Color.CYAN, stylesTable.getIndexedColors());
         cellStyle.setLeftBorderColor(clr);
         assertEquals(clr.getCTColor().toString(), cellStyle.getLeftBorderXSSFColor().getCTColor().toString());
         byte[] rgb = cellStyle.getLeftBorderXSSFColor().getRGB();
@@ -482,7 +482,7 @@ public class TestXSSFCellStyle {
 
         //setting XSSFColor
         num = stylesTable.getBorders().size();
-        clr = new XSSFColor(java.awt.Color.CYAN);
+        clr = new XSSFColor(java.awt.Color.CYAN, stylesTable.getIndexedColors());
         cellStyle.setRightBorderColor(clr);
         assertEquals(clr.getCTColor().toString(), cellStyle.getRightBorderXSSFColor().getCTColor().toString());
         byte[] rgb = cellStyle.getRightBorderXSSFColor().getRGB();
@@ -523,7 +523,7 @@ public class TestXSSFCellStyle {
 
         //setting XSSFColor
         num = stylesTable.getFills().size();
-        clr = new XSSFColor(java.awt.Color.CYAN);
+        clr = new XSSFColor(java.awt.Color.CYAN, stylesTable.getIndexedColors());
         cellStyle.setFillBackgroundColor(clr);
         assertEquals(clr.getCTColor().toString(), cellStyle.getFillBackgroundXSSFColor().getCTColor().toString());
         byte[] rgb = cellStyle.getFillBackgroundXSSFColor().getRGB();
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java
index 8eee77dfc..96b14bda3 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java
@@ -70,7 +70,7 @@ public class TestXSSFConditionalFormatting extends BaseTestConditionalFormatting
 
     @Test
     public void testFontFormattingColor() {
-        Workbook wb = XSSFITestDataProvider.instance.createWorkbook();
+        XSSFWorkbook wb = XSSFITestDataProvider.instance.createWorkbook();
         final Sheet sheet = wb.createSheet();
 
         final SheetConditionalFormatting formatting = sheet.getSheetConditionalFormatting();
@@ -101,7 +101,7 @@ public class TestXSSFConditionalFormatting extends BaseTestConditionalFormatting
         assertEquals(-1, fontFmt.getFontColorIndex());
 
         //fontFmt.setFontColorIndex((short)11);
-        final ExtendedColor extendedColor = new XSSFColor(PEAK_ORANGE);
+        final ExtendedColor extendedColor = new XSSFColor(PEAK_ORANGE, wb.getStylesSource().getIndexedColors());
         fontFmt.setFontColor(extendedColor);
 
         PatternFormatting patternFmt = formattingRule.createPatternFormatting();
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java
index 75a87ddba..76ffac8ba 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java
@@ -223,7 +223,7 @@ public class TestXSSFDrawing {
         XSSFRichTextString rt = new XSSFRichTextString("Test String");
 
         XSSFFont font = wb.createFont();
-        font.setColor(new XSSFColor(new Color(0, 128, 128)));
+        font.setColor(new XSSFColor(new Color(0, 128, 128), wb.getStylesSource().getIndexedColors()));
         font.setItalic(true);
         font.setBold(true);
         font.setUnderline(FontUnderline.SINGLE);
@@ -297,7 +297,7 @@ public class TestXSSFDrawing {
         XSSFRichTextString rt = new XSSFRichTextString("Test String");
 
         XSSFFont font = wb.createFont();
-        font.setColor(new XSSFColor(new Color(0, 128, 128)));
+        font.setColor(new XSSFColor(new Color(0, 128, 128), wb.getStylesSource().getIndexedColors()));
         font.setFontName("Arial");
         rt.applyFont(font);
 
@@ -328,7 +328,7 @@ public class TestXSSFDrawing {
         XSSFRichTextString rt = new XSSFRichTextString("Test String");
 
         XSSFFont font = wb.createFont();
-        font.setColor(new XSSFColor(new Color(0, 255, 255)));
+        font.setColor(new XSSFColor(new Color(0, 255, 255), wb.getStylesSource().getIndexedColors()));
         font.setFontName("Arial");
         rt.applyFont(font);
 
@@ -389,12 +389,12 @@ public class TestXSSFDrawing {
         XSSFRichTextString rt = new XSSFRichTextString("Test Rich Text String");
 
         XSSFFont font = wb1.createFont();
-        font.setColor(new XSSFColor(new Color(0, 255, 255)));
+        font.setColor(new XSSFColor(new Color(0, 255, 255), wb1.getStylesSource().getIndexedColors()));
         font.setFontName("Arial");
         rt.applyFont(font);
 
         XSSFFont midfont = wb1.createFont();
-        midfont.setColor(new XSSFColor(new Color(0, 255, 0)));
+        midfont.setColor(new XSSFColor(new Color(0, 255, 0), wb1.getStylesSource().getIndexedColors()));
         rt.applyFont(5, 14, midfont);	// set the text "Rich Text" to be green and the default font
 
         XSSFTextParagraph para = shape.addNewTextParagraph(rt);
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java
index dd198edf8..3ccf5a337 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java
@@ -256,7 +256,7 @@ public final class TestXSSFFont extends BaseTestFont{
 		
 		byte[] bytes = Integer.toHexString(0xF1F1F1).getBytes(LocaleUtil.CHARSET_1252);
         color.setRgb(bytes);
-		XSSFColor newColor=new XSSFColor(color, null);
+		XSSFColor newColor=XSSFColor.from(color, null);
 		xssfFont.setColor(newColor);
 		assertEquals(ctFont.getColorArray(0).getRgb()[2],newColor.getRGB()[2]);
 		
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
index 09adc3561..c8b5821c4 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
@@ -71,6 +71,7 @@ import org.junit.Test;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCalcPr;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCol;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComments;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTIgnoredError;
@@ -1924,7 +1925,7 @@ public final class TestXSSFSheet extends BaseTestXSheet {
             assertEquals(expected, wb.getSheet("indexedRed").getTabColor());
 
             // test regular-colored (non-indexed, ARGB) sheet
-            expected = new XSSFColor();
+            expected = XSSFColor.from(CTColor.Factory.newInstance(), wb.getStylesSource().getIndexedColors());
             expected.setARGBHex("FF7F2700");
             assertEquals(expected, wb.getSheet("customOrange").getTabColor());
         }
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
index e9eebe68e..06733425e 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
@@ -38,7 +38,7 @@ public class TestXSSFSimpleShape {
             
             XSSFFont font = wb.createFont();
             Color color = new Color(0, 255, 255);
-            font.setColor(new XSSFColor(color));
+            font.setColor(new XSSFColor(color, wb.getStylesSource().getIndexedColors()));
             font.setFontName("Arial");
             rt.applyFont(font);
     
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTextParagraph.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTextParagraph.java
index 7952f1f01..9ba2aa9fc 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTextParagraph.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTextParagraph.java
@@ -37,7 +37,7 @@ public class TestXSSFTextParagraph {
     
             XSSFFont font = wb.createFont();
             Color color = new Color(0, 255, 255);
-            font.setColor(new XSSFColor(color));
+            font.setColor(new XSSFColor(color, wb.getStylesSource().getIndexedColors()));
             font.setFontName("Arial");
             rt.applyFont(font);
     
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java
index c9279e26d..97e450b84 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java
@@ -20,12 +20,14 @@ package org.apache.poi.xssf.usermodel.extensions;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 import java.io.IOException;
 
 import org.apache.poi.ss.usermodel.FillPatternType;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.apache.poi.xssf.usermodel.XSSFCell;
+import org.apache.poi.xssf.usermodel.XSSFCellStyle;
 import org.apache.poi.xssf.usermodel.XSSFColor;
 import org.apache.poi.xssf.usermodel.XSSFWorkbook;
 import org.junit.Test;
@@ -98,4 +100,14 @@ public class TestXSSFCellFill {
         assertEquals(rgbWithTint[2],-80);
         wb.close();
     }
+    
+    @Test
+    public void testFillWithoutColors() {
+        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("FillWithoutColor.xlsx");
+        XSSFCell cellWithFill = wb.getSheetAt(0).getRow(5).getCell(1);
+        XSSFCellStyle style = cellWithFill.getCellStyle();
+        assertNotNull(style);
+        assertNull("had an empty background color", style.getFillBackgroundColorColor());
+        assertNull("had an empty background color", style.getFillBackgroundXSSFColor());
+    }
 }