Skip to content

Commit faf6d81

Browse files
oleibmanoleibman
authored
Keep Calculated String Results Below 32K (#2921)
* Keep Calculated String Results Below 32K This is the result of an investigation into issue #2884 (see also PR #2913). It is, unfortunately, not a fix for the original problem; see the discussion in that PR for why I don't think there is a practical fix for that specific problem at this time. Excel limits strings to 32,767 characters. We already truncate strings to that length when added to the spreadsheet. However, we have been able to exceed that length as a result of the concatenation operator (Excel truncates); as a result of the CONCATENATE or TEXTJOIN functions (Excel returns #CALC!); or as a result of the REPLACE, REPT, SUBSTITUTE functions (Excel returns #VALUE!). This PR changes PhpSpreadsheet to return the same value as Excel in these cases. Note that Excel2003 truncates in all those cases; I don't think there is a way to differentiate that behavior in PhpSpreadsheet. However, LibreOffice and Gnumeric do not have that limit; if they have a limit at all, it is much higher. It would be fairly easy to use existing settings to differentiate between Excel and LibreOffice/Gnumeric in this respect. I have not done so in this PR because I am not sure how useful that is, and I can easily see it leading to problems (read in a LibreOffice spreadsheet with a 33K cell and then output to an Excel spreadsheet). Perhaps it should be handled with an additional opt-in setting. I changed the maximum size from a literal to a constant in the one place where it was already being enforced (Cell/DataType). I am not sure that is the best place for it to be defined; I am open to suggestions. * Implement Some Suggestions ... from @MarkBaker.
1 parent db12b73 commit faf6d81

File tree

14 files changed

+219
-39
lines changed

14 files changed

+219
-39
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PhpOffice\PhpSpreadsheet\Calculation\Token\Stack;
1212
use PhpOffice\PhpSpreadsheet\Cell\Cell;
1313
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
14+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
1415
use PhpOffice\PhpSpreadsheet\DefinedName;
1516
use PhpOffice\PhpSpreadsheet\ReferenceHelper;
1617
use PhpOffice\PhpSpreadsheet\Shared;
@@ -4711,11 +4712,19 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)
47114712
// Perform the required operation against the operand 1 matrix, passing in operand 2
47124713
$matrixResult = $matrix->concat($operand2);
47134714
$result = $matrixResult->getArray();
4715+
if (isset($result[0][0])) {
4716+
$result[0][0] = Shared\StringHelper::substring($result[0][0], 0, DataType::MAX_STRING_LENGTH);
4717+
}
47144718
} catch (\Exception $ex) {
47154719
$this->debugLog->writeDebugLog('JAMA Matrix Exception: %s', $ex->getMessage());
47164720
$result = '#VALUE!';
47174721
}
47184722
} else {
4723+
// In theory, we should truncate here.
4724+
// But I can't figure out a formula
4725+
// using the concatenation operator
4726+
// with literals that fits in 32K,
4727+
// so I don't think we can overflow here.
47194728
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
47204729
}
47214730
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($result));

‎src/PhpSpreadsheet/Calculation/Information/ErrorValue.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public static function isError($value = '')
4747
return false;
4848
}
4949

50-
return in_array($value, ExcelError::$errorCodes, true) || $value === ExcelError::CALC();
50+
return in_array($value, ExcelError::$errorCodes, true);
5151
}
5252

5353
/**

‎src/PhpSpreadsheet/Calculation/Information/ExcelError.php‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,20 @@ class ExcelError
1414
* @var array<string, string>
1515
*/
1616
public static $errorCodes = [
17-
'null' => '#NULL!',
18-
'divisionbyzero' => '#DIV/0!',
19-
'value' => '#VALUE!',
20-
'reference' => '#REF!',
21-
'name' => '#NAME?',
22-
'num' => '#NUM!',
23-
'na' => '#N/A',
24-
'gettingdata' => '#GETTING_DATA',
25-
'spill' => '#SPILL!',
17+
'null' => '#NULL!', // 1
18+
'divisionbyzero' => '#DIV/0!', // 2
19+
'value' => '#VALUE!', // 3
20+
'reference' => '#REF!', // 4
21+
'name' => '#NAME?', // 5
22+
'num' => '#NUM!', // 6
23+
'na' => '#N/A', // 7
24+
'gettingdata' => '#GETTING_DATA', // 8
25+
'spill' => '#SPILL!', // 9
26+
'connect' => '#CONNECT!', //10
27+
'blocked' => '#BLOCKED!', //11
28+
'unknown' => '#UNKNOWN!', //12
29+
'field' => '#FIELD!', //13
30+
'calculation' => '#CALC!', //14
2631
];
2732

2833
/**
@@ -54,10 +59,6 @@ public static function type($value = '')
5459
++$i;
5560
}
5661

57-
if ($value === self::CALC()) {
58-
return 14;
59-
}
60-
6162
return self::NA();
6263
}
6364

@@ -154,6 +155,6 @@ public static function DIV0(): string
154155
*/
155156
public static function CALC(): string
156157
{
157-
return '#CALC!';
158+
return self::$errorCodes['calculation'];
158159
}
159160
}

‎src/PhpSpreadsheet/Calculation/TextData/Concatenate.php‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
66
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
7+
use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue;
78
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
9+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
10+
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
811

912
class Concatenate
1013
{
@@ -23,7 +26,18 @@ public static function CONCATENATE(...$args): string
2326
$aArgs = Functions::flattenArray($args);
2427

2528
foreach ($aArgs as $arg) {
29+
$value = Helpers::extractString($arg);
30+
if (ErrorValue::isError($value)) {
31+
$returnValue = $value;
32+
33+
break;
34+
}
2635
$returnValue .= Helpers::extractString($arg);
36+
if (StringHelper::countCharacters($returnValue) > DataType::MAX_STRING_LENGTH) {
37+
$returnValue = ExcelError::CALC();
38+
39+
break;
40+
}
2741
}
2842

2943
return $returnValue;
@@ -56,15 +70,27 @@ public static function TEXTJOIN($delimiter, $ignoreEmpty, ...$args)
5670

5771
// Loop through arguments
5872
$aArgs = Functions::flattenArray($args);
73+
$returnValue = '';
5974
foreach ($aArgs as $key => &$arg) {
75+
$value = Helpers::extractString($arg);
76+
if (ErrorValue::isError($value)) {
77+
$returnValue = $value;
78+
79+
break;
80+
}
6081
if ($ignoreEmpty === true && is_string($arg) && trim($arg) === '') {
6182
unset($aArgs[$key]);
6283
} elseif (is_bool($arg)) {
6384
$arg = Helpers::convertBooleanValue($arg);
6485
}
6586
}
6687

67-
return implode($delimiter, $aArgs);
88+
$returnValue = ($returnValue !== '') ? $returnValue : implode($delimiter, $aArgs);
89+
if (StringHelper::countCharacters($returnValue) > DataType::MAX_STRING_LENGTH) {
90+
$returnValue = ExcelError::CALC();
91+
}
92+
93+
return $returnValue;
6894
}
6995

7096
/**
@@ -90,9 +116,16 @@ public static function builtinREPT($stringValue, $repeatCount)
90116
$stringValue = Helpers::extractString($stringValue);
91117

92118
if (!is_numeric($repeatCount) || $repeatCount < 0) {
93-
return ExcelError::VALUE();
119+
$returnValue = ExcelError::VALUE();
120+
} elseif (ErrorValue::isError($stringValue)) {
121+
$returnValue = $stringValue;
122+
} else {
123+
$returnValue = str_repeat($stringValue, (int) $repeatCount);
124+
if (StringHelper::countCharacters($returnValue) > DataType::MAX_STRING_LENGTH) {
125+
$returnValue = ExcelError::VALUE(); // note VALUE not CALC
126+
}
94127
}
95128

96-
return str_repeat($stringValue, (int) $repeatCount);
129+
return $returnValue;
97130
}
98131
}

‎src/PhpSpreadsheet/Calculation/TextData/Helpers.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
66
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
77
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
8+
use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue;
89
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
910

1011
class Helpers
@@ -21,11 +22,14 @@ public static function convertBooleanValue(bool $value): string
2122
/**
2223
* @param mixed $value String value from which to extract characters
2324
*/
24-
public static function extractString($value): string
25+
public static function extractString($value, bool $throwIfError = false): string
2526
{
2627
if (is_bool($value)) {
2728
return self::convertBooleanValue($value);
2829
}
30+
if ($throwIfError && is_string($value) && ErrorValue::isError($value)) {
31+
throw new CalcExp($value);
32+
}
2933

3034
return (string) $value;
3135
}

