From 324588fb04b721953327a92d1c4c9d566b8b9189 Mon Sep 17 00:00:00 2001 From: wp_xxyyzz Date: Fri, 12 Sep 2014 09:38:12 +0000 Subject: [PATCH] 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 | 66 +++++++++++++++++-- .../fpspreadsheet/tests/internaltests.pas | 37 +++++++++++ .../fpspreadsheet/tests/spreadtestgui.lpi | 8 ++- components/fpspreadsheet/xlsbiff2.pas | 2 +- 4 files changed, 103 insertions(+), 10 deletions(-) diff --git a/components/fpspreadsheet/fpspreadsheet.pas b/components/fpspreadsheet/fpspreadsheet.pas index 775502547..c8844eedd 100755 --- a/components/fpspreadsheet/fpspreadsheet.pas +++ b/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,15 +3125,22 @@ 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; {@@ Returns true if the specified cell is the base of a merged cell range, i.e. @@ -5683,12 +5692,17 @@ end; It is added to the end of the list of worksheets - @param AName The name of the new worksheet + @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. diff --git a/components/fpspreadsheet/tests/internaltests.pas b/components/fpspreadsheet/tests/internaltests.pas index 509824630..aa9786d21 100644 --- a/components/fpspreadsheet/tests/internaltests.pas +++ b/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'; diff --git a/components/fpspreadsheet/tests/spreadtestgui.lpi b/components/fpspreadsheet/tests/spreadtestgui.lpi index 251664efc..1defb5eb5 100644 --- a/components/fpspreadsheet/tests/spreadtestgui.lpi +++ b/components/fpspreadsheet/tests/spreadtestgui.lpi @@ -48,6 +48,7 @@ + @@ -56,7 +57,6 @@ - @@ -66,20 +66,20 @@ - + - + @@ -110,10 +110,12 @@ + + diff --git a/components/fpspreadsheet/xlsbiff2.pas b/components/fpspreadsheet/xlsbiff2.pas index 0a5bd8a6d..3ef8c5208 100755 --- a/components/fpspreadsheet/xlsbiff2.pas +++ b/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