Skip to content
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

Wildcard searches fails with empty cells and those with forward slashes in wildcard #2430

Closed
lucasnetau opened this issue Dec 2, 2021 · 0 comments · Fixed by #2431
Closed

Comments

@lucasnetau
Copy link
Contributor

lucasnetau commented Dec 2, 2021

This is:

- [ X ] a bug report

What is the expected behavior?

Wildcard search handles comparing empty cells and comparing a wildcard value including a forward slash /

What is the current behavior?

  1. Wildcard Search via SUMIF against Empty Cells results in a fatal error

Fatal error: Uncaught TypeError: PhpOffice\PhpSpreadsheet\Calculation\Internal\WildcardMatch::compare(): Argument #1 ($value) must be of type string, null given in vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Calculation/Internal/WildcardMatch.php:33
Stack trace:
#0 [internal function]: PhpOffice\PhpSpreadsheet\Calculation\Internal\WildcardMatch::compare(NULL, 'ANABAENA.*')

  1. MATCH with a wildcard value including a forward slash results the forward slash terminating the regex and being interpreted as a modifier:

Warning: preg_match(): Unknown modifier 'P' in

  1. Calling WildcardMatch::compare() against an blank string returns true, we would expected false as there cannot be a wildcard match against a blank string

What are the steps to reproduce?

Using https://gist.github.com/MarkBaker/3ea8664c73bfe6af5bc6f437803589f2 as a template script

widlcard search error (not triggerable directly via MATCH)

$spreadSheet = new Spreadsheet();
$workSheet = $spreadSheet->getActiveSheet();

// Set details for the formula that we want to evaluate, together with any data on which it depends
$workSheet->setCellValue('A1', null)
    ->setCellValue('A2', 'abc123fff')
    ->setCellValue('A3', 'abc123*')
    ->setCellValue('B2', 1)
    ->setCellValue('B3', 2)
    ->setCellValue('A4', '=SUMIF(A1:A2,A3,B2:B3)');

preg_match error

$spreadSheet = new Spreadsheet();
$workSheet = $spreadSheet->getActiveSheet();

// Set details for the formula that we want to evaluate, together with any data on which it depends
$workSheet->setCellValue('A1', 'abc123fff')
    ->setCellValue('A2', 'abc/123fff')
    ->setCellValue('A3', 'abc/123*')
    ->setCellValue('A4', '=MATCH(A3,A1:A2,0)');

Which versions of PhpSpreadsheet and PHP are affected?

1.19, 1.20, master (Type error introduced in PR#2162)

Patch

Will create a PR for this issue. This patch has been written and tested so far

diff --git a/src/PhpSpreadsheet/Calculation/Internal/WildcardMatch.php b/src/PhpSpreadsheet/Calculation/Internal/WildcardMatch.php
index 5731569f..371ad8b3 100644
--- a/src/PhpSpreadsheet/Calculation/Internal/WildcardMatch.php
+++ b/src/PhpSpreadsheet/Calculation/Internal/WildcardMatch.php
@@ -25,13 +25,13 @@ class WildcardMatch
     public static function wildcard(string $wildcard): string
     {
         // Preg Escape the wildcard, but protecting the Excel * and ? search characters
-        return str_replace(self::SEARCH_SET, self::REPLACEMENT_SET, preg_quote($wildcard));
+        return str_replace(self::SEARCH_SET, self::REPLACEMENT_SET, preg_quote($wildcard, '/'));
     }
 
-    public static function compare(string $value, string $wildcard): bool
+    public static function compare(?string $value, string $wildcard): bool
     {
-        if ($value === '') {
-            return true;
+        if ($value === '' || $value === null) {
+            return false;
         }
 
         return (bool) preg_match("/^{$wildcard}\$/mui", $value);

lucasnetau added a commit to lucasnetau/PhpSpreadsheet that referenced this issue Dec 2, 2021
* Handle a wildcard match that contains a forward slash in the pattern by adding / to the delimiter list of preg_quote
* Fix SUMIF doing a wildcard match on empty cells (NULL)
* Fix compare logic to return false when value is an empty string or NULL (Verified against LibreOffice SUMIF and MATCH handling of empty cells)
oleibman pushed a commit that referenced this issue Dec 4, 2021
* Handle a wildcard match that contains a forward slash in the pattern by adding / to the delimiter list of preg_quote
* Fix SUMIF doing a wildcard match on empty cells (NULL)
* Fix compare logic to return false when value is an empty string or NULL (Verified against LibreOffice SUMIF and MATCH handling of empty cells)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant