From 5216731bfda6979378a2da12ae4dd698790720e1 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Fri, 14 Oct 2016 10:11:23 +0000 Subject: [PATCH] bug 56781,60246: fix named range validation to match valid name rules per Excel docs git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1764854 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/usermodel/HSSFName.java | 60 +++++++++++++++---- .../apache/poi/xssf/usermodel/XSSFName.java | 58 ++++++++++++++---- .../poi/ss/usermodel/BaseTestNamedRange.java | 34 +++++++++-- 3 files changed, 123 insertions(+), 29 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java index f4250d52e1..bbd48c23ff 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java @@ -21,9 +21,11 @@ import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.model.InternalWorkbook; import org.apache.poi.hssf.record.NameCommentRecord; import org.apache.poi.hssf.record.NameRecord; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.FormulaType; import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.ss.usermodel.Name; +import org.apache.poi.ss.util.CellReference; /** * High Level Representation of a 'defined name' which could be a 'built-in' name, @@ -155,36 +157,70 @@ public final class HSSFName implements Name { } } + /** + * https://support.office.com/en-us/article/Define-and-use-names-in-formulas-4D0F13AC-53B7-422E-AFD2-ABD7FF379C64#bmsyntax_rules_for_names + * + * Valid characters: + * First character: { letter | underscore | backslash } + * Remaining characters: { letter | number | period | underscore } + * + * Cell shorthand: cannot be { "C" | "c" | "R" | "r" } + * + * Cell references disallowed: cannot be a cell reference $A$1 or R1C1 + * + * Spaces are not valid (follows from valid characters above) + * + * Name length: (XSSF-specific?) 255 characters maximum + * + * Case sensitivity: all names are case-insensitive + * + * Uniqueness: must be unique (for names with the same scope) + * + * @param name + */ private static void validateName(String name) { - /* equivalent to: - Pattern.compile( - "[\\p{IsAlphabetic}_]" + - "[\\p{IsAlphabetic}0-9_\\\\]*", - Pattern.CASE_INSENSITIVE).matcher(name).matches(); - \p{IsAlphabetic} doesn't work on Java 6, and other regex-based character classes don't work on unicode - thus we are stuck with Character.isLetter (for now). - */ - + if (name.length() == 0) { throw new IllegalArgumentException("Name cannot be blank"); } + if (name.length() > 255) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot exceed 255 characters in length"); + } + if (name.equalsIgnoreCase("R") || name.equalsIgnoreCase("C")) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be special shorthand R or C"); + } // is first character valid? char c = name.charAt(0); - String allowedSymbols = "_"; + String allowedSymbols = "_\\"; boolean characterIsValid = (Character.isLetter(c) || allowedSymbols.indexOf(c) != -1); if (!characterIsValid) { throw new IllegalArgumentException("Invalid name: '"+name+"': first character must be underscore or a letter"); } // are all other characters valid? - allowedSymbols = "_\\"; //backslashes needed for unicode escape + allowedSymbols = "_.\\"; //backslashes needed for unicode escape for (final char ch : name.toCharArray()) { characterIsValid = (Character.isLetterOrDigit(ch) || allowedSymbols.indexOf(ch) != -1); if (!characterIsValid) { - throw new IllegalArgumentException("Invalid name: '"+name+"'"); + throw new IllegalArgumentException("Invalid name: '"+name+"': name must be letter, digit, period, or underscore"); } } + + // Is the name a valid $A$1 cell reference + // Because $, :, and ! are disallowed characters, A1-style references become just a letter-number combination + if (name.matches("[A-Za-z]+\\d+")) { + String col = name.replaceAll("\\d", ""); + String row = name.replaceAll("[A-Za-z]", ""); + if (CellReference.cellReferenceIsWithinRange(col, row, SpreadsheetVersion.EXCEL97)) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be $A$1-style cell reference"); + } + } + + // Is the name a valid R1C1 cell reference? + if (name.matches("[Rr]\\d+[Cc]\\d+")) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be R1C1-style cell reference"); + } } public void setRefersToFormula(String formulaText) { diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java index 0710626201..dd6fbccc7c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java @@ -16,12 +16,14 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.ss.formula.FormulaParser; import org.apache.poi.ss.formula.FormulaType; import org.apache.poi.ss.usermodel.Name; import org.apache.poi.ss.util.AreaReference; +import org.apache.poi.ss.util.CellReference; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedName; /** @@ -345,35 +347,69 @@ public final class XSSFName implements Name { return _ctName.toString().equals(cf.getCTName().toString()); } + /** + * https://support.office.com/en-us/article/Define-and-use-names-in-formulas-4D0F13AC-53B7-422E-AFD2-ABD7FF379C64#bmsyntax_rules_for_names + * + * Valid characters: + * First character: { letter | underscore | backslash } + * Remaining characters: { letter | number | period | underscore } + * + * Cell shorthand: cannot be { "C" | "c" | "R" | "r" } + * + * Cell references disallowed: cannot be a cell reference $A$1 or R1C1 + * + * Spaces are not valid (follows from valid characters above) + * + * Name length: (XSSF-specific?) 255 characters maximum + * + * Case sensitivity: all names are case-insensitive + * + * Uniqueness: must be unique (for names with the same scope) + * + * @param name + */ private static void validateName(String name) { - /* equivalent to: - Pattern.compile( - "[\\p{IsAlphabetic}_]" + - "[\\p{IsAlphabetic}0-9_\\\\]*", - Pattern.CASE_INSENSITIVE).matcher(name).matches(); - \p{IsAlphabetic} doesn't work on Java 6, and other regex-based character classes don't work on unicode - thus we are stuck with Character.isLetter (for now). - */ if (name.length() == 0) { throw new IllegalArgumentException("Name cannot be blank"); } + if (name.length() > 255) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot exceed 255 characters in length"); + } + if (name.equalsIgnoreCase("R") || name.equalsIgnoreCase("C")) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be special shorthand R or C"); + } // is first character valid? char c = name.charAt(0); - String allowedSymbols = "_"; + String allowedSymbols = "_\\"; boolean characterIsValid = (Character.isLetter(c) || allowedSymbols.indexOf(c) != -1); if (!characterIsValid) { throw new IllegalArgumentException("Invalid name: '"+name+"': first character must be underscore or a letter"); } // are all other characters valid? - allowedSymbols = "_\\"; //backslashes needed for unicode escape + allowedSymbols = "_.\\"; //backslashes needed for unicode escape for (final char ch : name.toCharArray()) { characterIsValid = (Character.isLetterOrDigit(ch) || allowedSymbols.indexOf(ch) != -1); if (!characterIsValid) { - throw new IllegalArgumentException("Invalid name: '"+name+"'"); + throw new IllegalArgumentException("Invalid name: '"+name+"': name must be letter, digit, period, or underscore"); } } + + // Is the name a valid $A$1 cell reference + // Because $, :, and ! are disallowed characters, A1-style references become just a letter-number combination + if (name.matches("[A-Za-z]+\\d+")) { + String col = name.replaceAll("\\d", ""); + String row = name.replaceAll("[A-Za-z]", ""); + if (CellReference.cellReferenceIsWithinRange(col, row, SpreadsheetVersion.EXCEL97)) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be $A$1-style cell reference"); + } + } + + // Is the name a valid R1C1 cell reference? + if (name.matches("[Rr]\\d+[Cc]\\d+")) { + throw new IllegalArgumentException("Invalid name: '"+name+"': cannot be R1C1-style cell reference"); + } } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java index 92e362df57..1bb73f9f5e 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java @@ -673,22 +673,33 @@ public abstract class BaseTestNamedRange { wb.close(); } + // bug 56781: name validation only checks for first character's validity and presence of spaces + // bug 60246: validate name does not allow DOT in named ranges @Test - public void test56781() throws IOException { + public void testValid() throws IOException { Workbook wb = _testDataProvider.createWorkbook(); Name name = wb.createName(); for (String valid : Arrays.asList( "Hello", "number1", - "_underscore" - //"p.e.r.o.i.d.s", - //"\\Backslash", - //"Backslash\\" + "_underscore", + "underscore_", + "p.e.r.o.i.d.s", + "\\Backslash", + "Backslash\\" )) { name.setNameName(valid); } + wb.close(); + } + + @Test + public void testInvalid() throws IOException { + Workbook wb = _testDataProvider.createWorkbook(); + + Name name = wb.createName(); try { name.setNameName(""); fail("expected exception: (blank)"); @@ -704,7 +715,18 @@ public abstract class BaseTestNamedRange { "Colon:", "A-Minus", "A+Plus", - "Dollar$")) { + "Dollar$", + ".periodAtBeginning", + "R", //special shorthand + "C", //special shorthand + "A1", // A1-style cell reference + "R1C1", // R1C1-style cell reference + "NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters..."+ + "NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters..."+ + "NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters..."+ + "NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters..."+ + "NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters.NameThatIsLongerThan255Characters" + )) { try { name.setNameName(invalid); fail("expected exception: " + invalid); -- 2.39.5