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

src/java/org/apache/poi/hssf/record/BoundSheetRecord.java

@@ -57,7 +57,6 @@ public final class BoundSheetRecord extends StandardRecord {
 	 *
 	 * 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();
@@ -76,8 +75,7 @@ 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
+	 * @param pos offset in bytes
-	 *			offset in bytes
 	 */
 	public void setPositionOfBof(int pos) {
 		field_1_position_of_BOF = pos;
@@ -120,6 +118,10 @@ public final class BoundSheetRecord extends StandardRecord {
 			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 (').");
+		}
 	}
 
 	/**

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 enum NameType {
-		public static final int CELL = 1;
+		CELL,
-		public static final int NAMED_RANGE = 2;
+		NAMED_RANGE,
-		public static final int COLUMN = 3;
+		COLUMN,
-		public static final int BAD_CELL_OR_NAMED_RANGE = -1;
+		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 <tt>NameType</tt>
 	 */
-	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;
 		}
-
+		return isRowWithnRange(rowStr, 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();
 	}
 
 	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

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;
@@ -55,25 +59,24 @@ public final class TestBoundSheetRecord extends TestCase {
 
 	public void testDeserializeUnicode() {
 
-		byte[] data = {
+		byte[] data = HexRead.readFromString(""
-//			(byte)0x85, 0x00,       // sid
+			+ "85 00 1A 00" // sid, length
-//			0x1a, 0x00, 			// length
+			+ "3C 09 00 00" // bof
-			0x3C, 0x09, 0x00, 0x00, // bof
+			+ "00 00"// flags
-			0x00, 0x00, 			// flags
+			+ "09 01" // str-len. unicode flag
-			0x09, 					// len( str )
+			// string data
-			0x01, 					// unicode
+			+ "21 04 42 04 40 04"
-			// <str>
+			+ "30 04 3D 04 38 04"
-			0x21, 0x04, 0x42, 0x04, 0x40, 0x04, 
+			+ "47 04 3A 04 30 04"
-			0x30, 0x04, 0x3D, 0x04, 0x38, 0x04,
+		);
-			0x47, 0x04, 0x3A, 0x04, 0x30, 0x04
-			// </str>
-		};
 
-		RecordInputStream in = TestcaseRecordInputStream.create(BoundSheetRecord.sid, data);
+		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() {
@@ -84,7 +87,7 @@ public final class TestBoundSheetRecord extends TestCase {
 		bs2.setPositionOfBof(33);
 		bs3.setPositionOfBof(22);
 
-    	ArrayList l = new ArrayList();
+		List<BoundSheetRecord> l = new ArrayList<BoundSheetRecord>();
 		l.add(bs1);
 		l.add(bs2);
 		l.add(bs3);
@@ -95,4 +98,33 @@ public final class TestBoundSheetRecord extends TestCase {
 		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");
+			}
+		}
+	}
 }

src/testcases/org/apache/poi/hssf/util/TestCellReference.java

@@ -104,8 +104,18 @@ public final class TestCellReference extends TestCase {
         confirmNameType("A1.", NameType.NAMED_RANGE);
     }
 
-    private void confirmNameType(String ref, int expectedResult) {
+    public void testClassificationOfRowReferences(){
-        int actualResult = CellReference.classifyCellReference(ref, SpreadsheetVersion.EXCEL97);
+        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);
     }
 }