Skip to content

Problem with isFormula() / Calculation::CALCULATION_REGEXP_CELLREF #2304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
billynoah opened this issue Sep 23, 2021 · 1 comment
Closed

Comments

@billynoah
Copy link

billynoah commented Sep 23, 2021

This is:

- [x] a bug report
- [ ] a feature request
- [x] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

In PhpSpreadsheet/Calculation/Functions.php the function isFormula() uses regex Calculation::CALCULATION_REGEXP_CELLREF which should be able to successfully parse $cellReference.

What is the current behavior?

Regex Calculation::CALCULATION_REGEXP_CELLREF fails to parse $cellReference. Actually - the regex is not even checked before referencing $matches.

What are the steps to reproduce?

In the spreadsheet I am trying to read when I echo $cellReference I get: Load Sheet!F3. The regex fails in this case and so the following lines reference keys which do not exist since $matches is empty:

$cellReference = $matches[6] . $matches[7];
$worksheetName = str_replace("''", "'", trim($matches[2], "'"));

At the very least there should be a check like:

if preg_match(...) {

And probably, the regex Calculation::CALCULATION_REGEXP_CELLREF should be updated to address this case.

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require __DIR__ . '/vendor/autoload.php';
PhpOffice\PhpSpreadsheet\IOFactory::load('test_case.xlsx');

Using this file for testing: test_case.xlsx

Which versions of PhpSpreadsheet and PHP are affected?

This has been an ongoing issue and affects 1.18.0 (Current version) + all previous versions I have used

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Sep 25, 2021
See issue PHPOffice#2304. User sets a cell to `ISFORMULA(cell)`, where `cell` exists on a sheet whose title contains a space, and receives an error. Coordinates are not being passed correctly to Functions::isFormula; in particular, the sheet name is not enclosed in apostrophes, e.g. `Sheet Name!A1` rather than `'Sheet Name'!A1`. (Note that sheet name was not specified in Cell; PhpSpreadsheet adds it before calling isFormula.) Sheets without embedded spaces (or other non-word characters) are handled correctly with or without apostrophes, but spaces require surrounding apostrophes.

As part of this investigation, I determined that Excel handles defined names and cell ranges in ISFORMULA (subject to spills), and that PhpSpreadsheet does not. It is changed to handle them. In the absence of spill support, it will use only the first cell in the range.

Existing tests for ISFORMULA used mocking unneccesarily. They are moved to a separate test member, and mocking is no longer used.
oleibman added a commit that referenced this issue Oct 3, 2021
* isFormula Referencing Sheet With Space in Title

See issue #2304. User sets a cell to `ISFORMULA(cell)`, where `cell` exists on a sheet whose title contains a space, and receives an error. Coordinates are not being passed correctly to Functions::isFormula; in particular, the sheet name is not enclosed in apostrophes, e.g. `Sheet Name!A1` rather than `'Sheet Name'!A1`. (Note that sheet name was not specified in Cell; PhpSpreadsheet adds it before calling isFormula.) Sheets without embedded spaces (or other non-word characters) are handled correctly with or without apostrophes, but spaces require surrounding apostrophes.

As part of this investigation, I determined that Excel handles defined names and cell ranges in ISFORMULA (subject to spills), and that PhpSpreadsheet does not. It is changed to handle them. In the absence of spill support, it will use only the first cell in the range.

Existing tests for ISFORMULA used mocking unneccesarily. They are moved to a separate test member, and mocking is no longer used.

* PhpUnit and Jpgraph

35_Char_render.php had previously been a problem only for PHP8+. It is now a problem for PHP7.4, and will therefore be skipped all the time.
@PowerKiKi
Copy link
Member

Fixed in 1.19.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants