Skip to content

Commit 3789837

Browse files
oleibmanoleibman
committed
Backport 2 Fixes to Writer/Xls/Parser
PR #4333 and PR #4344.
1 parent 8be52af commit 3789837

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
@@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org).
99

1010
### Fixed
1111

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+
15+
# 2025-01-26 - 2.1.8
16+
17+
### Fixed
18+
1219
- Backported security patch for control characters in protocol.
1320
- Use Composer\Pcre in Xls/Parser. Partial backport of [PR #4203](https://github.com/PHPOffice/PhpSpreadsheet/pull/4203)
1421

‎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
*/
@@ -991,37 +993,38 @@ private function advance(): void
991993
{
992994
$token = '';
993995
$i = $this->currentCharacter;
994-
$formula_length = strlen($this->formula);
996+
$formula = mb_str_split($this->formula, 1, self::UTF8);
997+
$formula_length = count($formula);
995998
// eat up white spaces
996999
if ($i < $formula_length) {
997-
while ($this->formula[$i] == ' ') {
1000+
while ($formula[$i] === ' ') {
9981001
++$i;
9991002
}
10001003

10011004
if ($i < ($formula_length - 1)) {
1002-
$this->lookAhead = $this->formula[$i + 1];
1005+
$this->lookAhead = $formula[$i + 1];
10031006
}
10041007
$token = '';
10051008
}
10061009

10071010
while ($i < $formula_length) {
1008-
$token .= $this->formula[$i];
1011+
$token .= $formula[$i];
10091012

10101013
if ($i < ($formula_length - 1)) {
1011-
$this->lookAhead = $this->formula[$i + 1];
1014+
$this->lookAhead = $formula[$i + 1];
10121015
} else {
10131016
$this->lookAhead = '';
10141017
}
10151018

1016-
if ($this->match($token) != '') {
1019+
if ($this->match($token) !== '') {
10171020
$this->currentCharacter = $i + 1;
10181021
$this->currentToken = $token;
10191022

10201023
return;
10211024
}
10221025

10231026
if ($i < ($formula_length - 2)) {
1024-
$this->lookAhead = $this->formula[$i + 2];
1027+
$this->lookAhead = $formula[$i + 2];
10251028
} else { // if we run out of characters lookAhead becomes empty
10261029
$this->lookAhead = '';
10271030
}
@@ -1198,8 +1201,8 @@ private function match(string $token): string
11981201
public function parse(string $formula): bool
11991202
{
12001203
$this->currentCharacter = 0;
1201-
$this->formula = (string) $formula;
1202-
$this->lookAhead = $formula[1] ?? '';
1204+
$this->formula = $formula;
1205+
$this->lookAhead = mb_substr($formula, 1, 1, self::UTF8);
12031206
$this->advance();
12041207
$this->parseTree = $this->condition();
12051208

@@ -1624,7 +1627,9 @@ public function toReversePolish(array $tree = []): string
16241627
}
16251628

16261629
// add its left subtree and return.
1627-
return $left_tree . $this->convertFunction($tree['value'], $tree['right']);
1630+
if ($left_tree !== '' || $tree['right'] !== '') {
1631+
return $left_tree . $this->convertFunction($tree['value'], $tree['right'] ?: 0);
1632+
}
16281633
}
16291634
$converted_tree = $this->convert($tree['value']);
16301635

‎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)