‎src/PhpSpreadsheet/Calculation/TextData/Replace.php‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
77
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
88
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
9+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
10+
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
911

1012
class Replace
1113
{
@@ -36,16 +38,20 @@ public static function replace($oldText, $start, $chars, $newText)
3638
try {
3739
$start = Helpers::extractInt($start, 1, 0, true);
3840
$chars = Helpers::extractInt($chars, 0, 0, true);
39-
$oldText = Helpers::extractString($oldText);
40-
$newText = Helpers::extractString($newText);
41-
$left = mb_substr($oldText, 0, $start - 1, 'UTF-8');
41+
$oldText = Helpers::extractString($oldText, true);
42+
$newText = Helpers::extractString($newText, true);
43+
$left = StringHelper::substring($oldText, 0, $start - 1);
4244

43-
$right = mb_substr($oldText, $start + $chars - 1, null, 'UTF-8');
45+
$right = StringHelper::substring($oldText, $start + $chars - 1, null);
4446
} catch (CalcExp $e) {
4547
return $e->getMessage();
4648
}
49+
$returnValue = $left . $newText . $right;
50+
if (StringHelper::countCharacters($returnValue) > DataType::MAX_STRING_LENGTH) {
51+
$returnValue = ExcelError::VALUE();
52+
}
4753

48-
return $left . $newText . $right;
54+
return $returnValue;
4955
}
5056

5157
/**
@@ -71,24 +77,29 @@ public static function substitute($text = '', $fromText = '', $toText = '', $ins
7177
}
7278

7379
try {
74-
$text = Helpers::extractString($text);
75-
$fromText = Helpers::extractString($fromText);
76-
$toText = Helpers::extractString($toText);
80+
$text = Helpers::extractString($text, true);
81+
$fromText = Helpers::extractString($fromText, true);
82+
$toText = Helpers::extractString($toText, true);
7783
if ($instance === null) {
78-
return str_replace($fromText, $toText, $text);
79-
}
80-
if (is_bool($instance)) {
81-
if ($instance === false || Functions::getCompatibilityMode() !== Functions::COMPATIBILITY_OPENOFFICE) {
82-
return ExcelError::Value();
84+
$returnValue = str_replace($fromText, $toText, $text);
85+
} else {
86+
if (is_bool($instance)) {
87+
if ($instance === false || Functions::getCompatibilityMode() !== Functions::COMPATIBILITY_OPENOFFICE) {
88+
return ExcelError::Value();
89+
}
90+
$instance = 1;
8391
}
84-
$instance = 1;
92+
$instance = Helpers::extractInt($instance, 1, 0, true);
93+
$returnValue = self::executeSubstitution($text, $fromText, $toText, $instance);
8594
}
86-
$instance = Helpers::extractInt($instance, 1, 0, true);
8795
} catch (CalcExp $e) {
8896
return $e->getMessage();
8997
}
98+
if (StringHelper::countCharacters($returnValue) > DataType::MAX_STRING_LENGTH) {
99+
$returnValue = ExcelError::VALUE();
100+
}
90101

91-
return self::executeSubstitution($text, $fromText, $toText, $instance);
102+
return $returnValue;
92103
}
93104

94105
/**
@@ -106,7 +117,7 @@ private static function executeSubstitution(string $text, string $fromText, stri
106117
}
107118

108119
if ($pos !== false) {
109-
return Functions::scalar(self::REPLACE($text, ++$pos, mb_strlen($fromText, 'UTF-8'), $toText));
120+
return Functions::scalar(self::REPLACE($text, ++$pos, StringHelper::countCharacters($fromText), $toText));
110121
}
111122

112123
return $text;

‎src/PhpSpreadsheet/Cell/DataType.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@ class DataType
3131
'#NAME?' => 4,
3232
'#NUM!' => 5,
3333
'#N/A' => 6,
34+
'#CALC!' => 7,
3435
];
3536

37+
public const MAX_STRING_LENGTH = 32767;
38+
3639
/**
3740
* Get list of error codes.
3841
*
@@ -58,7 +61,7 @@ public static function checkString($textValue)
5861
}
5962

6063
// string must never be longer than 32,767 characters, truncate if necessary
61-
$textValue = StringHelper::substring((string) $textValue, 0, 32767);
64+
$textValue = StringHelper::substring((string) $textValue, 0, self::MAX_STRING_LENGTH);
6265

6366
// we require that newline is represented as "\n" in core, not as "\r\n" or "\r"
6467
$textValue = str_replace(["\r\n", "\r"], "\n", $textValue);

‎src/PhpSpreadsheet/Shared/StringHelper.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,9 @@ public static function countCharacters(string $textValue, string $encoding = 'UT
467467
*
468468
* @param string $textValue UTF-8 encoded string
469469
* @param int $offset Start offset
470-
* @param int $length Maximum number of characters in substring
470+
* @param ?int $length Maximum number of characters in substring
471471
*/
472-
public static function substring(string $textValue, int $offset, int $length = 0): string
472+
public static function substring(string $textValue, int $offset, ?int $length = 0): string
473473
{
474474
return mb_substr($textValue, $offset, $length, 'UTF-8');
475475
}

