Skip to content

Commit 8352c49

Browse files
oleibmanoleibman
committed
Backport 2 Changes to Writer/Xls/Parser
PR #4333 and PR #4344.
1 parent ffb47b6 commit 8352c49

File tree

4 files changed

+143
-10
lines changed

4 files changed

+143
-10
lines changed

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com)
66
and this project adheres to [Semantic Versioning](https://semver.org).
77

8+
# TBD - 1.29.10
9+
10+
### Fixed
11+
12+
- Xls writer Parser Mishandling True/False Argument. Backport of [PR #4333](https://github.com/PHPOffice/PhpSpreadsheet/pull/4333)
13+
- Xls writer Parser Parse By Character Not Byte. Backport of [PR #4344](https://github.com/PHPOffice/PhpSpreadsheet/pull/4344)
14+
815
# 2025-01-26 - 1.29.9
916

1017
### Fixed

‎src/PhpSpreadsheet/Writer/Xls/Parser.php‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ class Parser
6767
. '[$]?[A-Ia-i]?[A-Za-z][$]?(\\d+)'
6868
. '$~u';
6969

70+
private const UTF8 = 'UTF-8';
71+
7072
/**
7173
* The index of the character we are currently looking at.
7274
*/
@@ -1001,37 +1003,38 @@ private function advance(): void
10011003
{
10021004
$token = '';
10031005
$i = $this->currentCharacter;
1004-
$formula_length = strlen($this->formula);
1006+
$formula = mb_str_split($this->formula, 1, self::UTF8);
1007+
$formula_length = count($formula);
10051008
// eat up white spaces
10061009
if ($i < $formula_length) {
1007-
while ($this->formula[$i] == ' ') {
1010+
while ($formula[$i] === ' ') {
10081011
++$i;
10091012
}
10101013

10111014
if ($i < ($formula_length - 1)) {
1012-
$this->lookAhead = $this->formula[$i + 1];
1015+
$this->lookAhead = $formula[$i + 1];
10131016
}
10141017
$token = '';
10151018
}
10161019

10171020
while ($i < $formula_length) {
1018-
$token .= $this->formula[$i];
1021+
$token .= $formula[$i];
10191022

10201023
if ($i < ($formula_length - 1)) {
1021-
$this->lookAhead = $this->formula[$i + 1];
1024+
$this->lookAhead = $formula[$i + 1];
10221025
} else {
10231026
$this->lookAhead = '';
10241027
}
10251028

1026-
if ($this->match($token) != '') {
1029+
if ($this->match($token) !== '') {
10271030
$this->currentCharacter = $i + 1;
10281031
$this->currentToken = $token;
10291032

10301033
return;
10311034
}
10321035

10331036
if ($i < ($formula_length - 2)) {
1034-
$this->lookAhead = $this->formula[$i + 2];
1037+
$this->lookAhead = $formula[$i + 2];
10351038
} else { // if we run out of characters lookAhead becomes empty
10361039
$this->lookAhead = '';
10371040
}
@@ -1208,8 +1211,8 @@ private function match(string $token): string
12081211
public function parse(string $formula): bool
12091212
{
12101213
$this->currentCharacter = 0;
1211-
$this->formula = (string) $formula;
1212-
$this->lookAhead = $formula[1] ?? '';
1214+
$this->formula = $formula;
1215+
$this->lookAhead = mb_substr($formula, 1, 1, self::UTF8);
12131216
$this->advance();
12141217
$this->parseTree = $this->condition();
12151218

@@ -1634,7 +1637,9 @@ public function toReversePolish(array $tree = []): string
16341637
}
16351638

16361639
// add its left subtree and return.
1637-
return $left_tree . $this->convertFunction($tree['value'], $tree['right']);
1640+
if ($left_tree !== '' || $tree['right'] !== '') {
1641+
return $left_tree . $this->convertFunction($tree['value'], $tree['right'] ?: 0);
1642+
}
16381643
}
16391644
$converted_tree = $this->convert($tree['value']);
16401645

‎tests/PhpSpreadsheetTests/Writer/Xls/Issue4331Test.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls;
6+
7+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
9+
10+
class Issue4331Test extends AbstractFunctional
11+
{
12+
public function testIssue4331(): void
13+
{
14+
$spreadsheet = new Spreadsheet();
15+
$sheet = $spreadsheet->getActiveSheet();
16+
$c3 = '=VLOOKUP(B3,$B$10:$C$13,2,FALSE)';
17+
$d3 = '=VLOOKUP("intermediate",$B$10:$C$13,2,TRUE)';
18+
$c4 = '=VLOOKUP(B3,$B$10:$C$13,2,FALSE())';
19+
$d4 = '=VLOOKUP("intermediate",$B$10:$C$13,2,TRUE())';
20+
$sheet->fromArray(
21+
[
22+
['level', 'result'],
23+
['medium', $c3, $d3],
24+
[null, $c4, $d4],
25+
],
26+
null,
27+
'B2',
28+
true
29+
);
30+
$sheet->fromArray(
31+
[
32+
['high', 6],
33+
['low', 2],
34+
['medium', 4],
35+
['none', 0],
36+
],
37+
null,
38+
'B10',
39+
true
40+
);
41+
42+
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xls');
43+
$spreadsheet->disconnectWorksheets();
44+
45+
$worksheet = $reloadedSpreadsheet->getActiveSheet();
46+
self::assertSame($c3, $worksheet->getCell('C3')->getValue());
47+
self::assertSame(4, $worksheet->getCell('C3')->getCalculatedValue());
48+
self::assertSame($d3, $worksheet->getCell('D3')->getValue());
49+
self::assertSame(6, $worksheet->getCell('D3')->getCalculatedValue());
50+
self::assertSame($c4, $worksheet->getCell('C4')->getValue());
51+
self::assertSame(4, $worksheet->getCell('C4')->getCalculatedValue());
52+
self::assertSame($d4, $worksheet->getCell('D4')->getValue());
53+
self::assertSame(6, $worksheet->getCell('D4')->getCalculatedValue());
54+
$reloadedSpreadsheet->disconnectWorksheets();
55+
}
56+
}

‎tests/PhpSpreadsheetTests/Writer/Xls/NonLatinFormulasTest.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls;
4+
5+
use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
6+
use PhpOffice\PhpSpreadsheet\Cell\DataValidator;
7+
use PhpOffice\PhpSpreadsheet\NamedRange;
8+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
9+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
10+
11+
class NonLatinFormulasTest extends AbstractFunctional
12+
{
13+
public function testNonLatin(): void
14+
{
15+
$spreadsheet = new Spreadsheet();
16+
$worksheet = $spreadsheet->getActiveSheet();
17+
18+
$validation = $worksheet->getCell('B1')->getDataValidation();
19+
$validation->setType(DataValidation::TYPE_LIST);
20+
$validation->setErrorStyle(DataValidation::STYLE_STOP);
21+
$validation->setAllowBlank(false);
22+
$validation->setShowInputMessage(true);
23+
$validation->setShowErrorMessage(true);
24+
$validation->setShowDropDown(true);
25+
$validation->setFormula1('"слово, сло"');
26+
27+
$dataValidator = new DataValidator();
28+
$worksheet->getCell('B1')->setValue('слово');
29+
self::assertTrue(
30+
$dataValidator->isValid($worksheet->getCell('B1'))
31+
);
32+
$worksheet->getCell('B1')->setValue('слов');
33+
self::assertFalse(
34+
$dataValidator->isValid($worksheet->getCell('B1'))
35+
);
36+
37+
$worksheet->setTitle('словслов');
38+
$worksheet->getCell('A1')->setValue('=словслов!B1');
39+
$worksheet->getCell('A2')->setValue("='словслов'!B1");
40+
$spreadsheet->addNamedRange(new NamedRange('слсл', $worksheet, '$B$1'));
41+
$worksheet->getCell('A3')->setValue('=слсл');
42+
43+
$robj = $this->writeAndReload($spreadsheet, 'Xls');
44+
$spreadsheet->disconnectWorksheets();
45+
$sheet0 = $robj->getActiveSheet();
46+
self::assertSame('словслов', $sheet0->getTitle());
47+
self::assertSame('=словслов!B1', $sheet0->getCell('A1')->getValue());
48+
self::assertSame('слов', $sheet0->getCell('A1')->getCalculatedValue());
49+
// Quotes around sheet name are stripped off - harmless
50+
//self::assertSame("='словслов'!B1", $sheet0->getCell('A2')->getValue());
51+
self::assertSame('слов', $sheet0->getCell('A2')->getCalculatedValue());
52+
// Formulas with defined names don't work in Xls Writer
53+
//self::assertSame('=слсл', $sheet0->getCell('A3')->getValue());
54+
// But result should be accurate
55+
self::assertSame('слов', $sheet0->getCell('A3')->getCalculatedValue());
56+
$names = $robj->getDefinedNames();
57+
self::assertCount(1, $names);
58+
// name has been uppercased
59+
$namedRange = $names['СЛСЛ'] ?? null;
60+
self::assertInstanceOf(NamedRange::class, $namedRange);
61+
self::assertSame('$B$1', $namedRange->getRange());
62+
63+
$robj->disconnectWorksheets();
64+
}
65+
}

0 commit comments

Comments
 (0)