From f90801d3662c3cb63302bbf836a6a45239e5c44a Mon Sep 17 00:00:00 2001 From: Greg Woolsey 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 | 33 +++++++++++++-- .../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, 113 insertions(+), 66 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 ac0dfc0835..11ffafc63e 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 4965c7922e..9b02214576 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 88a910b89a..dc40bdcc38 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 423baf8a5f..bbbe08395b 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 e7dbbb99ca..ae206f3c34 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 + * new color with the given indexed color map + * @param colorMap + */ + public XSSFColor(IndexedColorMap colorMap) { + this(CTColor.Factory.newInstance(), colorMap); + } + + /** + * TEST ONLY * @param clr awt Color + * @param map */ - public XSSFColor(java.awt.Color clr) { - this(); + 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 8a6b29d21b..31f5f28373 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