From 352eaae3ee03ea6f43c8ae2a11bab489fcdb3a95 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 6 Sep 2024 01:11:34 -0700 Subject: [PATCH 1/2] Php 8.4 Will Deprecate fgetcsv Parameter As described in issue #4161, Php seems to be prepared to break the fgetcsv function in release 9, marking the existing usage deprecated in 8.4. This gives us a long-term problem. This PR provides a short-term solution. --- src/PhpSpreadsheet/Reader/Csv.php | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index ac4fa688ab..5f188ad94c 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -228,11 +228,11 @@ public function listWorksheetInfo(string $filename): array $delimiter = $this->delimiter ?? ''; // Loop through each line of the file in turn - $rowData = fgetcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); while (is_array($rowData)) { ++$worksheetInfo[0]['totalRows']; $worksheetInfo[0]['lastColumnIndex'] = max($worksheetInfo[0]['lastColumnIndex'], count($rowData) - 1); - $rowData = fgetcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); } $worksheetInfo[0]['lastColumnLetter'] = Coordinate::stringFromColumnIndex($worksheetInfo[0]['lastColumnIndex'] + 1); @@ -379,7 +379,7 @@ private function loadStringOrFile(string $filename, Spreadsheet $spreadsheet, bo // Loop through each line of the file in turn $delimiter = $this->delimiter ?? ''; - $rowData = fgetcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); $valueBinder = Cell::getValueBinder(); $preserveBooleanString = method_exists($valueBinder, 'getBooleanConversion') && $valueBinder->getBooleanConversion(); $this->getTrue = Calculation::getTRUE(); @@ -416,7 +416,7 @@ private function loadStringOrFile(string $filename, Spreadsheet $spreadsheet, bo } ++$columnLetter; } - $rowData = fgetcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); ++$currentRow; } @@ -649,4 +649,27 @@ public function setSheetNameIsFileName(bool $sheetNameIsFileName): self return $this; } + + /** + * Php8.4 deprecates use of anything other than null string + * as escape Character. + * + * @param resource $stream + * @param null|int<0, max> $length + * + * @return array|false + */ + private static function getCsv( + $stream, + ?int $length = null, + string $separator = ',', + string $enclosure = '"', + string $escape = '\\' + ): array|false { + if (PHP_VERSION_ID >= 80400 && $escape !== '') { + return @fgetcsv($stream, $length, $separator, $enclosure, $escape); + } + + return fgetcsv($stream, $length, $separator, $enclosure, $escape); + } } From 2a8fb60fbbd6f6dafc62a1336c169408cd3bbd49 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 7 Sep 2024 07:35:04 -0700 Subject: [PATCH 2/2] Some Long-Term Prep Enable us to determine if user has explicitly changed escape character vs. using default. --- src/PhpSpreadsheet/Reader/Csv.php | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index 5f188ad94c..762ec31910 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -62,8 +62,19 @@ class Csv extends BaseReader /** * The character that can escape the enclosure. + * This will probably become unsupported in Php 9. + * Not yet ready to mark deprecated in order to give users + * a migration path. */ - private string $escapeCharacter = '\\'; + private ?string $escapeCharacter = null; + + /** + * The character that will be supplied to fgetcsv + * when escapeCharacter is null. + * It is anticipated that it will conditionally be set + * to null-string for Php9 and above. + */ + private static string $defaultEscapeCharacter = '\\'; /** * Callback for setting defaults in construction. @@ -185,7 +196,7 @@ protected function inferSeparator(): void return; } - $inferenceEngine = new Delimiter($this->fileHandle, $this->escapeCharacter, $this->enclosure); + $inferenceEngine = new Delimiter($this->fileHandle, $this->escapeCharacter ?? self::$defaultEscapeCharacter, $this->enclosure); // If number of lines is 0, nothing to infer : fall back to the default if ($inferenceEngine->linesCounted() === 0) { @@ -527,6 +538,11 @@ public function getContiguous(): bool return $this->contiguous; } + /** + * Php9 intends to drop support for this parameter in fgetcsv. + * Not yet ready to mark deprecated in order to give users + * a migration path. + */ public function setEscapeCharacter(string $escapeCharacter): self { $this->escapeCharacter = $escapeCharacter; @@ -536,7 +552,7 @@ public function setEscapeCharacter(string $escapeCharacter): self public function getEscapeCharacter(): string { - return $this->escapeCharacter; + return $this->escapeCharacter ?? self::$defaultEscapeCharacter; } /** @@ -664,8 +680,9 @@ private static function getCsv( ?int $length = null, string $separator = ',', string $enclosure = '"', - string $escape = '\\' + ?string $escape = null ): array|false { + $escape = $escape ?? self::$defaultEscapeCharacter; if (PHP_VERSION_ID >= 80400 && $escape !== '') { return @fgetcsv($stream, $length, $separator, $enclosure, $escape); }