AMORDEGRC and Csv Reader

Backports of PR #4162 and PR #4164 intended for 3.0.0.

Author: oleibman

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

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;

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 '<a href="/download.php?type=' . pathinfo($path, PATHINFO_EXTENSION) . '&name=' . basename($path) . '">Download ' . basename($path) . '</a><br />';

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);
         }

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

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