fpspreadsheet: Validity check for sheet names (test case included)

git-svn-id: https://svn.code.sf.net/p/lazarus-ccr/svn@3552 8e941d3f-bd1b-0410-a28a-d453659cc2b4

components/fpspreadsheet/fpspreadsheet.pas

@@ -901,12 +901,13 @@ type
     procedure WriteToStream(AStream: TStream; AFormat: TsSpreadsheetFormat);
 
     { Worksheet list handling methods }
-    function  AddWorksheet(AName: string): TsWorksheet;
+    function  AddWorksheet(AName: string; AcceptEmptyName: boolean = false): TsWorksheet;
     function  GetFirstWorksheet: TsWorksheet;
     function  GetWorksheetByIndex(AIndex: Cardinal): TsWorksheet;
     function  GetWorksheetByName(AName: String): TsWorksheet;
     function  GetWorksheetCount: Cardinal;
     procedure RemoveAllWorksheets;
+    function  ValidWorksheetName(AName: String; AcceptEmptyName: Boolean = false): Boolean;
 
     { Font handling }
     function AddFont(const AFontName: String; ASize: Single;
@@ -1237,7 +1238,7 @@ function HasFormula(ACell: PCell): Boolean;
 implementation
 
 uses
-  Math, StrUtils, TypInfo,
+  Math, StrUtils, TypInfo, lazutf8,
   fpsStreams, fpsUtils, fpsNumFormatParser, fpsExprParser;
 
 { Translatable strings }
@@ -1245,6 +1246,7 @@ resourcestring
   lpUnsupportedReadFormat = 'Tried to read a spreadsheet using an unsupported format';
   lpUnsupportedWriteFormat = 'Tried to write a spreadsheet using an unsupported format';
   lpNoValidSpreadsheetFile = '"%s" is not a valid spreadsheet file';
+  lpInvalidWorksheetName = '"%s" is not a valid worksheet name.';
   lpUnknownSpreadsheetFormat = 'unknown format';
   lpMaxRowsExceeded = 'This workbook contains %d rows, but the selected ' +
     'file format does not support more than %d rows.';
@@ -3123,13 +3125,20 @@ begin
   n := 0;
   SetLength(AList, n);
   for r := 0 to GetLastOccupiedRowIndex do
-    for c := 0 to GetLastOccupiedColIndex do
+  begin
+    c := 0;
+    while (c <= GetLastOccupiedColIndex) do
     begin
       cell := FindCell(r, c);
-      if IsMergeBase(cell) then begin
+      if IsMergeBase(cell) then
+      begin
         FindMergedRange(cell, rng.Row1, rng.Col1, rng.Row2, rng.Col2);
         SetLength(AList, n+1);
         AList[n] := rng;
+        inc(n);
+        c := rng.Col2;   // jump to next cell not belonging to this block
+      end;
+      inc(c);
     end;
   end;
 end;
@@ -5684,11 +5693,16 @@ end;
   It is added to the end of the list of worksheets
 
   @param  AName            The name of the new worksheet
+  @param  AcceptEmptyName  Allow an empty worksheet name (for Excel2)
   @return The instance of the newly created worksheet
   @see    TsWorksheet
 }
-function TsWorkbook.AddWorksheet(AName: string): TsWorksheet;
+function TsWorkbook.AddWorksheet(AName: string;
+  AcceptEmptyName: Boolean = false): TsWorksheet;
 begin
+  if not ValidWorksheetName(AName, AcceptEmptyName) then
+    raise Exception.CreateFmt(lpInvalidWorksheetName, [AName]);
+
   Result := TsWorksheet.Create;
 
   Result.Name := AName;
@@ -5779,6 +5793,46 @@ procedure TsWorkbook.RemoveAllWorksheets;
 begin
   FWorksheets.ForEachCall(RemoveWorksheetsCallback, nil);
 end;
