tag r1847494 as 4.0.1REL_4_0_1

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

15 files changed, 205 insertions, 141 deletions

diff --git a/src/java/org/apache/poi/hpsf/VariantSupport.java b/src/java/org/apache/poi/hpsf/VariantSupport.java
index e60679027d..0e8dc08c6e 100644
--- a/src/java/org/apache/poi/hpsf/VariantSupport.java
+++ b/src/java/org/apache/poi/hpsf/VariantSupport.java
@@ -36,7 +36,7 @@ import org.apache.poi.util.POILogger;
  * Supports reading and writing of variant data.<p>
  *
  * <strong>FIXME (3):</strong> Reading and writing should be made more
- * uniform than it is now. The following items should be resolved:<p>
+ * uniform than it is now. The following items should be resolved:
  *
  * <ul>
  *
diff --git a/src/java/org/apache/poi/hpsf/wellknown/PropertyIDMap.java b/src/java/org/apache/poi/hpsf/wellknown/PropertyIDMap.java
index 130408e7b5..c01ded7fe8 100644
--- a/src/java/org/apache/poi/hpsf/wellknown/PropertyIDMap.java
+++ b/src/java/org/apache/poi/hpsf/wellknown/PropertyIDMap.java
@@ -33,8 +33,11 @@ import org.apache.poi.hpsf.SummaryInformation;
  * The methods {@link #getSummaryInformationProperties} and {@link
  * #getDocumentSummaryInformationProperties} return singleton {@link
  * PropertyIDMap}s. An application that wants to extend these maps
- * should treat them as unmodifiable, copy them and modifiy the
+ * should treat them as unmodifiable, copy them and modify the
  * copies.