‎tests/PhpSpreadsheetTests/Calculation/StringLengthTest.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
4+
5+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
use PHPUnit\Framework\TestCase;
8+
9+
class StringLengthTest extends TestCase
10+
{
11+
public function testStringLength(): void
12+
{
13+
$spreadsheet = new Spreadsheet();
14+
$sheet = $spreadsheet->getActiveSheet();
15+
// Note use Armenian character below to make sure chars, not bytes
16+
$longstring = str_repeat('Ԁ', DataType::MAX_STRING_LENGTH - 5);
17+
$sheet->getCell('C1')->setValue($longstring);
18+
self::assertSame($longstring, $sheet->getCell('C1')->getValue());
19+
$sheet->getCell('C2')->setValue($longstring . 'abcdef');
20+
self::assertSame($longstring . 'abcde', $sheet->getCell('C2')->getValue());
21+
$sheet->getCell('C3')->setValue('abcdef');
22+
$sheet->getCell('C4')->setValue('=C1 & C3');
23+
self::assertSame($longstring . 'abcde', $sheet->getCell('C4')->getCalculatedValue(), 'truncate cell concat with cell');
24+
$sheet->getCell('C5')->setValue('=C1 & "A"');
25+
self::assertSame($longstring . 'A', $sheet->getCell('C5')->getCalculatedValue(), 'okay cell concat with literal');
26+
$sheet->getCell('C6')->setValue('=C1 & "ABCDEF"');
27+
self::assertSame($longstring . 'ABCDE', $sheet->getCell('C6')->getCalculatedValue(), 'truncate cell concat with literal');
28+
$sheet->getCell('C7')->setValue('="ABCDEF" & C1');
29+
self::assertSame('ABCDEF' . str_repeat('Ԁ', DataType::MAX_STRING_LENGTH - 6), $sheet->getCell('C7')->getCalculatedValue(), 'truncate literal concat with cell');
30+
$sheet->getCell('C8')->setValue('="ABCDE" & C1');
31+
self::assertSame('ABCDE' . $longstring, $sheet->getCell('C8')->getCalculatedValue(), 'okay literal concat with cell');
32+
$spreadsheet->disconnectWorksheets();
33+
}
34+
}

‎tests/data/Calculation/TextData/CONCATENATE.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
4+
35
return [
46
[
57
'ABCDEFGHIJ',
@@ -19,4 +21,17 @@
1921
true,
2022
],
2123
'no arguments' => ['exception'],
24+
'result just fits' => [
25+
// Note use Armenian character below to make sure chars, not bytes
26+
str_repeat('Ԁ', DataType::MAX_STRING_LENGTH - 5) . 'ABCDE',
27+
str_repeat('Ԁ', DataType::MAX_STRING_LENGTH - 5),
28+
'ABCDE',
29+
],
30+
'result too long' => [
31+
'#CALC!',
32+
str_repeat('Ԁ', DataType::MAX_STRING_LENGTH - 5),
33+
'abc',
34+
'=A2',
35+
],
36+
'propagate DIV0' => ['#DIV/0!', '1', '=2/0', '3'],
2237
];

0 commit comments

Comments
 (0)