+
+{@@
+  Checks whether the passed string is a valid worksheet name according to Excel
+  (ODS seems to be a bit less restrictive, but if we follow Excel's convention
+  we always have valid sheet names independent of the format.
+
+  @param   AName             Name to be checked
+  @param   AcceptEmptyName   Accepts an empty name (for Excel2)
+  @return  TRUE if it is a valid worksheet name, FALSE otherwise
+}
+function TsWorkbook.ValidWorksheetName(AName: String;
+  AcceptEmptyName: Boolean = false): Boolean;
+// see: http://stackoverflow.com/questions/451452/valid-characters-for-excel-sheet-names
+var
+  INVALID_CHARS: array [0..6] of char = ('[', ']', ':', '*', '?', '/', '\');
+var
+  i: Integer;
+begin
+  Result := false;
+
+  // Name must not be empty
+  if (AName = '') and (not AcceptEmptyName) then
+    exit;
+
+  // Length must be less than 31 characters
+  if UTF8Length(AName) > 31 then
+    exit;
+
+  // Name must not contain any of the INVALID_CHARS
+  for i:=0 to High(INVALID_CHARS) do
+    if UTF8Pos(INVALID_CHARS[i], AName) > 0 then
+      exit;
+
+  // Name must be unique
+  if GetWorksheetByName(AName) <> nil then
+    exit;
+
+  Result := true;
+end;
+
                             (*
 {@@
   Sets the selected flag for the sheet with the given index.

components/fpspreadsheet/tests/internaltests.pas

@@ -42,6 +42,8 @@ type
     // Verify GetSheetByName returns the correct sheet number
     // GetSheetByName was implemented in SVN revision 2857
     procedure GetSheetByName;
+    // Test for invalid sheet names
+    procedure InvalidSheetName;
     // Tests whether overwriting existing file works
     procedure OverwriteExistingFile;
     // Write out date cell and try to read as UTF8; verify if contents the same
@@ -92,6 +94,41 @@ begin
   end;
 end;
 
+procedure TSpreadInternalTests.InvalidSheetName;
+type
+  TSheetNameCheck = record
+    Valid: Boolean;
+    SheetName: String;
+  end;
+const
+  TestCases: array[0..7] of TSheetNameCheck = (
+    (Valid: true;  SheetName:'Sheet'),
+    (Valid: true;  SheetName:'äöü'),
+    (Valid: false; SheetName:'Test'),   // duplicate
+    (Valid: false; SheetName:''),       // empty string
+    (Valid: false; SheetName:'Very very very very very very very very long'),  // too long
+    (Valid: false; SheetName:'[sheet]'), // fobidden chars in following cases
+    (Valid: false; SheetName:'/sheet/'),
+    (Valid: false; SheetName:'\sheet\')
+  );
+var
+  i: Integer;
+  MyWorkbook: TsWorkbook;
+  ok: Boolean;
+begin
+  MyWorkbook := TsWorkbook.Create;
+  try
+    MyWorkbook.AddWorksheet('Test');
+    for i:=0 to High(TestCases) do
+    begin
+      ok := MyWorkbook.ValidWorksheetName(TestCases[i].SheetName);
+      CheckEquals(TestCases[i].Valid, ok, 'Sheet name validity check mismatch: ' + TestCases[i].SheetName);
+    end;
+  finally
+    MyWorkbook.Free;
+  end;
+end;
+
 procedure TSpreadInternalTests.OverwriteExistingFile;
 const
   FirstFileCellText='Old version';

components/fpspreadsheet/tests/spreadtestgui.lpi

@@ -48,6 +48,7 @@
       <Unit1>
         <Filename Value="datetests.pas"/>
         <IsPartOfProject Value="True"/>
+        <UnitName Value="datetests"/>
       </Unit1>
       <Unit2>
         <Filename Value="stringtests.pas"/>
@@ -56,7 +57,6 @@
       <Unit3>
         <Filename Value="numberstests.pas"/>
         <IsPartOfProject Value="True"/>
-        <UnitName Value="numberstests"/>
       </Unit3>
       <Unit4>
         <Filename Value="manualtests.pas"/>
@@ -66,20 +66,20 @@
       <Unit5>
         <Filename Value="testsutility.pas"/>
         <IsPartOfProject Value="True"/>
-        <UnitName Value="testsutility"/>
       </Unit5>
       <Unit6>
         <Filename Value="internaltests.pas"/>
         <IsPartOfProject Value="True"/>
+        <UnitName Value="internaltests"/>
       </Unit6>
       <Unit7>
         <Filename Value="formattests.pas"/>
         <IsPartOfProject Value="True"/>
-        <UnitName Value="formattests"/>
       </Unit7>
       <Unit8>
         <Filename Value="colortests.pas"/>
         <IsPartOfProject Value="True"/>
+        <UnitName Value="colortests"/>
       </Unit8>
       <Unit9>
         <Filename Value="fonttests.pas"/>
@@ -110,10 +110,12 @@
       <Unit15>
         <Filename Value="errortests.pas"/>
         <IsPartOfProject Value="True"/>
+        <UnitName Value="errortests"/>
       </Unit15>
       <Unit16>
         <Filename Value="virtualmodetests.pas"/>
         <IsPartOfProject Value="True"/>
+        <UnitName Value="virtualmodetests"/>
       </Unit16>
     </Units>
   </ProjectOptions>

components/fpspreadsheet/xlsbiff2.pas

@@ -509,7 +509,7 @@ begin
   BIFF2EOF := False;
 
   { In BIFF2 files there is only one worksheet, let's create it }
-  FWorksheet := AData.AddWorksheet('');
+  FWorksheet := AData.AddWorksheet('', true);  // true --> accept empty sheet name
 
   { Read all records in a loop }
   while not BIFF2EOF do