From ec0d4af57499cf74e9c9b4223273fc50ee0c3852 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 7 Sep 2024 21:47:30 -0700 Subject: [PATCH] AMORDEGRC and Csv Reader Backports of PR #4162 and PR #4164 intended for 3.0.0. --- CHANGELOG.md | 13 ++++++++++ .../Calculation/Financial/Amortization.php | 4 +--- src/PhpSpreadsheet/Helper/Sample.php | 7 +++++- src/PhpSpreadsheet/Reader/Csv.php | 24 +++++++++++++++---- .../Functional/StreamTest.php | 12 +++++++++- .../Writer/Dompdf/PaperSizeArrayTest.php | 18 ++++++++++++-- 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42d925ae8d..1bc6b8f325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com) and this project adheres to [Semantic Versioning](https://semver.org). +## 1.29.2 - TBD + +### Fixed + +- Backported security patches. +- Support for Php8.4. +- Change to Csv Reader (see below under Deprecated). Backport of PR #4162 intended for 3.0.0. [Issue #4161](https://github.com/PHPOffice/PhpSpreadsheet/issues/4161) +- Tweaks to ROUNDUP, ROUNDDOWN, TRUNC, AMORDEGRC (results had been different under 8.4). + +### Deprecated + +- Php8.4 will deprecate the escape parameter of fgetcsv. Csv Reader is affected by this; code is changed to be unaffected, but this will mean a breaking change is coming with Php9. Any code which uses the default escape value of backslash will fail in Php9. It is recommended to explicitly set the escape value to null string before then. + ## 1.29.1 - 2024-09-03 ### Fixed diff --git a/src/PhpSpreadsheet/Calculation/Financial/Amortization.php b/src/PhpSpreadsheet/Calculation/Financial/Amortization.php index e1a1c5fefd..a531e31be6 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Amortization.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Amortization.php @@ -9,8 +9,6 @@ class Amortization { - private const ROUNDING_ADJUSTMENT = (PHP_VERSION_ID < 80400) ? 0 : 1e-14; - /** * AMORDEGRC. * @@ -82,7 +80,7 @@ public static function AMORDEGRC( $amortiseCoeff = self::getAmortizationCoefficient($rate); $rate *= $amortiseCoeff; - $rate += self::ROUNDING_ADJUSTMENT; + $rate = (float) (string) $rate; // ugly way to avoid rounding problem $fNRate = round($yearFrac * $rate * $cost, 0); $cost -= $fNRate; $fRest = $cost - $salvage; diff --git a/src/PhpSpreadsheet/Helper/Sample.php b/src/PhpSpreadsheet/Helper/Sample.php index 6244375fe3..034d79b688 100644 --- a/src/PhpSpreadsheet/Helper/Sample.php +++ b/src/PhpSpreadsheet/Helper/Sample.php @@ -8,6 +8,7 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PhpOffice\PhpSpreadsheet\Writer\IWriter; +use PhpOffice\PhpSpreadsheet\Writer\Pdf\Dompdf; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use RecursiveRegexIterator; @@ -137,7 +138,11 @@ public function write(Spreadsheet $spreadsheet, $filename, array $writers = ['Xl $writerCallback($writer); } $callStartTime = microtime(true); - $writer->save($path); + if (PHP_VERSION_ID >= 80400 && $writer instanceof Dompdf) { + @$writer->save($path); + } else { + $writer->save($path); + } $this->logWrite($writer, $path, /** @scrutinizer ignore-type */ $callStartTime); if ($this->isCli() === false) { echo 'Download ' . basename($path) . '
'; diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index 03fdc005a1..3feec8e885 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -75,9 +75,17 @@ class Csv extends BaseReader /** * The character that can escape the enclosure. * - * @var string + * @var ?string */ - private $escapeCharacter = '\\'; + private $escapeCharacter; + + /** + * 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. @@ -198,7 +206,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) { @@ -529,6 +537,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; @@ -538,7 +551,7 @@ public function setEscapeCharacter(string $escapeCharacter): self public function getEscapeCharacter(): string { - return $this->escapeCharacter; + return $this->escapeCharacter ?? self::$defaultEscapeCharacter; } /** @@ -657,8 +670,9 @@ private static function getCsv( ?int $length = null, string $separator = ',', string $enclosure = '"', - string $escape = '\\' + ?string $escape = null ) { + $escape = $escape ?? self::$defaultEscapeCharacter; if (PHP_VERSION_ID >= 80400 && $escape !== '') { return @fgetcsv($stream, $length, $separator, $enclosure, $escape); } diff --git a/tests/PhpSpreadsheetTests/Functional/StreamTest.php b/tests/PhpSpreadsheetTests/Functional/StreamTest.php index e0a4b27d65..5230078db7 100644 --- a/tests/PhpSpreadsheetTests/Functional/StreamTest.php +++ b/tests/PhpSpreadsheetTests/Functional/StreamTest.php @@ -6,6 +6,12 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; +/** + * Not clear that Dompdf will be Php8.4 compatible in time. + * Run in separate process and add version test till it is ready. + * + * @runTestsInSeparateProcesses + */ class StreamTest extends TestCase { public static function providerFormats(): array @@ -40,7 +46,11 @@ public function testAllWritersCanWriteToStream(string $format): void } else { self::assertSame(0, $stat['size']); - $writer->save($stream); + if ($format === 'Dompdf' && PHP_VERSION_ID >= 80400) { + @$writer->save($stream); + } else { + $writer->save($stream); + } self::assertIsResource($stream, 'should not close the stream for further usage out of PhpSpreadsheet'); $stat = fstat($stream); diff --git a/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php b/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php index 815d7e3dcc..bfebd2f6a1 100644 --- a/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php @@ -8,6 +8,12 @@ use PhpOffice\PhpSpreadsheet\Writer\Pdf\Dompdf; use PHPUnit\Framework\TestCase; +/** + * Not clear that Dompdf will be Php8.4 compatible in time. + * Run in separate process and add version test till it is ready. + * + * @runTestsInSeparateProcesses + */ class PaperSizeArrayTest extends TestCase { /** @var string */ @@ -35,7 +41,11 @@ public function testPaperSizeArray(): void $sheet->setCellValue('A7', 'Lorem Ipsum'); $writer = new Dompdf($spreadsheet); $this->outfile = File::temporaryFilename(); - $writer->save($this->outfile); + if (PHP_VERSION_ID >= 80400) { // hopefully temporary + @$writer->save($this->outfile); + } else { + $writer->save($this->outfile); + } $spreadsheet->disconnectWorksheets(); unset($spreadsheet); $contents = file_get_contents($this->outfile); @@ -55,7 +65,11 @@ public function testPaperSizeNotArray(): void $sheet->setCellValue('A7', 'Lorem Ipsum'); $writer = new Dompdf($spreadsheet); $this->outfile = File::temporaryFilename(); - $writer->save($this->outfile); + if (PHP_VERSION_ID >= 80400) { // hopefully temporary + @$writer->save($this->outfile); + } else { + $writer->save($this->outfile); + } $spreadsheet->disconnectWorksheets(); unset($spreadsheet); $contents = file_get_contents($this->outfile);