From: Dominik Stadler Date: Sun, 30 Dec 2018 22:44:40 +0000 (+0000) Subject: Bug 60460: Handle null workbook or sheet names and emit #REF as Excel does X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=05246549ddf1f39c30f2f9d54198ab2dcde06a3f;p=poi.git 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 --- diff --git a/src/integrationtest/org/apache/poi/stress/SpreadsheetHandler.java b/src/integrationtest/org/apache/poi/stress/SpreadsheetHandler.java index 05147ff6d0..38f633b4cd 100644 --- a/src/integrationtest/org/apache/poi/stress/SpreadsheetHandler.java +++ b/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 { diff --git a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java b/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java index 6e10e67c58..37daee5165 100644 --- a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java +++ b/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(); } - - /** - * 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 appendFormat(StringBuilder out, String rawSheetName) 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 appendFormat(StringBuilder out, String workbookName, String rawSheetName) instead + * @deprecated Only kept for binary compatibility, will be replaced by the version with Appendable as parameter */ @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); + @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 StringBuilder is already available. + * 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 + * @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(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); } } - 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); + + private static void appendOrREF(Appendable out, String name) throws IOException { + if(name == null) { + out.append("#REF"); } else { - out.append('['); - out.append(workbookName); - out.append(']'); - out.append(rawSheetName); + out.append(name); } } - static void appendAndEscape(Appendable sb, String rawSheetName) { - int len = rawSheetName.length(); - for(int i=0; itrue 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 diff --git a/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java b/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java index 24d39cf274..3ee1d77df5 100644 --- a/src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java +++ b/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; } } diff --git a/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java b/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java index c1989e17db..2717662d2b 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java +++ b/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); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index fda515f7ce..333704fd82 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/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(); + } } diff --git a/src/testcases/org/apache/poi/ss/formula/TestSheetNameFormatter.java b/src/testcases/org/apache/poi/ss/formula/TestSheetNameFormatter.java index 2125810923..76a4fc7ac2 100644 --- a/src/testcases/org/apache/poi/ss/formula/TestSheetNameFormatter.java +++ b/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("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); diff --git a/test-data/spreadsheet/60460.xls b/test-data/spreadsheet/60460.xls new file mode 100644 index 0000000000..73fad8aa84 Binary files /dev/null and b/test-data/spreadsheet/60460.xls differ