+ *
+ * Trying to modify the map directly will cause exceptions
+ * {@link UnsupportedOperationException} to be thrown.
  */
 public class PropertyIDMap implements Map<Long,String> {
 
@@ -490,11 +493,13 @@ public class PropertyIDMap implements Map<Long,String> {
 
     @Override
     public String put(Long key, String value) {
+        //noinspection ConstantConditions
         return idMap.put(key, value);
     }
 
     @Override
     public String remove(Object key) {
+        //noinspection ConstantConditions
         return idMap.remove(key);
     }
 
diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
index 46d8678229..4ff370125c 100644
--- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
+++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
@@ -2276,6 +2276,8 @@ public final class InternalWorkbook {
 
     /**
      * Only for internal calls - code based on this is not supported ...
+     *
+     * @return The list of records.
      */
     @Internal
     public WorkbookRecordList getWorkbookRecordList() {
diff --git a/src/java/org/apache/poi/hssf/record/RecordInputStream.java b/src/java/org/apache/poi/hssf/record/RecordInputStream.java
index 4f5f589032..8347655a81 100644
--- a/src/java/org/apache/poi/hssf/record/RecordInputStream.java
+++ b/src/java/org/apache/poi/hssf/record/RecordInputStream.java
@@ -33,8 +33,9 @@ import org.apache.poi.util.LittleEndianInputStream;
 import org.apache.poi.util.RecordFormatException;
 
 /**
- * Title:  Record Input Stream<P>
- * Description:  Wraps a stream and provides helper methods for the construction of records.<P>
+ * Title:  Record Input Stream
+ *
+ * Description:  Wraps a stream and provides helper methods for the construction of records.
  */
 public final class RecordInputStream implements LittleEndianInput {
 
@@ -194,11 +195,11 @@ public final class RecordInputStream implements LittleEndianInput {
 	private int readNextSid() {
 		int nAvailable  = _bhi.available();
 		if (nAvailable < EOFRecord.ENCODED_SIZE) {
-			if (nAvailable > 0) {
+			/*if (nAvailable > 0) {
 				// some scrap left over?
 				// ex45582-22397.xls has one extra byte after the last record
 				// Excel reads that file OK
-			}
+			}*/
 			return INVALID_SID_VALUE;
 		}
 		int result = _bhi.readRecordSID();
@@ -305,14 +306,13 @@ public final class RecordInputStream implements LittleEndianInput {
 	@Override
     public double readDouble() {
 		long valueLongBits = readLong();
-		double result = Double.longBitsToDouble(valueLongBits);
-		if (Double.isNaN(result)) {
+		/*if (Double.isNaN(result)) {
             // YK: Excel doesn't write NaN but instead converts the cell type into {@link CellType#ERROR}.
             // HSSF prior to version 3.7 had a bug: it could write Double.NaN but could not read such a file back.
             // This behavior was fixed in POI-3.7.
             //throw new RuntimeException("Did not expect to read NaN"); // (Because Excel typically doesn't write NaN)
-		}
-		return result;
+		}*/
+		return Double.longBitsToDouble(valueLongBits);
 	}
 	
 	public void readPlain(byte[] buf, int off, int len) {
diff --git a/src/java/org/apache/poi/hssf/record/SSTRecord.java b/src/java/org/apache/poi/hssf/record/SSTRecord.java
index 20d99319d6..1de2fe4fc2 100644
--- a/src/java/org/apache/poi/hssf/record/SSTRecord.java
+++ b/src/java/org/apache/poi/hssf/record/SSTRecord.java
@@ -161,7 +161,7 @@ public final class SSTRecord extends ContinuableRecord {
      * <P>
      * The data consists of sets of string data. This string data is
      * arranged as follows:
-     * <P>
+     * </P><P>
      * <pre>
      * short  string_length;   // length of string data
      * byte   string_flag;     // flag specifying special string
@@ -176,9 +176,9 @@ public final class SSTRecord extends ContinuableRecord {
      * byte[] extension;       // optional extension (length of array
      *                         // is extend_length)
      * </pre>
-     * <P>
+     * </P><P>
      * The string_flag is bit mapped as follows:
-     * <P>
+     * </P><P>
      * <TABLE summary="string_flag mapping">
      *   <TR>
      *      <TH>Bit number</TH>
@@ -232,7 +232,7 @@ public final class SSTRecord extends ContinuableRecord {
      * associated data. The UnicodeString class can handle the byte[]
      * vs short[] nature of the actual string data
      *
-     * @param in the RecordInputstream to read the record from
+     * @param in the RecordInputStream to read the record from
      */
     public SSTRecord(RecordInputStream in) {
         // this method is ALWAYS called after construction -- using
diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
index 770f740a09..328402db80 100644
--- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
+++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
@@ -77,7 +77,7 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
 
     public String toString()
     {
-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
 
         buffer.append("[SHARED FORMULA (").append(HexDump.intToHex(sid)).append("]\n");
         buffer.append("    .range      = ").append(getRange()).append("\n");
@@ -99,6 +99,10 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
     }
 
     /**
+     * Convert formula into an array of {@link Ptg} tokens.
+     *
+     * @param formula The record to break into tokens, cannot be null
+     *
      * @return the equivalent {@link Ptg} array that the formula would have, were it not shared.
      */
     public Ptg[] getFormulaTokens(FormulaRecord formula) {
diff --git a/src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java b/src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java
index a5035303a3..51fe091808 100644
--- a/src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java
+++ b/src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java
@@ -42,6 +42,8 @@ public abstract class SharedValueRecordBase extends StandardRecord {
 
 	/**
 	 * reads only the range (1 {@link CellRangeAddress8Bit}) from the stream
+	 *
+	 * @param in The interface for reading the record data.
 	 */
 	public SharedValueRecordBase(LittleEndianInput in) {
 		_range = new CellRangeAddress8Bit(in);
@@ -99,14 +101,12 @@ public abstract class SharedValueRecordBase extends StandardRecord {
 			&& r.getLastColumn() >= colIx;
 	}
 	/**
-	 * @return {@code true} if (rowIx, colIx) describes the first cell in this shared value
-	 * object's range
-	 * 
 	 * @param rowIx the row index
 	 * @param colIx the column index
-	 * 
-	 * @return {@code true} if its the first cell in this shared value object range
-	 * 
+	 *
+	 * @return {@code true} if (rowIx, colIx) describes the first cell in this shared value
+	 * object's range
+	 *
 	 * @see #getRange()
 	 */
 	public final boolean isFirstCell(int rowIx, int colIx) {
diff --git a/src/java/org/apache/poi/poifs/filesystem/FileMagic.java b/src/java/org/apache/poi/poifs/filesystem/FileMagic.java
index 4ac616082e..bab62c6437 100644
--- a/src/java/org/apache/poi/poifs/filesystem/FileMagic.java
+++ b/src/java/org/apache/poi/poifs/filesystem/FileMagic.java
@@ -78,7 +78,7 @@ public enum FileMagic {
     /** PDF document */
     PDF("%PDF"),
     /** Some different HTML documents */
-    HTML("<!DOCTYP".getBytes(UTF_8), "<html".getBytes(UTF_8)),
+    HTML("<!DOCTYP".getBytes(UTF_8), "<html".getBytes(UTF_8), "<HTML".getBytes(UTF_8)),
     WORD2(new byte[]{ (byte)0xdb, (byte)0xa5, 0x2d, 0x00}),
     // keep UNKNOWN always as last enum!
     /** UNKNOWN magic */
@@ -101,17 +101,8 @@ public enum FileMagic {
 
     public static FileMagic valueOf(byte[] magic) {
         for (FileMagic fm : values()) {
-            int i=0;
-            boolean found = true;
             for (byte[] ma : fm.magic) {
-                for (byte m : ma) {
-                    byte d = magic[i++];
-                    if (!(d == m || (m == 0x70 && (d == 0x10 || d == 0x20 || d == 0x40)))) {
-                        found = false;
-                        break;
-                    }
-                }
-                if (found) {
+                if (findMagic(ma, magic)) {
                     return fm;
                 }
             }
@@ -119,6 +110,17 @@ public enum FileMagic {
         return UNKNOWN;
     }
 
+    private static boolean findMagic(byte[] cmp, byte[] actual) {
+        int i=0;
+        for (byte m : cmp) {
+            byte d = actual[i++];
+            if (!(d == m || (m == 0x70 && (d == 0x10 || d == 0x20 || d == 0x40)))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
 
     /**
      * Get the file magic of the supplied {@link File}<p>
diff --git a/src/java/org/apache/poi/sl/draw/DrawPaint.java b/src/java/org/apache/poi/sl/draw/DrawPaint.java
index d6986f3023..a830605385 100644
--- a/src/java/org/apache/poi/sl/draw/DrawPaint.java
+++ b/src/java/org/apache/poi/sl/draw/DrawPaint.java
@@ -29,7 +29,10 @@ import java.awt.geom.Rectangle2D;
 import java.awt.image.BufferedImage;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Map;
 import java.util.Objects;
+import java.util.TreeMap;
+import java.util.function.BiFunction;
 
 import org.apache.poi.sl.usermodel.AbstractColorStyle;
 import org.apache.poi.sl.usermodel.ColorStyle;
@@ -197,28 +200,17 @@ public class DrawPaint {
 
             @Override
             public int getShade() {
-                int shade = orig.getShade();
-                switch (modifier) {
-                    case DARKEN:
-                        return Math.min(100000, Math.max(0,shade)+40000);
-                    case DARKEN_LESS:
-                        return Math.min(100000, Math.max(0,shade)+20000);
-                    default:
-                        return shade;
-                }
+                return scale(orig.getShade(), PaintModifier.DARKEN_LESS, PaintModifier.DARKEN);
             }
 
             @Override
             public int getTint() {
-                int tint = orig.getTint();
-                switch (modifier) {
-                    case LIGHTEN:
-                        return Math.min(100000, Math.max(0,tint)+40000);
-                    case LIGHTEN_LESS:
-                        return Math.min(100000, Math.max(0,tint)+20000);
-                    default:
-                        return tint;
-                }
+                return scale(orig.getTint(), PaintModifier.LIGHTEN_LESS, PaintModifier.LIGHTEN);
+            }
+
+            private int scale(int value, PaintModifier lessModifier, PaintModifier moreModifier) {
+                int delta = (modifier == lessModifier ? 20000 : (modifier == moreModifier ? 40000 : 0));
+                return Math.min(100000, Math.max(0,value)+delta);
             }
         };
 
@@ -300,7 +292,7 @@ public class DrawPaint {
         Color result = color.getColor();
 
         double alpha = getAlpha(result, color);
-        double hsl[] = RGB2HSL(result); // values are in the range [0..100] (usually ...)
+        double[] hsl = RGB2HSL(result); // values are in the range [0..100] (usually ...)
         applyHslModOff(hsl, 0, color.getHueMod(), color.getHueOff());
         applyHslModOff(hsl, 1, color.getSatMod(), color.getSatOff());
         applyHslModOff(hsl, 2, color.getLumMod(), color.getLumOff());
@@ -344,7 +336,7 @@ public class DrawPaint {
      * @param mod the modulation adjustment
      * @param off the offset adjustment
      */
-    private static void applyHslModOff(double hsl[], int hslPart, int mod, int off) {
+    private static void applyHslModOff(double[] hsl, int hslPart, int mod, int off) {
         if (mod == -1) {
             mod = 100000;
         }
@@ -363,7 +355,7 @@ public class DrawPaint {
      *
      * For a shade, the equation is luminance * %tint.
      */
-    private static void applyShade(double hsl[], ColorStyle fc) {
+    private static void applyShade(double[] hsl, ColorStyle fc) {
         int shade = fc.getShade();
         if (shade == -1) {
             return;
@@ -380,7 +372,7 @@ public class DrawPaint {
      * For a tint, the equation is luminance * %tint + (1-%tint).
      * (Note that 1-%tint is equal to the lumOff value in DrawingML.)
      */
-    private static void applyTint(double hsl[], ColorStyle fc) {
+    private static void applyTint(double[] hsl, ColorStyle fc) {
         int tint = fc.getTint();
         if (tint == -1) {
             return;
@@ -403,70 +395,63 @@ public class DrawPaint {
         }
 
         Rectangle2D anchor = DrawShape.getAnchor(graphics, shape);
-        final double h = anchor.getHeight(), w = anchor.getWidth(), x = anchor.getX(), y = anchor.getY();
 
         AffineTransform at = AffineTransform.getRotateInstance(Math.toRadians(angle), anchor.getCenterX(), anchor.getCenterY());
 
-        double diagonal = Math.sqrt(h * h + w * w);
-        Point2D p1 = new Point2D.Double(x + w / 2 - diagonal / 2, y + h / 2);
-        p1 = at.transform(p1, null);
-
-        Point2D p2 = new Point2D.Double(x + w, y + h / 2);
-        p2 = at.transform(p2, null);
+        double diagonal = Math.sqrt(Math.pow(anchor.getWidth(),2) + Math.pow(anchor.getHeight(),2));
+        final Point2D p1 = at.transform(new Point2D.Double(anchor.getCenterX() - diagonal / 2, anchor.getCenterY()), null);
+        final Point2D p2 = at.transform(new Point2D.Double(anchor.getMaxX(), anchor.getCenterY()), null);
 
 //        snapToAnchor(p1, anchor);
 //        snapToAnchor(p2, anchor);
 
-        if (p1.equals(p2)) {
-            // gradient paint on the same point throws an exception ... and doesn't make sense
-            return null;
-        }
-
-        float[] fractions = fill.getGradientFractions();
-        Color[] colors = new Color[fractions.length];
-
-        int i = 0;
-        for (ColorStyle fc : fill.getGradientColors()) {
-            // if fc is null, use transparent color to get color of background
-            colors[i++] = (fc == null) ? TRANSPARENT : applyColorTransform(fc);
-        }
-
-        return new LinearGradientPaint(p1, p2, fractions, colors);
+        // gradient paint on the same point throws an exception ... and doesn't make sense
+        return (p1.equals(p2)) ? null : safeFractions((f,c)->new LinearGradientPaint(p1,p2,f,c), fill);
     }
 
+
     @SuppressWarnings("WeakerAccess")
     protected Paint createRadialGradientPaint(GradientPaint fill, Graphics2D graphics) {
         Rectangle2D anchor = DrawShape.getAnchor(graphics, shape);
 
-        Point2D pCenter = new Point2D.Double(anchor.getX() + anchor.getWidth()/2,
-                anchor.getY() + anchor.getHeight()/2);
+        final Point2D pCenter = new Point2D.Double(anchor.getCenterX(), anchor.getCenterY());
 
-        float radius = (float)Math.max(anchor.getWidth(), anchor.getHeight());
+        final float radius = (float)Math.max(anchor.getWidth(), anchor.getHeight());
 
-        float[] fractions = fill.getGradientFractions();
-        Color[] colors = new Color[fractions.length];
-
-        int i=0;
-        for (ColorStyle fc : fill.getGradientColors()) {
-            colors[i++] = applyColorTransform(fc);
-        }
-
-        return new RadialGradientPaint(pCenter, radius, fractions, colors);
+        return safeFractions((f,c)->new RadialGradientPaint(pCenter,radius,f,c), fill);
     }
 
     @SuppressWarnings({"WeakerAccess", "unused"})
     protected Paint createPathGradientPaint(GradientPaint fill, Graphics2D graphics) {
         // currently we ignore an eventually center setting
 
+        return safeFractions(PathGradientPaint::new, fill);
+    }
+
+    private Paint safeFractions(BiFunction<float[],Color[],Paint> init, GradientPaint fill) {
         float[] fractions = fill.getGradientFractions();
-        Color[] colors = new Color[fractions.length];
+        final ColorStyle[] styles = fill.getGradientColors();
+
+        // need to remap the fractions, because Java doesn't like repeating fraction values
+        Map<Float,Color> m = new TreeMap<>();
+        for (int i = 0; i<fractions.length; i++) {
+            // if fc is null, use transparent color to get color of background
+            m.put(fractions[i], (styles[i] == null ? TRANSPARENT : applyColorTransform(styles[i])));
+        }
+
+        final Color[] colors = new Color[m.size()];
+        if (fractions.length != m.size()) {
+            fractions = new float[m.size()];
+        }
 
         int i=0;
-        for (ColorStyle fc : fill.getGradientColors()) {
-            colors[i++] = applyColorTransform(fc);
+        for (Map.Entry<Float,Color> me : m.entrySet()) {
+            fractions[i] = me.getKey();
+            colors[i] = me.getValue();
+            i++;
         }
 
-        return new PathGradientPaint(colors, fractions);
+        return init.apply(fractions, colors);
     }
 
     /**
diff --git a/src/java/org/apache/poi/sl/draw/PathGradientPaint.java b/src/java/org/apache/poi/sl/draw/PathGradientPaint.java
index d4a2a5fa5f..2281bbff83 100644
--- a/src/java/org/apache/poi/sl/draw/PathGradientPaint.java
+++ b/src/java/org/apache/poi/sl/draw/PathGradientPaint.java
@@ -23,21 +23,24 @@ import java.awt.MultipleGradientPaint.CycleMethod;
 import java.awt.geom.*;
 import java.awt.image.*;
 
+import org.apache.poi.util.Internal;
+
+@Internal
 class PathGradientPaint implements Paint {
 
     // http://asserttrue.blogspot.de/2010/01/how-to-iimplement-custom-paint-in-50.html
-    protected final Color colors[];
-    protected final float fractions[];
-    protected final int capStyle;
-    protected final int joinStyle;
-    protected final int transparency;
+    private final Color[] colors;
+    private final float[] fractions;
+    private final int capStyle;
+    private final int joinStyle;
+    private final int transparency;
 
     
-    public PathGradientPaint(Color colors[], float fractions[]) {
-        this(colors,fractions,BasicStroke.CAP_ROUND,BasicStroke.JOIN_ROUND);
+    PathGradientPaint(float[] fractions, Color[] colors) {
+        this(fractions,colors,BasicStroke.CAP_ROUND,BasicStroke.JOIN_ROUND);
     }
     
-    public PathGradientPaint(Color colors[], float fractions[], int capStyle, int joinStyle) {
+    private PathGradientPaint(float[] fractions, Color[] colors, int capStyle, int joinStyle) {
         this.colors = colors.clone();
         this.fractions = fractions.clone();
         this.capStyle = capStyle;
@@ -66,26 +69,26 @@ class PathGradientPaint implements Paint {
     }
 
     class PathGradientContext implements PaintContext {
-        protected final Rectangle deviceBounds;
-        protected final Rectangle2D userBounds;
+        final Rectangle deviceBounds;
+        final Rectangle2D userBounds;
         protected final AffineTransform xform;
-        protected final RenderingHints hints;
+        final RenderingHints hints;
 
         /**
          * for POI: the shape will be only known when the subclasses determines the concrete implementation 
          * in the draw/-content method, so we need to postpone the setting/creation as long as possible
          **/
         protected final Shape shape;
-        protected final PaintContext pCtx;
-        protected final int gradientSteps;
+        final PaintContext pCtx;
+        final int gradientSteps;
         WritableRaster raster;
 
-        public PathGradientContext(
-              ColorModel cm
-            , Rectangle deviceBounds
-            , Rectangle2D userBounds
-            , AffineTransform xform
-            , RenderingHints hints
+        PathGradientContext(
+                ColorModel cm
+                , Rectangle deviceBounds
+                , Rectangle2D userBounds
+                , AffineTransform xform
+                , RenderingHints hints
         ) {
             shape = (Shape)hints.get(Drawable.GRADIENT_SHAPE);
             if (shape == null) {
@@ -139,7 +142,7 @@ class PathGradientPaint implements Paint {
             return childRaster;
         }
 
-        protected int getGradientSteps(Shape gradientShape) {
+        int getGradientSteps(Shape gradientShape) {
             Rectangle rect = gradientShape.getBounds();
             int lower = 1;
             int upper = (int)(Math.max(rect.getWidth(),rect.getHeight())/2.0);
@@ -158,7 +161,7 @@ class PathGradientPaint implements Paint {
         
         
         
-        protected void createRaster() {
+        void createRaster() {
             ColorModel cm = getColorModel();
             raster = cm.createCompatibleWritableRaster((int)deviceBounds.getWidth(), (int)deviceBounds.getHeight());
             BufferedImage img = new BufferedImage(cm, raster, false, null);
@@ -168,7 +171,7 @@ class PathGradientPaint implements Paint {
             graphics.transform(xform);
 
             Raster img2 = pCtx.getRaster(0, 0, gradientSteps, 1);
-            int rgb[] = new int[cm.getNumComponents()];
+            int[] rgb = new int[cm.getNumComponents()];
 
             for (int i = gradientSteps-1; i>=0; i--) {
                 img2.getPixel(i, 0, rgb);
diff --git a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java b/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java
index c6e13c3110..6e10e67c58 100644
--- a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java
+++ b/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java
@@ -59,6 +59,7 @@ public final class SheetNameFormatter {
      * @param rawSheetName - sheet name
      * @deprecated use <code>appendFormat(StringBuilder out, String rawSheetName)</code> instead
 	 */
+	@Deprecated
 	public static void appendFormat(StringBuffer out, String rawSheetName) {
 		boolean needsQuotes = needsDelimiting(rawSheetName);
 		if(needsQuotes) {
@@ -73,6 +74,7 @@ public final class SheetNameFormatter {
     /**
      * @deprecated use <code>appendFormat(StringBuilder out, String workbookName, String rawSheetName)</code> instead
      */
+    @Deprecated
 	public static void appendFormat(StringBuffer out, String workbookName, String rawSheetName) {
 		boolean needsQuotes = needsDelimiting(workbookName) || needsDelimiting(rawSheetName);
 		if(needsQuotes) {
@@ -123,7 +125,7 @@ public final class SheetNameFormatter {
 		}
 	}
 
-    private static void appendAndEscape(Appendable sb, String rawSheetName) {
+    static void appendAndEscape(Appendable sb, String rawSheetName) {
         int len = rawSheetName.length();
         for(int i=0; i<len; i++) {
             char ch = rawSheetName.charAt(i);
@@ -139,7 +141,12 @@ public final class SheetNameFormatter {
         }
     }
 
-	private static boolean needsDelimiting(String rawSheetName) {
+	/**
+	 * Tell if the given raw sheet name needs screening/delimiting.
+	 * @param rawSheetName the sheet name.
+	 * @return true if the given raw sheet name needs screening/delimiting, false otherwise.
+	 */
+	static boolean needsDelimiting(String rawSheetName) {
 		int len = rawSheetName.length();
 		if(len < 1) {
 			throw new RuntimeException("Zero length string is an invalid sheet name");
diff --git a/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java b/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java
new file mode 100644
index 0000000000..24d39cf274
--- /dev/null
+++ b/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java
@@ -0,0 +1,73 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.ss.formula;
+
+public class SheetRangeAndWorkbookIndexFormatter {
+    private SheetRangeAndWorkbookIndexFormatter() {
+    }
+
+    public static String format(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        if (anySheetNameNeedsEscaping(firstSheetName, lastSheetName)) {
+            return formatWithDelimiting(sb, workbookIndex, firstSheetName, lastSheetName);
+        } else {
+            return formatWithoutDelimiting(sb, workbookIndex, firstSheetName, lastSheetName);
+        }
+    }
+
+    private static String formatWithDelimiting(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        sb.append('\'');
+        if (workbookIndex >= 0) {
+            sb.append('[');
+            sb.append(workbookIndex);
+            sb.append(']');
+        }
+
+        SheetNameFormatter.appendAndEscape(sb, firstSheetName);
+
+        if (lastSheetName != null) {
+            sb.append(':');
+            SheetNameFormatter.appendAndEscape(sb, lastSheetName);
+        }
+
+        sb.append('\'');
+        return sb.toString();
+    }
+
+    private static String formatWithoutDelimiting(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        if (workbookIndex >= 0) {
+            sb.append('[');
+            sb.append(workbookIndex);
+            sb.append(']');
+        }
+
+        sb.append(firstSheetName);
+
+        if (lastSheetName != null) {
+            sb.append(':');
+            sb.append(lastSheetName);
+        }
+
+        return sb.toString();
+    }
+
+    private static boolean anySheetNameNeedsEscaping(String firstSheetName, String lastSheetName) {
+        boolean anySheetNameNeedsDelimiting = firstSheetName != null && SheetNameFormatter.needsDelimiting(firstSheetName);
+        anySheetNameNeedsDelimiting |= lastSheetName != null && SheetNameFormatter.needsDelimiting(lastSheetName);
+        return anySheetNameNeedsDelimiting;
+    }
+}
diff --git a/src/java/org/apache/poi/ss/formula/ptg/Area3DPxg.java b/src/java/org/apache/poi/ss/formula/ptg/Area3DPxg.java
index 65f59e83e1..41bea0c950 100644
--- a/src/java/org/apache/poi/ss/formula/ptg/Area3DPxg.java
+++ b/src/java/org/apache/poi/ss/formula/ptg/Area3DPxg.java
@@ -20,6 +20,7 @@ package org.apache.poi.ss.formula.ptg;
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.formula.SheetIdentifier;
 import org.apache.poi.ss.formula.SheetNameFormatter;
+import org.apache.poi.ss.formula.SheetRangeAndWorkbookIndexFormatter;
 import org.apache.poi.ss.formula.SheetRangeIdentifier;
 import org.apache.poi.ss.util.AreaReference;
 import org.apache.poi.util.LittleEndianOutput;
@@ -102,16 +103,8 @@ public final class Area3DPxg extends AreaPtgBase implements Pxg3D {
     
     public String toFormulaString() {
         StringBuilder sb = new StringBuilder(64);
-        if (externalWorkbookNumber >= 0) {
-            sb.append('[');
-            sb.append(externalWorkbookNumber);
-            sb.append(']');
-        }
-        SheetNameFormatter.appendFormat(sb, firstSheetName);
-        if (lastSheetName != null) {
-            sb.append(':');
-            SheetNameFormatter.appendFormat(sb, lastSheetName);
-        }
+
+        SheetRangeAndWorkbookIndexFormatter.format(sb, externalWorkbookNumber, firstSheetName, lastSheetName);
         sb.append('!');
         sb.append(formatReferenceAsString());
         return sb.toString();
diff --git a/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java b/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java
index 67f73b360d..12e7e54eda 100644
--- a/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java
+++ b/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java
@@ -18,7 +18,7 @@
 package org.apache.poi.ss.formula.ptg;
 
 import org.apache.poi.ss.formula.SheetIdentifier;
-import org.apache.poi.ss.formula.SheetNameFormatter;
+import org.apache.poi.ss.formula.SheetRangeAndWorkbookIndexFormatter;
 import org.apache.poi.ss.formula.SheetRangeIdentifier;
 import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.util.LittleEndianOutput;
@@ -101,18 +101,8 @@ public final class Ref3DPxg extends RefPtgBase implements Pxg3D {
 
     public String toFormulaString() {
         StringBuilder sb = new StringBuilder(64);
-        if (externalWorkbookNumber >= 0) {
-            sb.append('[');
-            sb.append(externalWorkbookNumber);
-            sb.append(']');
-        }
-        if (firstSheetName != null) {
-            SheetNameFormatter.appendFormat(sb, firstSheetName);
-        }
-        if (lastSheetName != null) {
-            sb.append(':');
-            SheetNameFormatter.appendFormat(sb, lastSheetName);
-        }
+
+        SheetRangeAndWorkbookIndexFormatter.format(sb, externalWorkbookNumber, firstSheetName, lastSheetName);
         sb.append('!');
         sb.append(formatReferenceAsString());
         return sb.toString();
diff --git a/src/java/org/apache/poi/util/RecordFormatException.java b/src/java/org/apache/poi/util/RecordFormatException.java
index 2bc4ba3bcb..65bea581ee 100644
--- a/src/java/org/apache/poi/util/RecordFormatException.java
+++ b/src/java/org/apache/poi/util/RecordFormatException.java
@@ -45,8 +45,8 @@ public class RecordFormatException
      * be thrown.  If assertTrue is <code>false</code>, this will throw this
      * exception with the message.
      *
-     * @param assertTrue
-     * @param message
+     * @param assertTrue If false, the exception is thrown, if true, no action is performed
+     * @param message The message to include in the thrown exception
      */
     public static void check(boolean assertTrue, String message) {
         if (! assertTrue) {