From 6c19ca21cb13c4f12307ededf6f8048f86b22e63 Mon Sep 17 00:00:00 2001 From: wp_xxyyzz Date: Sun, 3 Jun 2018 11:07:42 +0000 Subject: [PATCH] fpspreadsheet: Fix crash when referenced worksheet does not exist. git-svn-id: https://svn.code.sf.net/p/lazarus-ccr/svn@6453 8e941d3f-bd1b-0410-a28a-d453659cc2b4 --- .../source/common/fpsexprparser.pas | 32 +++-- .../source/common/fpspreadsheet.pas | 8 +- .../fpspreadsheet/source/common/xlscommon.pas | 59 ++------ .../tests/singleformulatests.pas | 129 +++++++++++++----- 4 files changed, 124 insertions(+), 104 deletions(-) diff --git a/components/fpspreadsheet/source/common/fpsexprparser.pas b/components/fpspreadsheet/source/common/fpsexprparser.pas index 199bfc65e..609305b45 100644 --- a/components/fpspreadsheet/source/common/fpsexprparser.pas +++ b/components/fpspreadsheet/source/common/fpsexprparser.pas @@ -606,6 +606,7 @@ type FCell: PCell; // cell which contains the formula // FSheetName: String; // referenced other sheet FSheetIndex: Integer; // index of referenced other sheet + FHas3DLink: Boolean; FIsRef: Boolean; FError: TsErrorValue; protected @@ -3845,18 +3846,23 @@ constructor TsCellExprNode.Create(AParser: TsExpressionParser; AWorksheet: TsBasicWorksheet; ASheetName: String; ARow, ACol: Cardinal; AFlags: TsRelFlags); begin + FError := errOK; FParser := AParser; FWorksheet := AWorksheet; - if (ASheetName = '') then - FSheetIndex := -1 - else + if (ASheetName = '') then begin + FSheetIndex := -1; + FHas3DLink := false; + end else begin FSheetIndex := TsWorkbook(GetWorkbook).GetWorksheetIndex(ASheetName); + if FSheetIndex = -1 then + FError := errIllegalRef; + FHas3DLink := true; + end; FRow := ARow; FCol := ACol; FFlags := AFlags; - FError := errOK; - FCell := (GetSheet as TsWorksheet).FindCell(FRow, FCol); - if Has3DLink then FParser.FContains3DRef := true; + FCell := TsWorksheet(FWorksheet).FindCell(FRow, FCol); +// FCell := (GetSheet as TsWorksheet).FindCell(FRow, FCol); end; function TsCellExprNode.AsRPNItem(ANext: PRPNItem): PRPNItem; @@ -4041,7 +4047,7 @@ end; function TsCellExprNode.Has3DLink: Boolean; begin - Result := FSheetIndex <> -1; + Result := FHas3dLink; end; function TsCellExprNode.NodeType: TsResultType; @@ -4082,17 +4088,13 @@ begin ((ASheet1 <> '') and (ASheet2 = '')); FSheetIndex[1] := book.GetWorksheetIndex(ASheet1); - { - if FSheetIndex[1] = -1 then + if (FSheetIndex[1] = -1) and (ASheet1 <> '') then FError := errIllegalREF else - } if ASheet2 <> '' then begin FSheetIndex[2] := book.GetWorksheetIndex(ASheet2); - { - if FSheetIndex[2] = -1 then + if (FSheetIndex[2] = -1) and (ASheet2 <> '') then FError := errIllegalREF; - } end else FSheetIndex[2] := FSheetIndex[1]; EnsureOrder(FSheetIndex[1], FSheetIndex[2]); @@ -4249,6 +4251,10 @@ begin for ss := s[1] to s[2] do begin sheet := (Workbook as TsWorkbook).GetWorksheetByIndex(ss); + if sheet = nil then begin + AResult := ErrorResult(errIllegalRef); + exit; + end; for formula in sheet.Formulas do if (formula^.Row >= r[1]) and (formula^.Row <= r[2]) and (formula^.Col >= c[1]) and (formula^.Col <= c[2]) diff --git a/components/fpspreadsheet/source/common/fpspreadsheet.pas b/components/fpspreadsheet/source/common/fpspreadsheet.pas index a21f3fe85..be1f6292a 100644 --- a/components/fpspreadsheet/source/common/fpspreadsheet.pas +++ b/components/fpspreadsheet/source/common/fpspreadsheet.pas @@ -2808,15 +2808,15 @@ end; If the cell contains a text value it is attempted to convert it to a number. If the cell is empty or its contents cannot be represented as a number the - value 0.0 is returned. + value NaN is returned. @param ACell Pointer to the cell - @return Floating-point value representing the cell contents, or 0.0 if cell + @return Floating-point value representing the cell contents, or NaN if cell does not exist or its contents cannot be converted to a number. -------------------------------------------------------------------------------} function TsWorksheet.ReadAsNumber(ACell: PCell): Double; begin - Result := 0.0; + Result := NaN; if ACell = nil then exit; @@ -2827,7 +2827,7 @@ begin Result := ACell^.NumberValue; cctUTF8String: if not TryStrToFloat(ACell^.UTF8StringValue, Result, FWorkbook.FormatSettings) - then Result := 0.0; + then Result := NaN; cctBool: if ACell^.BoolValue then Result := 1.0 else Result := 0.0; end; diff --git a/components/fpspreadsheet/source/common/xlscommon.pas b/components/fpspreadsheet/source/common/xlscommon.pas index ca5e3c1a2..8b2d091e7 100644 --- a/components/fpspreadsheet/source/common/xlscommon.pas +++ b/components/fpspreadsheet/source/common/xlscommon.pas @@ -3469,14 +3469,21 @@ var sheetlist: TsBIFFExternSheetList; sheetIdx, sheetIdx1, sheetIdx2: Integer; workbook: TsWorkbook; + sheetName: String; begin Unused(MustRebuildFormulas); sheetlist := TsBIFFExternSheetList(AData); if (ANode is TsCellExprNode) and TsCellExprNode(ANode).Has3DLink then - sheetList.AddSheet(TsCellExprNode(ANode).GetSheetName, ebkInternal) - else + begin + if (ANode as TsCellExprNode).Error <> errOK then + exit; + sheetName := TsCellExprNode(ANode).GetSheetName; + sheetList.AddSheet(sheetName, ebkInternal) + end else if (ANode is TsCellRangeExprNode) and TsCellRangeExprNode(ANode).Has3DLink then begin + if (ANode as TsCellRangeExprNode).Error <> errOK then + exit; workbook := TsCellRangeExprNode(ANode).Workbook as TsWorkbook; sheetIdx1 := TsCellRangeExprNode(ANode).GetSheetIndex(1); sheetIdx2 := TsCellRangeExprNode(ANode).GetSheetIndex(2); @@ -3500,54 +3507,6 @@ function TsSpreadBIFFWriter.CollectExternData(AWorksheet: TsBasicWorksheet = nil for formula in ASheet.Formulas do formula^.Parser.IterateNodes(@DoCollectSheetsWith3dRefs, ASheetList); end; -{ - var - cell: PCell; - workbook: TsWorkbook; - parser: TsExpressionParser; - rpn: TsRPNFormula; - fe: TsFormulaElement; - i, j: Integer; - kind: TsBIFFExternKind; - begin - workbook := ASheet.Workbook; - for cell in ASheet.Cells do - begin - if not HasFormula(cell) then - Continue; - if (cell^.Flags * [cf3dFormula, cfCalculated] = [cfCalculated]) then - Continue; - - if (pos('[', ASheet.Name) = 0) then - kind := ebkInternal - else - kind := ebkExternal; // External refs: [filename]Sheet1!A1 - - parser := TsSpreadsheetParser.Create(ASheet); - try - parser.Expression := cell^.FormulaValue; - rpn := parser.RPNFormula; - for i:=0 to High(rpn) do - begin - fe := rpn[i]; - if fe.ElementKind in [fekCell3d, fekCellRef3d, fekCellRange3d] then begin - if fe.Sheet = -1 then - ASheetList.AddSheet(ASheet.Name, kind) - else - if fe.Sheet2 = -1 then - ASheetList.AddSheet(workbook.GetWorksheetByIndex(fe.Sheet).Name, kind) - else - for j :=fe.Sheet to fe.Sheet2 do - ASheetList.AddSheet(workbook.GetWorksheetbyIndex(j).Name, kind); - end; - end; - finally - parser.Free; - rpn := nil; - end; - end; - end; - } var sheet: TsWorksheet; diff --git a/components/fpspreadsheet/tests/singleformulatests.pas b/components/fpspreadsheet/tests/singleformulatests.pas index f0eb803bb..c7d353fb7 100644 --- a/components/fpspreadsheet/tests/singleformulatests.pas +++ b/components/fpspreadsheet/tests/singleformulatests.pas @@ -23,7 +23,7 @@ type protected procedure SetUp; override; procedure TearDown; override; - procedure TestFloatFormula(AFormula: String; AExpected: Double; + procedure TestFormula(AFormula: String; AExpected: String; ATestKind: TFormulaTestKind; AFormat: TsSpreadsheetFormat; AExpectedFormula: String = ''); procedure TestWorksheet(ATestKind: TWorksheetTestKind; ATestCase: Integer); @@ -63,6 +63,16 @@ type procedure SumMultiSheetRange_FlippedSheetsAndCells_OOXML; procedure SumMultiSheetRange_FlippedSheetsAndCells_ODS; + procedure NonExistantSheet_BIFF5; + procedure NonExistantSheet_BIFF8; + procedure NonExistantSheet_OOXML; + procedure NonExistantSheet_ODS; + + procedure NonExistantSheetRange_BIFF5; + procedure NonExistantSheetRange_BIFF8; + procedure NonExistantSheetRange_OOXML; + procedure NonExistantSheetRange_ODS; + procedure RenameWorksheet_Single; procedure RenameWorksheet_Multi_First; procedure RenameWorksheet_Multi_Inner; @@ -87,7 +97,7 @@ uses {$IFDEF FORMULADEBUG} LazLogger, {$ENDIF} - typinfo, lazUTF8, fpsUtils; + Math, typinfo, lazUTF8, fpsUtils; { TSpreadExtendedFormulaTests } @@ -102,8 +112,8 @@ begin inherited TearDown; end; -procedure TSpreadSingleFormulaTests.TestFloatFormula(AFormula: String; - AExpected: Double; ATestKind: TFormulaTestKind; AFormat: TsSpreadsheetFormat; +procedure TSpreadSingleFormulaTests.TestFormula(AFormula: String; + AExpected: String; ATestKind: TFormulaTestKind; AFormat: TsSpreadsheetFormat; AExpectedFormula: String = ''); const SHEET1 = 'Sheet1'; @@ -118,7 +128,7 @@ var TempFile: string; //write xls/xml to this file and read back from it cell: PCell; actualformula: String; - actualValue: Double; + actualValue: String; begin TempFile := GetTempFileName; if AExpectedFormula = '' then AExpectedFormula := AFormula; @@ -127,6 +137,7 @@ begin // Create test workbook and write test formula and needed cells workbook := TsWorkbook.Create; try + workbook.FormatSettings.DecimalSeparator := '.'; workbook.Options := workbook.Options + [boCalcBeforeSaving, boAutoCalc]; workSheet:= workBook.AddWorksheet(SHEET1); @@ -162,7 +173,7 @@ begin CheckEquals(AExpectedFormula, actualFormula, 'Unsaved formula text mismatch'); // Read calculated value before saving - actualvalue := worksheet.ReadAsNumber(TESTCELL_ROW, TESTCELL_COL); + actualValue := worksheet.ReadAsText(TESTCELL_ROW, TESTCELL_COL); CheckEquals(AExpected, actualvalue, 'Unsaved calculated value mismatch'); // Save @@ -174,18 +185,18 @@ begin // Read file workbook := TsWorkbook.Create; try + workbook.FormatSettings.DecimalSeparator := '.'; workbook.Options := workbook.Options + [boReadFormulas, boAutoCalc]; workbook.ReadFromFile(TempFile, AFormat); worksheet := workbook.GetFirstWorksheet; // Read calculated formula value - actualvalue := worksheet.ReadAsNumber(TESTCELL_ROW, TESTCELL_COL); + actualValue := worksheet.ReadAsText(TESTCELL_ROW, TESTCELL_COL); CheckEquals(AExpected, actualValue, 'Saved calculated value mismatch'); cell := worksheet.FindCell(TESTCELL_ROW, TESTCELL_COL); actualformula := worksheet.Formulas.FindFormula(cell)^.Text; -// actualformula := cell^.FormulaValue; - // When writing ranges are reconstructed in correct order. + // When writing ranges are reconstructed in correct order -> compare against AExpectedFormula CheckEquals(AExpectedFormula, actualformula, 'Saved formula text mismatch.'); finally workbook.Free; @@ -198,125 +209,125 @@ end; procedure TSpreadSingleFormulaTests.AddConst_BIFF2; begin - TestFloatFormula('1+1', 2, ftkConstants, sfExcel2); + TestFormula('1+1', '2', ftkConstants, sfExcel2); end; procedure TSpreadSingleFormulaTests.AddConst_BIFF5; begin - TestFloatFormula('1+1', 2, ftkConstants, sfExcel5); + TestFormula('1+1', '2', ftkConstants, sfExcel5); end; procedure TSpreadSingleFormulaTests.AddConst_BIFF8; begin - TestFloatFormula('1+1', 2, ftkConstants, sfExcel8); + TestFormula('1+1', '2', ftkConstants, sfExcel8); end; procedure TSpreadSingleFormulaTests.AddConst_OOXML; begin - TestFloatFormula('1+1', 2, ftkConstants, sfOOXML); + TestFormula('1+1', '2', ftkConstants, sfOOXML); end; procedure TSpreadSingleFormulaTests.AddConst_ODS; begin - TestFloatFormula('1+1', 2, ftkConstants, sfOpenDocument); + TestFormula('1+1', '2', ftkConstants, sfOpenDocument); end; {---------------} procedure TSpreadSingleFormulaTests.AddCells_BIFF2; begin - TestFloatFormula('C3+C4', -1.0, ftkCells, sfExcel2); + TestFormula('C3+C4', '-1', ftkCells, sfExcel2); end; procedure TSpreadSingleFormulaTests.AddCells_BIFF5; begin - TestFloatFormula('C3+C4', -1.0, ftkCells, sfExcel5); + TestFormula('C3+C4', '-1', ftkCells, sfExcel5); end; procedure TSpreadSingleFormulaTests.AddCells_BIFF8; begin - TestFloatFormula('C3+C4', -1.0, ftkCells, sfExcel8); + TestFormula('C3+C4', '-1', ftkCells, sfExcel8); end; procedure TSpreadSingleFormulaTests.AddCells_OOXML; begin - TestFloatFormula('C3+C4', -1.0, ftkCells, sfOOXML); + TestFormula('C3+C4', '-1', ftkCells, sfOOXML); end; procedure TSpreadSingleFormulaTests.AddCells_ODS; begin - TestFloatFormula('C3+C4', -1.0, ftkCells, sfOpenDocument); + TestFormula('C3+C4', '-1', ftkCells, sfOpenDocument); end; { ------ } procedure TSpreadSingleFormulaTests.SumRange_BIFF2; begin - TestFloatFormula('SUM(C3:C5)', 0.5, ftkCellRange, sfExcel2); + TestFormula('SUM(C3:C5)', '0.5', ftkCellRange, sfExcel2); end; procedure TSpreadSingleFormulaTests.SumRange_BIFF5; begin - TestFloatFormula('SUM(C3:C5)', 0.5, ftkCellRange, sfExcel5); + TestFormula('SUM(C3:C5)', '0.5', ftkCellRange, sfExcel5); end; procedure TSpreadSingleFormulaTests.SumRange_BIFF8; begin - TestFloatFormula('SUM(C3:C5)', 0.5, ftkCellRange, sfExcel8); + TestFormula('SUM(C3:C5)', '0.5', ftkCellRange, sfExcel8); end; procedure TSpreadSingleFormulaTests.SumRange_OOXML; begin - TestFloatFormula('SUM(C3:C5)', 0.5, ftkCellRange, sfOOXML); + TestFormula('SUM(C3:C5)', '0.5', ftkCellRange, sfOOXML); end; procedure TSpreadSingleFormulaTests.SumRange_ODS; begin - TestFloatFormula('SUM(C3:C5)', 0.5, ftkCellRange, sfOpenDocument); + TestFormula('SUM(C3:C5)', '0.5', ftkCellRange, sfOpenDocument); end; { ---- } procedure TSpreadSingleFormulaTests.SumSheetRange_BIFF5; begin - TestFloatFormula('SUM(Sheet2!C3:C5)', 5.0, ftkCellRangeSheet, sfExcel5); + TestFormula('SUM(Sheet2!C3:C5)', '5', ftkCellRangeSheet, sfExcel5); end; procedure TSpreadSingleFormulaTests.SumSheetRange_BIFF8; begin - TestFloatFormula('SUM(Sheet2!C3:C5)', 5.0, ftkCellRangeSheet, sfExcel8); + TestFormula('SUM(Sheet2!C3:C5)', '5', ftkCellRangeSheet, sfExcel8); end; procedure TSpreadSingleFormulaTests.SumSheetRange_OOXML; begin - TestFloatFormula('SUM(Sheet2!C3:C5)', 5.0, ftkCellRangeSheet, sfOOXML); + TestFormula('SUM(Sheet2!C3:C5)', '5', ftkCellRangeSheet, sfOOXML); end; procedure TSpreadSingleFormulaTests.SumSheetRange_ODS; begin - TestFloatFormula('SUM(Sheet2!C3:C5)', 5.0, ftkCellRangeSheet, sfOpenDocument); + TestFormula('SUM(Sheet2!C3:C5)', '5', ftkCellRangeSheet, sfOpenDocument); end; { ---- } procedure TSpreadSingleFormulaTests.SumMultiSheetRange_BIFF5; begin - TestFloatFormula('SUM(Sheet2:Sheet3!C3:C5)', 55.0, ftkCellRangeSheetRange, sfExcel5); + TestFormula('SUM(Sheet2:Sheet3!C3:C5)', '55', ftkCellRangeSheetRange, sfExcel5); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_BIFF8; begin - TestFloatFormula('SUM(Sheet2:Sheet3!C3:C5)', 55.0, ftkCellRangeSheetRange, sfExcel8); + TestFormula('SUM(Sheet2:Sheet3!C3:C5)', '55', ftkCellRangeSheetRange, sfExcel8); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_OOXML; begin - TestFloatFormula('SUM(Sheet2:Sheet3!C3:C5)', 55.0, ftkCellRangeSheetRange, sfOOXML); + TestFormula('SUM(Sheet2:Sheet3!C3:C5)', '55', ftkCellRangeSheetRange, sfOOXML); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_ODS; begin - TestFloatFormula('SUM(Sheet2:Sheet3!C3:C5)', 55.0, ftkCellRangeSheetRange, sfOpenDocument); + TestFormula('SUM(Sheet2:Sheet3!C3:C5)', '55', ftkCellRangeSheetRange, sfOpenDocument); end; { --- } @@ -326,28 +337,72 @@ end; expected range must be in correct order. } procedure TSpreadSingleFormulaTests.SumMultiSheetRange_FlippedSheetsAndCells_OOXML; begin - TestFloatFormula('SUM(Sheet3:Sheet2!C5:C3)', 55.0, ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); + TestFormula('SUM(Sheet3:Sheet2!C5:C3)', '55', ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_FlippedSheetsAndCells_ODS; begin - TestFloatFormula('SUM(Sheet3:Sheet2!C5:C3)', 55.0, ftkCellRangeSheetRange, sfOpenDocument, 'SUM(Sheet2:Sheet3!C3:C5)'); + TestFormula('SUM(Sheet3:Sheet2!C5:C3)', '55', ftkCellRangeSheetRange, sfOpenDocument, 'SUM(Sheet2:Sheet3!C3:C5)'); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_FlippedCells_BIFF8; begin // Upon writing the ranges are reconstructed for BIFF in correct order. - TestFloatFormula('SUM(Sheet2:Sheet3!C5:C3)', 55.0, ftkCellRangeSheetRange, sfExcel8, 'SUM(Sheet2:Sheet3!C3:C5)'); + TestFormula('SUM(Sheet2:Sheet3!C5:C3)', '55', ftkCellRangeSheetRange, sfExcel8, 'SUM(Sheet2:Sheet3!C3:C5)'); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_FlippedCells_OOXML; begin - TestFloatFormula('SUM(Sheet2:Sheet3!C5:C3)', 55.0, ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); + TestFormula('SUM(Sheet2:Sheet3!C5:C3)', '55', ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); end; procedure TSpreadSingleFormulaTests.SumMultiSheetRange_FlippedSheets_OOXML; begin - TestFloatFormula('SUM(Sheet3:Sheet2!C3:C5)', 55.0, ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); + TestFormula('SUM(Sheet3:Sheet2!C3:C5)', '55', ftkCellRangeSheetRange, sfOOXML, 'SUM(Sheet2:Sheet3!C3:C5)'); +end; + +{ --- } + +procedure TSpreadSingleFormulaTests.NonExistantSheet_BIFF5; +begin + TestFormula('Missing!C3', '#REF!', ftkCellRangeSheet, sfExcel5, '#REF!'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheet_BIFF8; +begin + TestFormula('Missing!C3', '#REF!', ftkCellRangeSheet, sfExcel8, '#REF!'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheet_OOXML; +begin + TestFormula('Missing!C3', '#REF!', ftkCellRangeSheet, sfOOXML, '#REF!'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheet_ODS; +begin + TestFormula('Missing!C3', '#REF!', ftkCellRangeSheet, sfOpenDocument, '#REF!'); +end; + +{ --- } + +procedure TSpreadSingleFormulaTests.NonExistantSheetRange_BIFF5; +begin + TestFormula('SUM(Missing1:Missing2!C3)', '#REF!', ftkCellRangeSheet, sfExcel5, 'SUM(#REF!)'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheetRange_BIFF8; +begin + TestFormula('SUM(Missing1:Missing2!C3)', '#REF!', ftkCellRangeSheet, sfExcel8, 'SUM(#REF!)'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheetRange_OOXML; +begin + TestFormula('SUM(Missing1:Missing2!C3)', '#REF!', ftkCellRangeSheet, sfOOXML, 'SUM(#REF!)'); +end; + +procedure TSpreadSingleFormulaTests.NonExistantSheetRange_ODS; +begin + TestFormula('SUM(Missing1:Missing2!C3)', '#REF!', ftkCellRangeSheet, sfOpenDocument, 'SUM(#REF!)'); end;