Bug 60460: Handle null workbook or sheet names and emit #REF as Excel does

instead of throwing NullPointerException

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

src/integrationtest/org/apache/poi/stress/SpreadsheetHandler.java

@@ -22,10 +22,10 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 
-import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.ss.extractor.EmbeddedData;
 import org.apache.poi.ss.extractor.EmbeddedExtractor;
 import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Name;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.Workbook;
@@ -91,6 +91,13 @@ public abstract class SpreadsheetHandler extends AbstractFileHandler {
 			    }
 			}
 		}
+
+		for (Name name : wb.getAllNames()) {
+			// this sometimes caused exceptions
+            if(!name.isFunctionName()) {
+                name.getRefersToFormula();
+            }
+		}
 	}
 
 	private void extractEmbedded(Workbook wb) throws IOException {

src/java/org/apache/poi/ss/formula/SheetNameFormatter.java

@@ -17,11 +17,13 @@
 
 package org.apache.poi.ss.formula;
 
+import java.io.IOException;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.ss.SpreadsheetVersion;
+import org.apache.poi.util.Removal;
 
 /**
  * Formats sheet names for use in formula expressions.
@@ -47,106 +49,134 @@ public final class SheetNameFormatter {
 	 * sheet name will be converted to double single quotes ('').  
 	 */
 	public static String format(String rawSheetName) {
-        StringBuilder sb = new StringBuilder(rawSheetName.length() + 2);
+        StringBuilder sb = new StringBuilder((rawSheetName == null ? 0 : rawSheetName.length()) + 2);
 		appendFormat(sb, rawSheetName);
 		return sb.toString();
 	}
 
+    /**
+     * @deprecated Only kept for binary compatibility, will be replaced by the version with Appendable as parameter
+     */
+    @Deprecated
+    @Removal(version="5.0.0")
+    public static void appendFormat(StringBuffer out, String rawSheetName) {
+        appendFormat((Appendable)out, rawSheetName);
+    }
+
+    /**
+     * @deprecated Only kept for binary compatibility, will be replaced by the version with Appendable as parameter
+     */
+    @Deprecated
+    @Removal(version="5.0.0")
+    public static void appendFormat(StringBuffer out, String workbookName, String rawSheetName) {
+        appendFormat((Appendable)out, workbookName, rawSheetName);
+    }
+
+    /**
+     * Only kept for binary compatibility, will be replaced by the version with Appendable as parameter
+     */
+    @Removal(version="5.0.0")
+    public static void appendFormat(StringBuilder out, String rawSheetName) {
+        appendFormat((Appendable)out, rawSheetName);
+    }
+
+    /**
+     * Only kept for binary compatibility, will be replaced by the version with Appendable as parameter
+     */
+    @Removal(version="5.0.0")
+    public static void appendFormat(StringBuilder out, String workbookName, String rawSheetName) {
+        appendFormat((Appendable)out, workbookName, rawSheetName);
+    }
+
+    /**
+     * Convenience method for ({@link #format(String)}) when a StringBuffer is already available.
+     *
+     * @param out - sheet name will be appended here possibly with delimiting quotes
+     * @param rawSheetName - sheet name
+     */
+	public static void appendFormat(Appendable out, String rawSheetName) {
+		try {
+			boolean needsQuotes = needsDelimiting(rawSheetName);
+			if(needsQuotes) {
+				out.append(DELIMITER);
+				appendAndEscape(out, rawSheetName);
+				out.append(DELIMITER);
+			} else {
+				appendAndEscape(out, rawSheetName);
+			}
+		} catch (Exception e) {
+			throw new RuntimeException(e);
+		}
+	}
+
 	/**
 	 * Convenience method for ({@link #format(String)}) when a StringBuffer is already available.
 	 *
-     * @param out - sheet name will be appended here possibly with delimiting quotes
-     * @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) {
-			out.append(DELIMITER);
-			appendAndEscape(out, rawSheetName);
-			out.append(DELIMITER);
-		} else {
-			out.append(rawSheetName);
-		}
-	}
-
-    /**
-     * @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) {
-			out.append(DELIMITER);
-			out.append('[');
-			appendAndEscape(out, workbookName.replace('[', '(').replace(']', ')'));
-			out.append(']');
-			appendAndEscape(out, rawSheetName);
-			out.append(DELIMITER);
-		} else {
-			out.append('[');
-			out.append(workbookName);
-			out.append(']');
-			out.append(rawSheetName);
-		}
-	}
-
-	/**
-	 * Convenience method for ({@link #format(String)}) when a StringBuilder is already available.
-	 *
 	 * @param out - sheet name will be appended here possibly with delimiting quotes
-     * @param rawSheetName - sheet name
+	 * @param workbookName - workbook name
+	 * @param rawSheetName - sheet name
 	 */
-	public static void appendFormat(StringBuilder out, String rawSheetName) {
-		boolean needsQuotes = needsDelimiting(rawSheetName);
-		if(needsQuotes) {
-			out.append(DELIMITER);
-			appendAndEscape(out, rawSheetName);
-			out.append(DELIMITER);
-		} else {
-			out.append(rawSheetName);
-		}
-	}
-	public static void appendFormat(StringBuilder out, String workbookName, String rawSheetName) {
-		boolean needsQuotes = needsDelimiting(workbookName) || needsDelimiting(rawSheetName);
-		if(needsQuotes) {
-			out.append(DELIMITER);
-			out.append('[');
-			appendAndEscape(out, workbookName.replace('[', '(').replace(']', ')'));
-			out.append(']');
-			appendAndEscape(out, rawSheetName);
-			out.append(DELIMITER);
-		} else {
-			out.append('[');
-			out.append(workbookName);
-			out.append(']');
-			out.append(rawSheetName);
+	public static void appendFormat(Appendable out, String workbookName, String rawSheetName) {
+		try {
+			boolean needsQuotes = needsDelimiting(workbookName) || needsDelimiting(rawSheetName);
+			if(needsQuotes) {
+				out.append(DELIMITER);
+				out.append('[');
+				appendAndEscape(out, workbookName.replace('[', '(').replace(']', ')'));
+				out.append(']');
+				appendAndEscape(out, rawSheetName);
+				out.append(DELIMITER);
+			} else {
+				out.append('[');
+				appendOrREF(out, workbookName);
+				out.append(']');
+				appendOrREF(out, rawSheetName);
+			}
+		} catch (Exception e) {
+			throw new RuntimeException(e);
 		}
 	}
 
-    static void appendAndEscape(Appendable sb, String rawSheetName) {
-        int len = rawSheetName.length();
-        for(int i=0; i<len; i++) {
-            char ch = rawSheetName.charAt(i);
-            try {
-                if(ch == DELIMITER) {
-                    // single quotes (') are encoded as ('')
-                    sb.append(DELIMITER);
-                }
-                sb.append(ch);
-            } catch (Exception e) {
-                throw new RuntimeException(e);
-            }
-        }
+	private static void appendOrREF(Appendable out, String name) throws IOException {
+		if(name == null) {
+			out.append("#REF");
+		} else {
+			out.append(name);
+		}
+	}
+
+	static void appendAndEscape(Appendable sb, String rawSheetName) {
+		try {
+			if (rawSheetName == null) {
+				sb.append("#REF");
+				return;
+			}
+
+			int len = rawSheetName.length();
+			for (int i = 0; i < len; i++) {
+				char ch = rawSheetName.charAt(i);
+				if (ch == DELIMITER) {
+					// single quotes (') are encoded as ('')
+					sb.append(DELIMITER);
+				}
+				sb.append(ch);
+			}
+		} catch (Exception e) {
+			throw new RuntimeException(e);
+		}
     }
 
 	/**
 	 * 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.
+	 * @return true if the given raw sheet name needs screening/delimiting, false otherwise or
+	 * 			if the sheet name is null.
 	 */
 	static boolean needsDelimiting(String rawSheetName) {
+		if(rawSheetName == null) {
+			return false;
+		}
+
 		int len = rawSheetName.length();
 		if(len < 1) {
 			throw new RuntimeException("Zero length string is an invalid sheet name");
@@ -185,6 +215,7 @@ public final class SheetNameFormatter {
 		}
 		return false;
 	}
+
 	/**
 	 * @return <code>true</code> if the presence of the specified character in a sheet name would 
 	 * require the sheet name to be delimited in formulas.  This includes every non-alphanumeric 

src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java

@@ -66,8 +66,8 @@ public class SheetRangeAndWorkbookIndexFormatter {
     }
 
     private static boolean anySheetNameNeedsEscaping(String firstSheetName, String lastSheetName) {
-        boolean anySheetNameNeedsDelimiting = firstSheetName != null && SheetNameFormatter.needsDelimiting(firstSheetName);
-        anySheetNameNeedsDelimiting |= lastSheetName != null && SheetNameFormatter.needsDelimiting(lastSheetName);
+        boolean anySheetNameNeedsDelimiting = SheetNameFormatter.needsDelimiting(firstSheetName);
+        anySheetNameNeedsDelimiting |= SheetNameFormatter.needsDelimiting(lastSheetName);
         return anySheetNameNeedsDelimiting;
     }
 }

src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java

@@ -37,7 +37,7 @@ final class ExternSheetNameResolver {
             String wbName = externalSheet.getWorkbookName();
             String sheetName = externalSheet.getSheetName();
             if (wbName != null) {
-                sb = new StringBuilder(wbName.length() + sheetName.length() + cellRefText.length() + 4);
+                sb = new StringBuilder(wbName.length() + (sheetName == null ? 0 : sheetName.length()) + cellRefText.length() + 4);
                 SheetNameFormatter.appendFormat(sb, wbName, sheetName);
             } else {
                 sb = new StringBuilder(sheetName.length() + cellRefText.length() + 4);

src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java

@@ -3135,4 +3135,21 @@ public final class TestBugs extends BaseTestBugzillaIssues {
             assertEquals("\uFF2D\uFF33 \uFF30\u30B4\u30B7\u30C3\u30AF", font.getFontName());
         }
     }
+
+    @Test
+    public void test60460() throws IOException {
+        final Workbook wb = HSSFTestDataSamples.openSampleWorkbook("60460.xls");
+
+        assertEquals(2, wb.getAllNames().size());
+
+        Name rangedName = wb.getAllNames().get(0);
+        assertFalse(rangedName.isFunctionName());
+        assertEquals("'[\\\\HEPPC3/gt$/Teaching/Syn/physyn.xls]#REF'!$AK$70:$AL$70", rangedName.getRefersToFormula());
+
+        rangedName = wb.getAllNames().get(1);
+        assertFalse(rangedName.isFunctionName());
+        assertEquals("Questionnaire!$A$1:$L$65", rangedName.getRefersToFormula());
+
+        wb.close();
+    }
 }

src/testcases/org/apache/poi/ss/formula/TestSheetNameFormatter.java

@@ -17,22 +17,23 @@
 
 package org.apache.poi.ss.formula;
 
-import junit.framework.TestCase;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 /**
  * Tests for {@link SheetNameFormatter}
  * 
  * @author Josh Micich
  */
-public final class TestSheetNameFormatter extends TestCase {
-
-	private static void confirmFormat(String rawSheetName, String expectedSheetNameEncoding) {
-		assertEquals(expectedSheetNameEncoding, SheetNameFormatter.format(rawSheetName));
-	}
-
+public final class TestSheetNameFormatter {
 	/**
 	 * Tests main public method 'format' 
 	 */
+	@Test
 	public void testFormat() {
 		
 		confirmFormat("abc", "abc");
@@ -43,14 +44,108 @@ public final class TestSheetNameFormatter extends TestCase {
 
 		confirmFormat("O'Brian", "'O''Brian'"); // single quote gets doubled
 		
-		
 		confirmFormat("3rdTimeLucky", "'3rdTimeLucky'"); // digit in first pos
 		confirmFormat("_", "_"); // plain underscore OK
 		confirmFormat("my_3rd_sheet", "my_3rd_sheet"); // underscores and digits OK
 		confirmFormat("A12220", "'A12220'"); 
 		confirmFormat("TAXRETURN19980415", "TAXRETURN19980415");
+
+		confirmFormat(null, "#REF");
 	}
 
+	private static void confirmFormat(String rawSheetName, String expectedSheetNameEncoding) {
+		// test all variants
+
+		assertEquals(expectedSheetNameEncoding, SheetNameFormatter.format(rawSheetName));
+
+		StringBuilder sb = new StringBuilder();
+		SheetNameFormatter.appendFormat(sb, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sb.toString());
+
+		sb = new StringBuilder();
+		SheetNameFormatter.appendFormat((Appendable)sb, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sb.toString());
+
+		StringBuffer sbf = new StringBuffer();
+		//noinspection deprecation
+		SheetNameFormatter.appendFormat(sbf, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sbf.toString());
+	}
+
+	@Test
+	public void testFormatWithWorkbookName() {
+
+		confirmFormat("abc", "abc", "[abc]abc");
+		confirmFormat("abc", "123", "'[abc]123'");
+
+		confirmFormat("abc", "my sheet", "'[abc]my sheet'"); // space
+		confirmFormat("abc", "A:MEM", "'[abc]A:MEM'"); // colon
+
+		confirmFormat("abc", "O'Brian", "'[abc]O''Brian'"); // single quote gets doubled
+
+		confirmFormat("abc", "3rdTimeLucky", "'[abc]3rdTimeLucky'"); // digit in first pos
+		confirmFormat("abc", "_", "[abc]_"); // plain underscore OK
+		confirmFormat("abc", "my_3rd_sheet", "[abc]my_3rd_sheet"); // underscores and digits OK
+		confirmFormat("abc", "A12220", "'[abc]A12220'");
+		confirmFormat("abc", "TAXRETURN19980415", "[abc]TAXRETURN19980415");
+
+		confirmFormat("abc", null, "[abc]#REF");
+		confirmFormat(null, "abc", "[#REF]abc");
+		confirmFormat(null, null, "[#REF]#REF");
+	}
+
+	private static void confirmFormat(String workbookName, String rawSheetName, String expectedSheetNameEncoding) {
+		// test all variants
+
+		StringBuilder sb = new StringBuilder();
+		SheetNameFormatter.appendFormat(sb, workbookName, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sb.toString());
+
+		sb = new StringBuilder();
+		SheetNameFormatter.appendFormat((Appendable)sb, workbookName, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sb.toString());
+
+		StringBuffer sbf = new StringBuffer();
+		//noinspection deprecation
+		SheetNameFormatter.appendFormat(sbf, workbookName, rawSheetName);
+		assertEquals(expectedSheetNameEncoding, sbf.toString());
+	}
+
+	@Test
+	public void testFormatException() {
+		Appendable mock = new Appendable() {
+			@Override
+			public Appendable append(CharSequence csq) throws IOException {
+				throw new IOException("Test exception");
+			}
+
+			@Override
+			public Appendable append(CharSequence csq, int start, int end) throws IOException {
+				throw new IOException("Test exception");
+			}
+
+			@Override
+			public Appendable append(char c) throws IOException {
+				throw new IOException("Test exception");
+			}
+		};
+
+		try {
+			SheetNameFormatter.appendFormat(mock, null, null);
+			fail("Should catch exception here");
+		} catch (RuntimeException e) {
+			// expected here
+		}
+
+		try {
+			SheetNameFormatter.appendFormat(mock, null);
+			fail("Should catch exception here");
+		} catch (RuntimeException e) {
+			// expected here
+		}
+	}
+
+	@Test
 	public void testBooleanLiterals() {
 		confirmFormat("TRUE", "'TRUE'");
 		confirmFormat("FALSE", "'FALSE'");
@@ -69,6 +164,7 @@ public final class TestSheetNameFormatter extends TestCase {
 	 * Tests functionality to determine whether a sheet name containing only letters and digits
 	 * would look (to Excel) like a cell name.
 	 */
+	@Test
 	public void testLooksLikePlainCellReference() {
 		
 		confirmCellNameMatch("A1", true);
@@ -91,6 +187,7 @@ public final class TestSheetNameFormatter extends TestCase {
 	 * Tests exact boundaries for names that look very close to cell names (i.e. contain 1 or more
 	 * letters followed by one or more digits).
 	 */
+	@Test
 	public void testCellRange() {
 		confirmCellRange("A1", 1, true);
 		confirmCellRange("a111", 1, true);