From 5f991cb6168932c7bba5c81bead14d16d69774db Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Thu, 20 Aug 2009 23:25:10 +0000 Subject: [PATCH] minor improvements to sheet name validation and identification of cell references vs defined names git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@806395 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/record/BoundSheetRecord.java | 32 ++-- .../org/apache/poi/ss/util/CellReference.java | 56 ++++--- .../poi/hssf/record/TestBoundSheetRecord.java | 148 +++++++++++------- .../poi/hssf/util/TestCellReference.java | 40 +++-- 4 files changed, 168 insertions(+), 108 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/BoundSheetRecord.java b/src/java/org/apache/poi/hssf/record/BoundSheetRecord.java index 391a3a08ba..b8b6d72ca3 100644 --- a/src/java/org/apache/poi/hssf/record/BoundSheetRecord.java +++ b/src/java/org/apache/poi/hssf/record/BoundSheetRecord.java @@ -54,10 +54,9 @@ public final class BoundSheetRecord extends StandardRecord { /** * UTF8: sid + len + bof + flags + len(str) + unicode + str 2 + 2 + 4 + 2 + * 1 + 1 + len(str) - * + * * UNICODE: sid + len + bof + flags + len(str) + unicode + str 2 + 2 + 4 + 2 + * 1 + 1 + 2 * len(str) - * */ public BoundSheetRecord(RecordInputStream in) { field_1_position_of_BOF = in.readInt(); @@ -75,9 +74,8 @@ public final class BoundSheetRecord extends StandardRecord { /** * set the offset in bytes of the Beginning of File Marker within the HSSF * Stream part of the POIFS file - * - * @param pos - * offset in bytes + * + * @param pos offset in bytes */ public void setPositionOfBof(int pos) { field_1_position_of_BOF = pos; @@ -86,10 +84,10 @@ public final class BoundSheetRecord extends StandardRecord { /** * Set the sheetname for this sheet. (this appears in the tabs at the bottom) * @param sheetName the name of the sheet - * @throws IllegalArgumentException if sheet name will cause excel to crash. + * @throws IllegalArgumentException if sheet name will cause excel to crash. */ public void setSheetname(String sheetName) { - + validateSheetName(sheetName); field_5_sheetname = sheetName; field_4_isMultibyteUnicode = StringUtil.hasMultibyte(sheetName) ? 1 : 0; @@ -117,10 +115,14 @@ public final class BoundSheetRecord extends StandardRecord { // all other chars OK continue; } - throw new IllegalArgumentException("Invalid char (" + ch + throw new IllegalArgumentException("Invalid char (" + ch + ") found at index (" + i + ") in sheet name '" + sheetName + "'"); } - } + if (sheetName.charAt(0) == '\'' || sheetName.charAt(len-1) == '\'') { + throw new IllegalArgumentException("Invalid sheet name '" + sheetName + + "'. Sheet names must not begin or end with (')."); + } + } /** * get the offset in bytes of the Beginning of File Marker within the HSSF Stream part of the POIFS file @@ -154,7 +156,7 @@ public final class BoundSheetRecord extends StandardRecord { buffer.append("[/BOUNDSHEET]\n"); return buffer.toString(); } - + protected int getDataSize() { return 8 + field_5_sheetname.length() * (isMultibyte() ? 2 : 1); } @@ -179,33 +181,33 @@ public final class BoundSheetRecord extends StandardRecord { } /** - * Is the sheet hidden? Different from very hidden + * Is the sheet hidden? Different from very hidden */ public boolean isHidden() { return hiddenFlag.isSet(field_2_option_flags); } /** - * Is the sheet hidden? Different from very hidden + * Is the sheet hidden? Different from very hidden */ public void setHidden(boolean hidden) { field_2_option_flags = hiddenFlag.setBoolean(field_2_option_flags, hidden); } /** - * Is the sheet very hidden? Different from (normal) hidden + * Is the sheet very hidden? Different from (normal) hidden */ public boolean isVeryHidden() { return veryHiddenFlag.isSet(field_2_option_flags); } /** - * Is the sheet very hidden? Different from (normal) hidden + * Is the sheet very hidden? Different from (normal) hidden */ public void setVeryHidden(boolean veryHidden) { field_2_option_flags = veryHiddenFlag.setBoolean(field_2_option_flags, veryHidden); } - + /** * Converts a List of {@link BoundSheetRecord}s to an array and sorts by the position of their * BOFs. diff --git a/src/java/org/apache/poi/ss/util/CellReference.java b/src/java/org/apache/poi/ss/util/CellReference.java index d52e9a8be6..d01cca3540 100644 --- a/src/java/org/apache/poi/ss/util/CellReference.java +++ b/src/java/org/apache/poi/ss/util/CellReference.java @@ -32,11 +32,12 @@ public class CellReference { /** * Used to classify identifiers found in formulas as cell references or not. */ - public static final class NameType { - public static final int CELL = 1; - public static final int NAMED_RANGE = 2; - public static final int COLUMN = 3; - public static final int BAD_CELL_OR_NAMED_RANGE = -1; + public enum NameType { + CELL, + NAMED_RANGE, + COLUMN, + ROW, + BAD_CELL_OR_NAMED_RANGE; } /** The character ($) that signifies a row or column value is absolute instead of relative */ @@ -57,6 +58,11 @@ public class CellReference { * The text may optionally be prefixed with a single '$'. */ private static final Pattern COLUMN_REF_PATTERN = Pattern.compile("\\$?([A-Za-z]+)"); + /** + * Matches a run of one or more digits. The run of digits is group 1. + * The text may optionally be prefixed with a single '$'. + */ + private static final Pattern ROW_REF_PATTERN = Pattern.compile("\\$?([0-9]+)"); /** * Named range names must start with a letter or underscore. Subsequent characters may include * digits or dot. (They can even end in dot). @@ -176,7 +182,7 @@ public class CellReference { * Classifies an identifier as either a simple (2D) cell reference or a named range name * @return one of the values from NameType */ - public static int classifyCellReference(String str, SpreadsheetVersion ssVersion) { + public static NameType classifyCellReference(String str, SpreadsheetVersion ssVersion) { int len = str.length(); if (len < 1) { throw new IllegalArgumentException("Empty string not allowed"); @@ -188,7 +194,7 @@ public class CellReference { case '_': break; default: - if (!Character.isLetter(firstChar)) { + if (!Character.isLetter(firstChar) && !Character.isDigit(firstChar)) { throw new IllegalArgumentException("Invalid first char (" + firstChar + ") of cell reference or named range. Letter expected"); } @@ -219,7 +225,7 @@ public class CellReference { return NameType.NAMED_RANGE; } - private static int validateNamedRangeName(String str, SpreadsheetVersion ssVersion) { + private static NameType validateNamedRangeName(String str, SpreadsheetVersion ssVersion) { Matcher colMatcher = COLUMN_REF_PATTERN.matcher(str); if (colMatcher.matches()) { String colStr = colMatcher.group(1); @@ -227,6 +233,13 @@ public class CellReference { return NameType.COLUMN; } } + Matcher rowMatcher = ROW_REF_PATTERN.matcher(str); + if (rowMatcher.matches()) { + String rowStr = rowMatcher.group(1); + if (isRowWithnRange(rowStr, ssVersion)) { + return NameType.ROW; + } + } if (!NAMED_RANGE_NAME_PATTERN.matcher(str).matches()) { return NameType.BAD_CELL_OR_NAMED_RANGE; } @@ -274,18 +287,7 @@ public class CellReference { if (!isColumnWithnRange(colStr, ssVersion)) { return false; } - - int rowNum = Integer.parseInt(rowStr); - - if (rowNum < 0) { - throw new IllegalStateException("Invalid rowStr '" + rowStr + "'."); - } - if (rowNum == 0) { - // execution gets here because caller does first pass of discriminating - // potential cell references using a simplistic regex pattern. - return false; - } - return rowNum <= ssVersion.getMaxRows(); + return isRowWithnRange(rowStr, ssVersion); } public static boolean isColumnWithnRange(String colStr, SpreadsheetVersion ssVersion) { @@ -308,6 +310,20 @@ public class CellReference { return true; } + public static boolean isRowWithnRange(String rowStr, SpreadsheetVersion ssVersion) { + int rowNum = Integer.parseInt(rowStr); + + if (rowNum < 0) { + throw new IllegalStateException("Invalid rowStr '" + rowStr + "'."); + } + if (rowNum == 0) { + // execution gets here because caller does first pass of discriminating + // potential cell references using a simplistic regex pattern. + return false; + } + return rowNum <= ssVersion.getMaxRows(); + } + /** * Separates the row from the columns and returns an array of three Strings. The first element * is the sheet name. Only the first element may be null. The second element in is the column diff --git a/src/testcases/org/apache/poi/hssf/record/TestBoundSheetRecord.java b/src/testcases/org/apache/poi/hssf/record/TestBoundSheetRecord.java index 766e396473..72d26d04d0 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestBoundSheetRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestBoundSheetRecord.java @@ -18,6 +18,10 @@ package org.apache.poi.hssf.record; import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.apache.poi.util.HexRead; import junit.framework.AssertionFailedError; import junit.framework.TestCase; @@ -32,67 +36,95 @@ import junit.framework.TestCase; public final class TestBoundSheetRecord extends TestCase { - public void testRecordLength() { - BoundSheetRecord record = new BoundSheetRecord("Sheet1"); - assertEquals(18, record.getRecordSize()); - } - - public void testWideRecordLength() { - BoundSheetRecord record = new BoundSheetRecord("Sheet\u20ac"); - assertEquals(24, record.getRecordSize()); - } - - public void testName() { - BoundSheetRecord record = new BoundSheetRecord("1234567890223456789032345678904"); - - try { - record.setSheetname("s//*s"); - throw new AssertionFailedError("Should have thrown IllegalArgumentException, but didnt"); - } catch (IllegalArgumentException e) { - // expected - } - } - - public void testDeserializeUnicode() { - - byte[] data = { -// (byte)0x85, 0x00, // sid -// 0x1a, 0x00, // length - 0x3C, 0x09, 0x00, 0x00, // bof - 0x00, 0x00, // flags - 0x09, // len( str ) - 0x01, // unicode - // - 0x21, 0x04, 0x42, 0x04, 0x40, 0x04, - 0x30, 0x04, 0x3D, 0x04, 0x38, 0x04, - 0x47, 0x04, 0x3A, 0x04, 0x30, 0x04 - // - }; - - RecordInputStream in = TestcaseRecordInputStream.create(BoundSheetRecord.sid, data); + public void testRecordLength() { + BoundSheetRecord record = new BoundSheetRecord("Sheet1"); + assertEquals(18, record.getRecordSize()); + } + + public void testWideRecordLength() { + BoundSheetRecord record = new BoundSheetRecord("Sheet\u20ac"); + assertEquals(24, record.getRecordSize()); + } + + public void testName() { + BoundSheetRecord record = new BoundSheetRecord("1234567890223456789032345678904"); + + try { + record.setSheetname("s//*s"); + throw new AssertionFailedError("Should have thrown IllegalArgumentException, but didnt"); + } catch (IllegalArgumentException e) { + // expected + } + } + + public void testDeserializeUnicode() { + + byte[] data = HexRead.readFromString("" + + "85 00 1A 00" // sid, length + + "3C 09 00 00" // bof + + "00 00"// flags + + "09 01" // str-len. unicode flag + // string data + + "21 04 42 04 40 04" + + "30 04 3D 04 38 04" + + "47 04 3A 04 30 04" + ); + + RecordInputStream in = TestcaseRecordInputStream.create(data); BoundSheetRecord bsr = new BoundSheetRecord(in); // sheet name is unicode Russian for 'minor page' assertEquals("\u0421\u0442\u0440\u0430\u043D\u0438\u0447\u043A\u0430", bsr.getSheetname()); - + + byte[] data2 = bsr.serialize(); + assertTrue(Arrays.equals(data, data2)); + } + + public void testOrdering() { + BoundSheetRecord bs1 = new BoundSheetRecord("SheetB"); + BoundSheetRecord bs2 = new BoundSheetRecord("SheetC"); + BoundSheetRecord bs3 = new BoundSheetRecord("SheetA"); + bs1.setPositionOfBof(11); + bs2.setPositionOfBof(33); + bs3.setPositionOfBof(22); + + List l = new ArrayList(); + l.add(bs1); + l.add(bs2); + l.add(bs3); + + BoundSheetRecord[] r = BoundSheetRecord.orderByBofPosition(l); + assertEquals(3, r.length); + assertEquals(bs1, r[0]); + assertEquals(bs3, r[1]); + assertEquals(bs2, r[2]); } - public void testOrdering() { - BoundSheetRecord bs1 = new BoundSheetRecord("SheetB"); - BoundSheetRecord bs2 = new BoundSheetRecord("SheetC"); - BoundSheetRecord bs3 = new BoundSheetRecord("SheetA"); - bs1.setPositionOfBof(11); - bs2.setPositionOfBof(33); - bs3.setPositionOfBof(22); - - ArrayList l = new ArrayList(); - l.add(bs1); - l.add(bs2); - l.add(bs3); - - BoundSheetRecord[] r = BoundSheetRecord.orderByBofPosition(l); - assertEquals(3, r.length); - assertEquals(bs1, r[0]); - assertEquals(bs3, r[1]); - assertEquals(bs2, r[2]); - } + public void testValidNames() { + confirmValid("Sheet1", true); + confirmValid("O'Brien's sales", true); + confirmValid(" data # ", true); + confirmValid("data $1.00", true); + + confirmValid("data?", false); + confirmValid("abc/def", false); + confirmValid("data[0]", false); + confirmValid("data*", false); + confirmValid("abc\\def", false); + confirmValid("'data", false); + confirmValid("data'", false); + } + + private static void confirmValid(String sheetName, boolean expectedResult) { + + try { + new BoundSheetRecord(sheetName); + if (!expectedResult) { + throw new AssertionFailedError("Expected sheet name '" + sheetName + "' to be invalid"); + } + } catch (IllegalArgumentException e) { + if (expectedResult) { + throw new AssertionFailedError("Expected sheet name '" + sheetName + "' to be valid"); + } + } + } } diff --git a/src/testcases/org/apache/poi/hssf/util/TestCellReference.java b/src/testcases/org/apache/poi/hssf/util/TestCellReference.java index acd3f00e79..523a86bf6b 100644 --- a/src/testcases/org/apache/poi/hssf/util/TestCellReference.java +++ b/src/testcases/org/apache/poi/hssf/util/TestCellReference.java @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - + package org.apache.poi.hssf.util; @@ -26,12 +26,12 @@ import org.apache.poi.ss.SpreadsheetVersion; public final class TestCellReference extends TestCase { - + public void testAbsRef1(){ CellReference cf = new CellReference("$B$5"); confirmCell(cf, null, 4, 1, true, true, "$B$5"); } - + public void testAbsRef2(){ CellReference cf = new CellReference(4,1,true,true); confirmCell(cf, null, 4, 1, true, true, "$B$5"); @@ -41,17 +41,17 @@ public final class TestCellReference extends TestCase { CellReference cf = new CellReference("B$5"); confirmCell(cf, null, 4, 1, true, false, "B$5"); } - + public void testAbsRef4(){ CellReference cf = new CellReference(4,1,true,false); confirmCell(cf, null, 4, 1, true, false, "B$5"); } - + public void testAbsRef5(){ CellReference cf = new CellReference("$B5"); confirmCell(cf, null, 4, 1, false, true, "$B5"); } - + public void testAbsRef6(){ CellReference cf = new CellReference(4,1,false,true); confirmCell(cf, null, 4, 1, false, true, "$B5"); @@ -61,27 +61,27 @@ public final class TestCellReference extends TestCase { CellReference cf = new CellReference("B5"); confirmCell(cf, null, 4, 1, false, false, "B5"); } - + public void testAbsRef8(){ CellReference cf = new CellReference(4,1,false,false); confirmCell(cf, null, 4, 1, false, false, "B5"); } - + public void testSpecialSheetNames() { CellReference cf; cf = new CellReference("'profit + loss'!A1"); confirmCell(cf, "profit + loss", 0, 0, false, false, "'profit + loss'!A1"); - + cf = new CellReference("'O''Brien''s Sales'!A1"); confirmCell(cf, "O'Brien's Sales", 0, 0, false, false, "'O''Brien''s Sales'!A1"); - + cf = new CellReference("'Amazing!'!A1"); confirmCell(cf, "Amazing!", 0, 0, false, false, "'Amazing!'!A1"); } - /* package */ static void confirmCell(CellReference cf, String expSheetName, int expRow, + /* package */ static void confirmCell(CellReference cf, String expSheetName, int expRow, int expCol, boolean expIsRowAbs, boolean expIsColAbs, String expText) { - + assertEquals(expSheetName, cf.getSheetName()); assertEquals("row index is wrong", expRow, cf.getRow()); assertEquals("col index is wrong", expCol, cf.getCol()); @@ -103,9 +103,19 @@ public final class TestCellReference extends TestCase { confirmNameType("A.1", NameType.NAMED_RANGE); confirmNameType("A1.", NameType.NAMED_RANGE); } - - private void confirmNameType(String ref, int expectedResult) { - int actualResult = CellReference.classifyCellReference(ref, SpreadsheetVersion.EXCEL97); + + public void testClassificationOfRowReferences(){ + confirmNameType("10", NameType.ROW); + confirmNameType("$10", NameType.ROW); + confirmNameType("65536", NameType.ROW); + + confirmNameType("65537", NameType.BAD_CELL_OR_NAMED_RANGE); + confirmNameType("$100000", NameType.BAD_CELL_OR_NAMED_RANGE); + confirmNameType("$1$1", NameType.BAD_CELL_OR_NAMED_RANGE); + } + + private void confirmNameType(String ref, NameType expectedResult) { + NameType actualResult = CellReference.classifyCellReference(ref, SpreadsheetVersion.EXCEL97); assertEquals(expectedResult, actualResult); } } -- 2.39.5